Skip to content

Commit 1000957

Browse files
compose_box: Replace compose box with a banner in DMs with deactivated users
Fixes: #675 Co-authored-by: Rajesh Malviya <[email protected]>
1 parent 30b4be2 commit 1000957

File tree

5 files changed

+195
-20
lines changed

5 files changed

+195
-20
lines changed

assets/l10n/app_en.arb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,10 @@
180180
"@successMessageLinkCopied": {
181181
"description": "Message when link of a message was copied to the user's system clipboard."
182182
},
183+
"errorBannerDeactivatedDmLabel": "You cannot send messages to deactivated users.",
184+
"@errorBannerDeactivatedDmLabel": {
185+
"description": "Label text for error banner when sending a message to one or multiple deactivated users."
186+
},
183187
"composeBoxAttachFilesTooltip": "Attach files",
184188
"@composeBoxAttachFilesTooltip": {
185189
"description": "Tooltip for compose box icon to attach a file to the message."

lib/widgets/compose_box.dart

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import '../model/store.dart';
1616
import 'autocomplete.dart';
1717
import 'dialog.dart';
1818
import 'store.dart';
19+
import 'theme.dart';
1920

2021
const double _inputVerticalPadding = 8;
2122
const double _sendButtonSize = 36;
@@ -850,11 +851,13 @@ class _ComposeBoxLayout extends StatelessWidget {
850851
required this.sendButton,
851852
required this.contentController,
852853
required this.contentFocusNode,
854+
this.blockingErrorBanner,
853855
});
854856

855857
final Widget? topicInput;
856858
final Widget contentInput;
857859
final Widget sendButton;
860+
final Widget? blockingErrorBanner;
858861
final ComposeContentController contentController;
859862
final FocusNode contentFocusNode;
860863

