compose: Show error with dismissable error banners

This replaces error dialogs.  The banners are stackable.

Signed-off-by: Zixuan James Li <[email protected]>

Author: PIG208

lib/widgets/compose_box.dart

@@ -275,17 +275,65 @@ class ComposeContentController extends ComposeController<ContentValidationError>
   }
 }
 
-class _TopBar extends StatelessWidget {
-  const _TopBar({required this.showProgressIndicator});
+class SendErrorController extends ChangeNotifier {
+  SendErrorController();
+
+  Iterable<String> get errors => _errors;
+  Set<String> _errors = {};
+  set errors (Iterable<String> value) {
+    _errors = Set.from(value);
+    assert(_errors.length == value.length, 'There should not be duplicate errors');
+    notifyListeners();
+  }
+
+  void remove(String value) {
+    final removed = _errors.remove(value);
+    assert(removed);
+    notifyListeners();
+  }
+}
+
+class _TopBar extends StatefulWidget {
+  const _TopBar({required this.showProgressIndicator, required this.sendErrorController});
 
   final bool showProgressIndicator;
+  final SendErrorController sendErrorController;
+
+  @override
+  State<_TopBar> createState() => _TopBarState();
+}
+
+class _TopBarState extends State<_TopBar> {
+  @override
+  void initState() {
+    super.initState();
+    widget.sendErrorController.addListener(_sendErrorChanged);
+  }
+
+  @override
+  void dispose() {
+    widget.sendErrorController.removeListener(_sendErrorChanged);
+    super.dispose();
+  }
+
+  void _sendErrorChanged() {
+    setState(() {
+      // The actual state lives in `widget.sendErrorController`.
+    });
+  }
 
   @override
   Widget build(BuildContext context) {
+    final designVariables = DesignVariables.of(context);
     // TODO: Figure out a way so that this does not shift the message list
     //   when it gains more height.
     return Column(children: [
-        if (showProgressIndicator) _progressIndicator(context),
+      if (widget.showProgressIndicator) _progressIndicator(context),
+      for (final value in widget.sendErrorController.errors)
+        _ErrorBanner(label: value,
+          action: IconButton(icon: Icon(
+            ZulipIcons.remove, color: designVariables.btnLabelAttLowIntDanger),
+          onPressed: () => widget.sendErrorController.remove(value))),
       ]);
   }
 }
@@ -968,12 +1016,14 @@ class _SendButton extends StatefulWidget {
     required this.enabled,
     required this.topicController,
     required this.contentController,
+    required this.sendErrorController,
     required this.getDestination,
   });
 
   final ValueNotifier<bool> enabled;
   final ComposeTopicController? topicController;
   final ComposeContentController contentController;
+  final SendErrorController sendErrorController;
   final MessageDestination Function() getDestination;
 
   @override
@@ -1029,10 +1079,7 @@ class _SendButtonState extends State<_SendButton> {
         for (final error in widget.contentController.validationErrors)
           error.message(zulipLocalizations),
       ];
