Skip to content

Commit e5d9552

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 0888685 commit e5d9552

File tree

4 files changed

+199
-23
lines changed

4 files changed

+199
-23
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: 65 additions & 23 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,28 +886,30 @@ 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: [
887-
Row(crossAxisAlignment: CrossAxisAlignment.end, children: [
888-
Expanded(
889-
child: Theme(
890-
data: inputThemeData,
891-
child: Column(children: [
892-
if (topicInput != null) topicInput!,
893-
if (topicInput != null) const SizedBox(height: 8),
894-
contentInput,
895-
]))),
896-
const SizedBox(width: 8),
897-
sendButton,
898-
]),
899-
Theme(
900-
data: themeData.copyWith(
901-
iconTheme: themeData.iconTheme.copyWith(color: colorScheme.onSurfaceVariant)),
902-
child: Row(children: [
903-
_AttachFileButton(contentController: contentController, contentFocusNode: contentFocusNode),
904-
_AttachMediaButton(contentController: contentController, contentFocusNode: contentFocusNode),
905-
_AttachFromCameraButton(contentController: contentController, contentFocusNode: contentFocusNode),
906-
])),
907-
])))); }
889+
child: blockingErrorBanner != null
890+
? SizedBox(width: double.infinity, child: blockingErrorBanner)
891+
: Column(children: [
892+
Row(crossAxisAlignment: CrossAxisAlignment.end, children: [
893+
Expanded(
894+
child: Theme(
895+
data: inputThemeData,
896+
child: Column(children: [
897+
if (topicInput != null) topicInput!,
898+
if (topicInput != null) const SizedBox(height: 8),
899+
contentInput,
900+
]))),
901+
const SizedBox(width: 8),
902+
sendButton,
903+
]),
904+
Theme(
905+
data: themeData.copyWith(
906+
iconTheme: themeData.iconTheme.copyWith(color: colorScheme.onSurfaceVariant)),
907+
child: Row(children: [
908+
_AttachFileButton(contentController: contentController, contentFocusNode: contentFocusNode),
909+
_AttachMediaButton(contentController: contentController, contentFocusNode: contentFocusNode),
910+
_AttachFromCameraButton(contentController: contentController, contentFocusNode: contentFocusNode),
911+
])),
912+
])))); }
908913
}
909914

910915
abstract class ComposeBoxController<T extends StatefulWidget> extends State<T> {
@@ -973,6 +978,29 @@ class _StreamComposeBoxState extends State<_StreamComposeBox> implements Compose
973978
}
974979
}
975980

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

@@ -998,6 +1026,19 @@ class _FixedDestinationComposeBoxState extends State<_FixedDestinationComposeBox
9981026
super.dispose();
9991027
}
10001028

1029+
Widget? _errorBanner(BuildContext context) {
1030+
if (widget.narrow case DmNarrow(:final otherRecipientIds)) {
1031+
final store = PerAccountStoreWidget.of(context);
1032+
final hasDeactivatedUser = otherRecipientIds.any((id) =>
1033+
!(store.users[id]?.isActive ?? true));
1034+
if (hasDeactivatedUser) {
1035+
return _ErrorBanner(label: ZulipLocalizations.of(context)
1036+
.errorBannerDeactivatedDmLabel);
1037+
}
1038+
}
1039+
return null;
1040+
}
1041+
10011042
@override
10021043
Widget build(BuildContext context) {
10031044
return _ComposeBoxLayout(
@@ -1013,7 +1054,8 @@ class _FixedDestinationComposeBoxState extends State<_FixedDestinationComposeBox
10131054
topicController: null,
10141055
contentController: _contentController,
10151056
getDestination: () => widget.narrow.destination,
1016-
));
1057+
),
1058+
blockingErrorBanner: _errorBanner(context));
10171059
}
10181060
}
10191061

lib/widgets/theme.dart

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

133136
DesignVariables.dark() :
@@ -142,6 +145,9 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
142145
star: const HSLColor.fromAHSL(0.5, 47, 1, 0.41).toColor(),
143146
// TODO(#95) need dark-theme color
144147
editedMovedMarkerCollapsed: const Color.fromARGB(128, 146, 167, 182),
148+
errorBannerLabel: const Color.fromRGBO(241, 170, 167, 1),
149+
errorBannerBackground: const Color.fromRGBO(78, 19, 19, 1),
150+
errorBannerBorder: const Color.fromRGBO(237, 145, 140, 0.4),
145151
);
146152

147153
DesignVariables._({
@@ -153,6 +159,9 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
153159
required this.streamColorSwatches,
154160
required this.star,
155161
required this.editedMovedMarkerCollapsed,
162+
required this.errorBannerLabel,
163+
required this.errorBannerBackground,
164+
required this.errorBannerBorder,
156165
});
157166

158167
/// The [DesignVariables] from the context's active theme.
@@ -177,6 +186,9 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
177186
// Not named variables in Figma; taken from older Figma drafts, or elsewhere.
178187
final Color star;
179188
final Color editedMovedMarkerCollapsed;
189+
final Color errorBannerLabel;
190+
final Color errorBannerBackground;
191+
final Color errorBannerBorder;
180192

181193
@override
182194
DesignVariables copyWith({
@@ -188,6 +200,9 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
188200
StreamColorSwatches? streamColorSwatches,
189201
Color? star,
190202
Color? editedMovedMarkerCollapsed,
203+
Color? errorBannerLabel,
204+
Color? errorBannerBackground,
205+
Color? errorBannerBorder,
191206
}) {
192207
return DesignVariables._(
193208
bgTopBar: bgTopBar ?? this.bgTopBar,
@@ -198,6 +213,9 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
198213
streamColorSwatches: streamColorSwatches ?? this.streamColorSwatches,
199214
star: star ?? this.star,
200215
editedMovedMarkerCollapsed: editedMovedMarkerCollapsed ?? this.editedMovedMarkerCollapsed,
216+
errorBannerLabel: errorBannerLabel ?? this.errorBannerLabel,
217+
errorBannerBackground: errorBannerBackground ?? this.errorBannerBackground,
218+
errorBannerBorder: errorBannerBorder ?? this.errorBannerBorder,
201219
);
202220
}
203221

@@ -215,6 +233,9 @@ class DesignVariables extends ThemeExtension<DesignVariables> {
215233
streamColorSwatches: StreamColorSwatches.lerp(streamColorSwatches, other.streamColorSwatches, t),
216234
star: Color.lerp(star, other.star, t)!,
217235
editedMovedMarkerCollapsed: Color.lerp(editedMovedMarkerCollapsed, other.editedMovedMarkerCollapsed, t)!,
236+
errorBannerLabel: Color.lerp(errorBannerLabel, other.errorBannerLabel, t)!,
237+
errorBannerBackground: Color.lerp(errorBannerBackground, other.errorBannerBackground, t)!,
238+
errorBannerBorder: Color.lerp(errorBannerBorder, other.errorBannerBorder, t)!,
218239
);
219240
}
220241
}

test/widgets/compose_box_test.dart

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

0 commit comments

Comments
 (0)