@@ -883,7 +886,7 @@ class _ComposeBoxLayout extends StatelessWidget {
883886
minimum: const EdgeInsets.fromLTRB(8, 0, 8, 8),
884887
child: Padding(
885888
padding: const EdgeInsets.only(top: 8.0),
886-
child: Column(children: [
889+
child: blockingErrorBanner ?? Column(children: [
887890
Row(crossAxisAlignment: CrossAxisAlignment.end, children: [
888891
Expanded(
889892
child: Theme(
@@ -973,6 +976,29 @@ class _StreamComposeBoxState extends State<_StreamComposeBox> implements Compose
973976
}
974977
}
975978

979+
class _ErrorBanner extends StatelessWidget {
980+
const _ErrorBanner({required this.label});
981+
982+
final String label;
983+
984+
@override
985+
Widget build(BuildContext context) {
986+
final designVariables = DesignVariables.of(context);
987+
return Container(
988+
padding: const EdgeInsets.all(8),
989+
decoration: BoxDecoration(
990+
color: designVariables.errorBannerBackground,
991+
border: Border.all(color: designVariables.errorBannerBorder),
992+
borderRadius: BorderRadius.circular(5)),
993+
child: Text(label,
994+
maxLines: 2,
995+
overflow: TextOverflow.ellipsis,
996+
style: TextStyle(fontSize: 18, color: designVariables.errorBannerLabel),
997+
),
998+
);
999+
}
1000+
}
1001+
9761002
class _FixedDestinationComposeBox extends StatefulWidget {
9771003
const _FixedDestinationComposeBox({super.key, required this.narrow});
9781004

@@ -998,6 +1024,19 @@ class _FixedDestinationComposeBoxState extends State<_FixedDestinationComposeBox
9981024
super.dispose();
9991025
}
10001026

1027+
Widget? _errorBanner(BuildContext context) {
1028+
if (widget.narrow case DmNarrow(:final otherRecipientIds)) {
1029+
final store = PerAccountStoreWidget.of(context);
1030+
final hasDeactivatedUser = otherRecipientIds.any((id) =>
1031+
!(store.users[id]?.isActive ?? false));
1032+
if (hasDeactivatedUser) {
1033+
return _ErrorBanner(label: ZulipLocalizations.of(context)
1034+
.errorBannerDeactivatedDmLabel);
1035+
}
1036+
}
1037+
return null;
1038+
}
1039+
10011040
@override
10021041
Widget build(BuildContext context) {
10031042
return _ComposeBoxLayout(
@@ -1013,7 +1052,8 @@ class _FixedDestinationComposeBoxState extends State<_FixedDestinationComposeBox
10131052
topicController: null,
10141053
contentController: _contentController,
10151054
getDestination: () => widget.narrow.destination,
1016-
));
1055+
),
1056+
blockingErrorBanner: _errorBanner(context));
10171057
}
10181058
}
10191059

lib/widgets/message_list.dart

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -117,24 +117,25 @@ class _MessageListPageState extends State<MessageListPage> implements MessageLis
117117
// https://www.figma.com/file/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=147%3A9088&mode=dev
118118
body: Builder(
119119
builder: (BuildContext context) => Center(
120-
child: Column(children: [
121-
MediaQuery.removePadding(
122-
// Scaffold knows about the app bar, and so has run this
123-
// BuildContext, which is under `body`, through
124-
// MediaQuery.removePadding with `removeTop: true`.
125-
context: context,
126-
127-
// The compose box, when present, pads the bottom inset.
128-
// TODO this copies the details of when the compose box is shown;
129-
// if those details get complicated, refactor to avoid copying.
130-
// TODO(#311) If we have a bottom nav, it will pad the bottom
131-
// inset, and this should always be true.
132-
removeBottom: widget.narrow is! CombinedFeedNarrow,
133-
134-
child: Expanded(
135-
child: MessageList(narrow: widget.narrow))),
136-
ComposeBox(controllerKey: _composeBoxKey, narrow: widget.narrow),
137-
]))));
120+
child: Column(crossAxisAlignment: CrossAxisAlignment.stretch,
121+
children: [
122+
MediaQuery.removePadding(
123+
// Scaffold knows about the app bar, and so has run this
124+
// BuildContext, which is under `body`, through
125+
// MediaQuery.removePadding with `removeTop: true`.
126+
context: context,
127+
128+
// The compose box, when present, pads the bottom inset.
129+
// TODO this copies the details of when the compose box is shown;
130+
// if those details get complicated, refactor to avoid copying.
131+
// TODO(#311) If we have a bottom nav, it will pad the bottom
132+
// inset, and this should always be true.
133+
removeBottom: widget.narrow is! CombinedFeedNarrow,
134+
135+
child: Expanded(
136+
child: MessageList(narrow: widget.narrow))),
137+
ComposeBox(controllerKey: _composeBoxKey, narrow: widget.narrow),
138+
]))));
138139
}
139140
}
140141

lib/widgets/theme.dart

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,9 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
125125
streamColorSwatches: StreamColorSwatches.light,
126126
star: const HSLColor.fromAHSL(0.5, 47, 1, 0.41).toColor(),
127127
editedMovedMarkerCollapsed: const Color.fromARGB(128, 146, 167, 182),
128+
errorBannerLabel: const Color.fromRGBO(133, 42, 35, 1),
129+
errorBannerBackground: const Color.fromRGBO(238, 222, 221, 1),
130+
errorBannerBorder: const Color.fromRGBO(132, 41, 36, 0.4),
128131
);
129132

130133
DesignVariables.dark() :
@@ -139,6 +142,9 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
139142
star: const HSLColor.fromAHSL(0.5, 47, 1, 0.41).toColor(),
140143
// TODO(#95) need dark-theme color
141144
editedMovedMarkerCollapsed: const Color.fromARGB(128, 146, 167, 182),
145+
errorBannerLabel: const Color.fromRGBO(241, 170, 167, 1),
146+
errorBannerBackground: const Color.fromRGBO(78, 19, 19, 1),
147+
errorBannerBorder: const Color.fromRGBO(237, 145, 140, 0.4),
142148
);
143149

144150
DesignVariables._({
@@ -150,6 +156,9 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
150156
required this.streamColorSwatches,
151157
required this.star,
152158
required this.editedMovedMarkerCollapsed,
159+
required this.errorBannerLabel,
160+
required this.errorBannerBackground,
161+
required this.errorBannerBorder,
153162
});
154163