-      showErrorDialog(
-        context: context,
-        title: zulipLocalizations.errorMessageNotSent,
-        message: validationErrorMessages.join('\n\n'));
+      widget.sendErrorController.errors = validationErrorMessages;
       return;
     }
     if (!widget.enabled.value) {
@@ -1053,6 +1100,7 @@ class _SendButtonState extends State<_SendButton> {
         .sendMessage(destination: widget.getDestination(), content: content)
         .timeout(kSendMessageTimeout);
       widget.contentController.clear();
+      widget.sendErrorController.errors = [];
     } catch (e) {
       if (!mounted) return;
 
@@ -1064,9 +1112,7 @@ class _SendButtonState extends State<_SendButton> {
         case ApiRequestException(): message = e.message;
         default: rethrow;
       }
-      showErrorDialog(context: context,
-        title: zulipLocalizations.errorMessageNotSent,
-        message: message);
+      widget.sendErrorController.errors = [message];
       return;
     } finally {
       widget.enabled.value = true;
@@ -1196,6 +1242,7 @@ abstract class ComposeBoxController<T extends StatefulWidget> extends State<T> {
   bool get enabled;
   ComposeTopicController? get topicController;
   ComposeContentController get contentController;
+  SendErrorController get sendErrorController;
   FocusNode get contentFocusNode;
 }
 
@@ -1229,6 +1276,9 @@ class _StreamComposeBoxState extends State<_StreamComposeBox> implements Compose
   FocusNode get topicFocusNode => _topicFocusNode;
   final _topicFocusNode = FocusNode();
 
+  @override SendErrorController get sendErrorController => _sendErrorController;
+  final _sendErrorController = SendErrorController();
+
   @override
   void initState() {
     super.initState();
@@ -1241,6 +1291,7 @@ class _StreamComposeBoxState extends State<_StreamComposeBox> implements Compose
     _topicController.dispose();
     _contentController.dispose();
     _contentFocusNode.dispose();
+    _sendErrorController.dispose();
     super.dispose();
   }
 
@@ -1253,7 +1304,10 @@ class _StreamComposeBoxState extends State<_StreamComposeBox> implements Compose
   @override
   Widget build(BuildContext context) {
     return _ComposeBoxLayout(
-      topBar: _TopBar(showProgressIndicator: !enabled),
+      topBar: _TopBar(
+        showProgressIndicator: !enabled,
+        sendErrorController: sendErrorController,
+      ),
       topicInput: _TopicInput(
         enabled: enabled,
         streamId: widget.narrow.streamId,
@@ -1277,16 +1331,18 @@ class _StreamComposeBoxState extends State<_StreamComposeBox> implements Compose
         enabled: _enabled,
         topicController: _topicController,
         contentController: _contentController,
+        sendErrorController: _sendErrorController,
         getDestination: () => StreamDestination(
           widget.narrow.streamId, _topicController.textNormalized),
       ));
   }
 }
 
 class _ErrorBanner extends StatelessWidget {
-  const _ErrorBanner({required this.label});
+  const _ErrorBanner({required this.label, this.action});
 
   final String label;
+  final Widget? action;
 
   @override
   Widget build(BuildContext context) {
@@ -1298,6 +1354,11 @@ class _ErrorBanner extends StatelessWidget {
       // which is a variable equivalent to this value.
       wght: 600));
 
+    final padding = (action == null)
+      // Ensure that the text is centered when it is the only element.
+      ? const EdgeInsets.symmetric(horizontal: 16, vertical: 5)
+      : const EdgeInsetsDirectional.fromSTEB(16, 5, 8, 5);
+
     return Container(
       constraints: const BoxConstraints(minHeight: 40),
       decoration: BoxDecoration(
@@ -1307,8 +1368,9 @@ class _ErrorBanner extends StatelessWidget {
         children: [
           Expanded(
             child: Padding(
-              padding: const EdgeInsets.symmetric(horizontal: 16, vertical: 5),
+              padding: padding,
               child: Text(label, style: labelTextStyle))),
+          if (action != null) action!,
         ]));
   }
 }
@@ -1334,6 +1396,9 @@ class _FixedDestinationComposeBoxState extends State<_FixedDestinationComposeBox
   @override FocusNode get contentFocusNode => _contentFocusNode;
   final _contentFocusNode = FocusNode();
 
+  @override SendErrorController get sendErrorController => _sendErrorController;
+  final _sendErrorController = SendErrorController();
+
   @override
   void initState() {
     super.initState();
@@ -1345,6 +1410,7 @@ class _FixedDestinationComposeBoxState extends State<_FixedDestinationComposeBox
     _enabled.dispose();
     _contentController.dispose();
     _contentFocusNode.dispose();
+    _sendErrorController.dispose();
     super.dispose();
   }
 
@@ -1357,7 +1423,10 @@ class _FixedDestinationComposeBoxState extends State<_FixedDestinationComposeBox
   @override
   Widget build(BuildContext context) {
     return _ComposeBoxLayout(
-      topBar: _TopBar(showProgressIndicator: !enabled),
+      topBar: _TopBar(
+        showProgressIndicator: !enabled,
+        sendErrorController: sendErrorController,
+      ),
       topicInput: null,
       contentInput: _FixedDestinationContentInput(
         enabled: enabled,
@@ -1374,6 +1443,7 @@ class _FixedDestinationComposeBoxState extends State<_FixedDestinationComposeBox
         enabled: _enabled,
         topicController: null,
         contentController: _contentController,
+        sendErrorController: _sendErrorController,
         getDestination: () => widget.narrow.destination,
       ));
   }

test/widgets/compose_box_test.dart

@@ -30,7 +30,6 @@ import '../model/binding.dart';
 import '../model/test_store.dart';
 import '../model/typing_status_test.dart';
 import '../stdlib_checks.dart';
-import 'dialog_checks.dart';
 import 'test_app.dart';
 
 void main() {
@@ -488,9 +487,7 @@ void main() {
 
       await tester.pump(kSendMessageTimeout);
       final zulipLocalizations = GlobalLocalizations.zulipLocalizations;
-      await tester.tap(find.byWidget(checkErrorDialog(tester,
-        expectedTitle: zulipLocalizations.errorMessageNotSent,
-        expectedMessage: zulipLocalizations.errorSendMessageTimeout)));
+      check(find.text(zulipLocalizations.errorSendMessageTimeout)).findsOne();
 
       await tester.pump(longDelay);
     });
@@ -506,11 +503,8 @@ void main() {
           });
       });
       final zulipLocalizations = GlobalLocalizations.zulipLocalizations;
-      await tester.tap(find.byWidget(checkErrorDialog(tester,
-        expectedTitle: zulipLocalizations.errorMessageNotSent,
-        expectedMessage: zulipLocalizations.errorServerMessage(
-          'You do not have permission to initiate direct message conversations.'),
-      )));
+      check(find.text(zulipLocalizations.errorServerMessage(
+        'You do not have permission to initiate direct message conversations.'))).findsOne();
     });
   });
 
@@ -860,6 +854,50 @@ void main() {
         checkComposeBox(isShown: true);
       });
     });
+
+    group('dismiss banners', () {
+      testWidgets('dismiss validation error banner by clicking remove icon', (tester) async {
+        final channel = eg.stream();
+        final zulipLocalizations = GlobalLocalizations.zulipLocalizations;
+        await prepareComposeBox(tester,
+          narrow: TopicNarrow(channel.streamId, 'topic'), streams: [channel]);
+
+        await tester.tap(find.byIcon(ZulipIcons.send));
+        await tester.pump();
+        check(find.text(zulipLocalizations.contentValidationErrorEmpty)).findsOne();
+
+        await tester.tap(find.byIcon(ZulipIcons.remove));
+        await tester.pump();
+        check(find.text(zulipLocalizations.contentValidationErrorEmpty)).findsNothing();
+      });
+
+      testWidgets('dismiss error banner after a successful request', (tester) async {
+        TypingNotifier.debugEnable = false;
+        addTearDown(TypingNotifier.debugReset);
+
+        final channel = eg.stream();
+        final zulipLocalizations = GlobalLocalizations.zulipLocalizations;
+        await prepareComposeBox(tester,
+          narrow: TopicNarrow(channel.streamId, 'topic'), streams: [channel]);
+
+        await tester.tap(find.byIcon(ZulipIcons.send));
+        await tester.pump();
+        check(find.text(zulipLocalizations.contentValidationErrorEmpty)).findsOne();
+
+        connection.prepare(
+          json: SendMessageResult(id: 123).toJson(),
+          delay: const Duration(seconds: 2));
+        await tester.enterText(contentInputFinder, 'hello world');
+        check(find.text(zulipLocalizations.contentValidationErrorEmpty)).findsOne();
+
+        await tester.tap(find.byIcon(ZulipIcons.send));
+        await tester.pump();
+        check(find.text(zulipLocalizations.contentValidationErrorEmpty)).findsOne();
+
+        await tester.pump(const Duration(seconds: 2));
+        check(find.text(zulipLocalizations.contentValidationErrorEmpty)).findsNothing();
+      });
+    });
   });
 
   group('ComposeBox content input scaling', () {