155164
/// The [DesignVariables] from the context's active theme.
@@ -174,6 +183,9 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
174183
// Not named variables in Figma; taken from older Figma drafts, or elsewhere.
175184
final Color star;
176185
final Color editedMovedMarkerCollapsed;
186+
final Color errorBannerLabel;
187+
final Color errorBannerBackground;
188+
final Color errorBannerBorder;
177189

178190
@override
179191
DesignVariables copyWith({
@@ -185,6 +197,9 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
185197
StreamColorSwatches? streamColorSwatches,
186198
Color? star,
187199
Color? editedMovedMarkerCollapsed,
200+
Color? errorBannerLabel,
201+
Color? errorBannerBackground,
202+
Color? errorBannerBorder,
188203
}) {
189204
return DesignVariables._(
190205
bgTopBar: bgTopBar ?? this.bgTopBar,
@@ -195,6 +210,9 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
195210
streamColorSwatches: streamColorSwatches ?? this.streamColorSwatches,
196211
star: star ?? this.star,
197212
editedMovedMarkerCollapsed: editedMovedMarkerCollapsed ?? this.editedMovedMarkerCollapsed,
213+
errorBannerLabel: errorBannerLabel ?? this.errorBannerLabel,
214+
errorBannerBackground: errorBannerBackground ?? this.errorBannerBackground,
215+
errorBannerBorder: errorBannerBorder ?? this.errorBannerBorder,
198216
);
199217
}
200218

@@ -212,6 +230,9 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
212230
streamColorSwatches: StreamColorSwatches.lerp(streamColorSwatches, other.streamColorSwatches, t),
213231
star: Color.lerp(star, other.star, t)!,
214232
editedMovedMarkerCollapsed: Color.lerp(editedMovedMarkerCollapsed, other.editedMovedMarkerCollapsed, t)!,
233+
errorBannerLabel: Color.lerp(errorBannerLabel, other.errorBannerLabel, t)!,
234+
errorBannerBackground: Color.lerp(errorBannerBackground, other.errorBannerBackground, t)!,
235+
errorBannerBorder: Color.lerp(errorBannerBorder, other.errorBannerBorder, t)!,
215236
);
216237
}
217238
}

test/widgets/compose_box_test.dart

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import 'package:flutter/material.dart';
77
import 'package:flutter_gen/gen_l10n/zulip_localizations.dart';
88
import 'package:flutter_test/flutter_test.dart';
99
import 'package:image_picker/image_picker.dart';
10+
import 'package:zulip/api/model/events.dart';
1011
import 'package:zulip/api/model/model.dart';
1112
import 'package:zulip/api/route/messages.dart';
1213
import 'package:zulip/model/localizations.dart';
@@ -373,4 +374,112 @@ void main() {
373374
// TODO test what happens when capturing/uploading fails
374375
});
375376
});
377+
378+
group('compose box in DMs with deactivated users', () {
379+
Finder contentFieldFinder() => find.descendant(
380+
of: find.byType(ComposeBox),
381+
matching: find.byType(TextField));
382+
383+
Finder attachButtonFinder(IconData icon) => find.descendant(
384+
of: find.byType(ComposeBox),
385+
matching: find.widgetWithIcon(IconButton, icon));
386+
387+
void checkComposeBoxParts({required bool areShown}) {
388+
check(contentFieldFinder().evaluate().length).equals(areShown ? 1 : 0);
389+
check(attachButtonFinder(Icons.attach_file).evaluate().length).equals(areShown ? 1 : 0);
390+
check(attachButtonFinder(Icons.image).evaluate().length).equals(areShown ? 1 : 0);
391+
check(attachButtonFinder(Icons.camera_alt).evaluate().length).equals(areShown ? 1 : 0);
392+
}
393+
394+
void checkBanner({required bool isShown}) {
395+
final bannerTextFinder = find.text(GlobalLocalizations.zulipLocalizations
396+
.errorBannerDeactivatedDmLabel);
397+
check(bannerTextFinder.evaluate().length).equals(isShown ? 1 : 0);
398+
}
399+
400+
void checkComposeBox({required bool isShown}) {
401+
checkComposeBoxParts(areShown: isShown);
402+
checkBanner(isShown: !isShown);
403+
}
404+
405+
Future<void> changeUserStatus(WidgetTester tester,
406+
{required User user, required bool isActive}) async {
407+
await store.handleEvent(RealmUserUpdateEvent(id: 1,
408+
userId: user.userId, isActive: isActive));
409+
await tester.pump();
410+
}
411+
412+
final selfUser = eg.selfUser;
413+
414+
DmNarrow dmNarrowWith(User otherUser) => DmNarrow.withUser(otherUser.userId,
415+
selfUserId: selfUser.userId);
416+
417+
DmNarrow groupDmNarrowWith(List<User> otherUsers) => DmNarrow.withOtherUsers(
418+
otherUsers.map((u) => u.userId), selfUserId: selfUser.userId);
419+
420+
group('1:1 DMs', () {
421+
testWidgets('compose box replaced with a banner', (tester) async {
422+
final deactivatedUser = eg.user(isActive: false);
423+
await prepareComposeBox(tester, narrow: dmNarrowWith(deactivatedUser),
424+
users: [deactivatedUser]);
425+
checkComposeBox(isShown: false);
426+
});
427+
428+
testWidgets('active user becomes deactivated -> '
429+
'compose box is replaced with a banner', (tester) async {
430+
final activeUser = eg.user(isActive: true);
431+
await prepareComposeBox(tester, narrow: dmNarrowWith(activeUser),
432+
users: [activeUser]);
433+
checkComposeBox(isShown: true);
434+
435+
await changeUserStatus(tester, user: activeUser, isActive: false);
436+
checkComposeBox(isShown: false);
437+
});
438+
439+
testWidgets('deactivated user becomes active -> '
440+
'banner is replaced with the compose box', (tester) async {
441+
final deactivatedUser = eg.user(isActive: false);
442+
await prepareComposeBox(tester, narrow: dmNarrowWith(deactivatedUser),
443+
users: [deactivatedUser]);
444+
checkComposeBox(isShown: false);
445+
446+
await changeUserStatus(tester, user: deactivatedUser, isActive: true);
447+
checkComposeBox(isShown: true);
448+
});
449+
});
450+
451+
group('group DMs', () {
452+
testWidgets('compose box replaced with a banner', (tester) async {
453+
final deactivatedUsers = [eg.user(isActive: false), eg.user(isActive: false)];
454+
await prepareComposeBox(tester, narrow: groupDmNarrowWith(deactivatedUsers),
455+
users: deactivatedUsers);
456+
checkComposeBox(isShown: false);
457+
});
458+
459+
testWidgets('at least one user becomes deactivated -> '
460+
'compose box is replaced with a banner', (tester) async {
461+
final activeUsers = [eg.user(isActive: true), eg.user(isActive: true)];
462+
await prepareComposeBox(tester, narrow: groupDmNarrowWith(activeUsers),
463+
users: activeUsers);
464+
checkComposeBox(isShown: true);
465+
466+
await changeUserStatus(tester, user: activeUsers[0], isActive: false);
467+
checkComposeBox(isShown: false);
468+
});
469+
470+
testWidgets('all deactivated users become active -> '
471+
'banner is replaced with the compose box', (tester) async {
472+
final deactivatedUsers = [eg.user(isActive: false), eg.user(isActive: false)];
473+
await prepareComposeBox(tester, narrow: groupDmNarrowWith(deactivatedUsers),
474+
users: deactivatedUsers);
475+
checkComposeBox(isShown: false);
476+
477+
await changeUserStatus(tester, user: deactivatedUsers[0], isActive: true);
478+
checkComposeBox(isShown: false);
479+
480+
await changeUserStatus(tester, user: deactivatedUsers[1], isActive: true);
481+
checkComposeBox(isShown: true);
482+
});
483+
});
484+
});
376485
}

0 commit comments

Comments
 (0)