squash! implement retry UX

See if the tests can be simplified

Author: PIG208

assets/l10n/app_en.arb

@@ -801,6 +801,10 @@
   "@messageIsMovedLabel": {
     "description": "Label for a moved message. (Use ALL CAPS for cased alphabets: Latin, Greek, Cyrillic, etc.)"
   },
+  "messageIsntSentLabel": "MESSAGE ISN'T SENT. CHECK YOUR CONNECTION.",
+  "@messageIsntSentLabel": {
+    "description": "Label for a message that isn't sent. (Use ALL CAPS for cased alphabets: Latin, Greek, Cyrillic, etc.)"
+  },
   "pollVoterNames": "({voterNames})",
   "@pollVoterNames": {
     "description": "The list of people who voted for a poll option, wrapped in parentheses.",

lib/generated/l10n/zulip_localizations.dart

@@ -1167,6 +1167,12 @@ abstract class ZulipLocalizations {
   /// **'MOVED'**
   String get messageIsMovedLabel;
 
+  /// Label for a message that isn't sent. (Use ALL CAPS for cased alphabets: Latin, Greek, Cyrillic, etc.)
+  ///
+  /// In en, this message translates to:
+  /// **'MESSAGE ISN\'T SENT. CHECK YOUR CONNECTION.'**
+  String get messageIsntSentLabel;
+
   /// The list of people who voted for a poll option, wrapped in parentheses.
   ///
   /// In en, this message translates to:

lib/generated/l10n/zulip_localizations_ar.dart

@@ -625,6 +625,9 @@ class ZulipLocalizationsAr extends ZulipLocalizations {
   @override
   String get messageIsMovedLabel => 'MOVED';
 
+  @override
+  String get messageIsntSentLabel => 'MESSAGE ISN\'T SENT. CHECK YOUR CONNECTION.';
+
   @override
   String pollVoterNames(String voterNames) {
     return '($voterNames)';

lib/generated/l10n/zulip_localizations_en.dart

@@ -625,6 +625,9 @@ class ZulipLocalizationsEn extends ZulipLocalizations {
   @override
   String get messageIsMovedLabel => 'MOVED';
 
+  @override
+  String get messageIsntSentLabel => 'MESSAGE ISN\'T SENT. CHECK YOUR CONNECTION.';
+
   @override
   String pollVoterNames(String voterNames) {
     return '($voterNames)';

lib/generated/l10n/zulip_localizations_ja.dart

@@ -625,6 +625,9 @@ class ZulipLocalizationsJa extends ZulipLocalizations {
   @override
   String get messageIsMovedLabel => 'MOVED';
 
+  @override
+  String get messageIsntSentLabel => 'MESSAGE ISN\'T SENT. CHECK YOUR CONNECTION.';
+
   @override
   String pollVoterNames(String voterNames) {
     return '($voterNames)';

lib/generated/l10n/zulip_localizations_nb.dart

@@ -625,6 +625,9 @@ class ZulipLocalizationsNb extends ZulipLocalizations {
   @override
   String get messageIsMovedLabel => 'MOVED';
 
+  @override
+  String get messageIsntSentLabel => 'MESSAGE ISN\'T SENT. CHECK YOUR CONNECTION.';
+
   @override
   String pollVoterNames(String voterNames) {
     return '($voterNames)';

lib/generated/l10n/zulip_localizations_pl.dart

@@ -625,6 +625,9 @@ class ZulipLocalizationsPl extends ZulipLocalizations {
   @override
   String get messageIsMovedLabel => 'PRZENIESIONO';
 
+  @override
+  String get messageIsntSentLabel => 'MESSAGE ISN\'T SENT. CHECK YOUR CONNECTION.';
+
   @override
   String pollVoterNames(String voterNames) {
     return '($voterNames)';

lib/generated/l10n/zulip_localizations_ru.dart

@@ -625,6 +625,9 @@ class ZulipLocalizationsRu extends ZulipLocalizations {
   @override
   String get messageIsMovedLabel => 'ПЕРЕМЕЩЕНО';
 
+  @override
+  String get messageIsntSentLabel => 'MESSAGE ISN\'T SENT. CHECK YOUR CONNECTION.';
+
   @override
   String pollVoterNames(String voterNames) {
     return '($voterNames)';

lib/generated/l10n/zulip_localizations_sk.dart

@@ -625,6 +625,9 @@ class ZulipLocalizationsSk extends ZulipLocalizations {
   @override
   String get messageIsMovedLabel => 'PRESUNUTÉ';
 
+  @override
+  String get messageIsntSentLabel => 'MESSAGE ISN\'T SENT. CHECK YOUR CONNECTION.';
+
   @override
   String pollVoterNames(String voterNames) {
     return '($voterNames)';

lib/widgets/message_list.dart

@@ -8,6 +8,7 @@ import 'package:intl/intl.dart' hide TextDirection;
 
 import '../api/model/model.dart';
 import '../generated/l10n/zulip_localizations.dart';
+import '../model/message.dart';
 import '../model/message_list.dart';
 import '../model/narrow.dart';
 import '../model/store.dart';
@@ -1479,21 +1480,67 @@ class OutboxMessageWithPossibleSender extends StatelessWidget {
 
   final MessageListOutboxMessageItem item;
 
+  // TODO should we restore the topic as well?
+  void _handlePress(BuildContext context) {
+    final content = item.message.content.endsWith('\n')
+      ? item.message.content : '${item.message.content}\n';
+
+    final composeBoxController =
+      MessageListPage.ancestorOf(context).composeBoxController;
+    composeBoxController!.content.insertPadded(content);
+    if (!composeBoxController.contentFocusNode.hasFocus) {
+      composeBoxController.contentFocusNode.requestFocus();
+    }
+
+    final store = PerAccountStoreWidget.of(context);
+    assert(store.outboxMessages.containsKey(item.message.localMessageId));
+    store.removeOutboxMessage(item.message.localMessageId);
+  }
+
   @override
   Widget build(BuildContext context) {
+    final designVariables = DesignVariables.of(context);
+    final zulipLocalizations = ZulipLocalizations.of(context);
     final message = item.message;
-    return Padding(
-      padding: const EdgeInsets.symmetric(vertical: 4),
-      child: Column(children: [
-        if (item.showSender) _SenderRow(message: message),
-        Padding(
-          padding: const EdgeInsets.symmetric(horizontal: 16),
-          // This is adapated from [MessageContent].
-          // TODO(#576): Offer InheritedMessage ancestor once we are ready
-          //   to support local echoing images and lightbox.
-          child: DefaultTextStyle(
-            style: ContentTheme.of(context).textStylePlainParagraph,
-            child: BlockContentList(nodes: item.content.nodes))),
-      ]));
+    final opacity = message.state == OutboxMessageLifecycle.failed ? 0.6 : 1.0;
+
+    final isComposeBoxOffered =
+      MessageListPage.ancestorOf(context).composeBoxController != null;
+
+    return GestureDetector(
+      onTap: isComposeBoxOffered && message.state == OutboxMessageLifecycle.failed
+        ? () => _handlePress(context) : null,
+      behavior: HitTestBehavior.opaque,
+      child: Padding(
+        padding: const EdgeInsets.symmetric(vertical: 4),
+        child: Column(children: [
+          if (item.showSender) Opacity(opacity: opacity, child: _SenderRow(message: message)),
+          Padding(
+            padding: const EdgeInsets.symmetric(horizontal: 16),
+            child: Column(crossAxisAlignment: CrossAxisAlignment.stretch,
+              children: [
+                Opacity(opacity: opacity,
+                  // This is adapated from [MessageContent].
+                  // TODO(#576): Offer InheritedMessage ancestor once we are ready
+                  //   to support local echoing images and lightbox.
+                  child: DefaultTextStyle(
+                    style: ContentTheme.of(context).textStylePlainParagraph,
+                    child: BlockContentList(nodes: item.content.nodes))),
+
+                if (message.state == OutboxMessageLifecycle.failed)
+                  Text(zulipLocalizations.messageIsntSentLabel,
+                    textAlign: TextAlign.end,
+                    style: TextStyle(
+                      color: designVariables.btnLabelAttLowIntDanger,
+                      fontSize: 12,
+                      height: 12 / 12,
+                      letterSpacing: proportionalLetterSpacing(
+                        context, 0.006, baseFontSize: 12),
+                    ).merge(weightVariableTextStyle(context, wght: 400)))
+                else LinearProgressIndicator(minHeight: 2,
+                  color: designVariables.foreground.withFadedAlpha(0.5),
+                  backgroundColor: designVariables.foreground.withFadedAlpha(0.2)),
+              ])),
+        ])));
   }
 }

test/widgets/message_list_test.dart

@@ -1366,24 +1366,146 @@ void main() {
       of: find.byType(MessageItem),
       matching: find.text(content, findRichText: true)).hitTestable();
 
-    testWidgets('sent message appear in message list after debounce timeout', (tester) async {
-      await setupMessageListPage(tester,
-        narrow: eg.topicNarrow(stream.streamId, 'topic'), streams: [stream],
-        messages: []);
+    Finder messageIsntSentErrorFinder = find.text(
+      'MESSAGE ISN\'T SENT. CHECK YOUR CONNECTION.').hitTestable();
 
-      connection.prepare(json: SendMessageResult(id: 1).toJson());
+    Future<void> sendMessageAndSucceed(WidgetTester tester, {
+      Duration delay = Duration.zero,
+    }) async {
+      connection.prepare(json: SendMessageResult(id: 1).toJson(), delay: delay);
+      await tester.enterText(contentInputFinder, content);
+      await tester.tap(find.byIcon(ZulipIcons.send));
+      await tester.pump(Duration.zero);
+    }
+
+    Future<void> sendMessageAndFail(WidgetTester tester) async {
+      // Send a message and fail.  Dismiss the error dialog as it pops up.
+      connection.prepare(apiException: eg.apiBadRequest());
       await tester.enterText(contentInputFinder, content);
       await tester.tap(find.byIcon(ZulipIcons.send));
       await tester.pump(Duration.zero);
+      await tester.tap(find.byWidget(
+          checkErrorDialog(tester, expectedTitle: 'Message not sent')));
+      await tester.pump();
+      check(outboxMessageFinder).findsOne();
+      check(messageIsntSentErrorFinder).findsOne();
+    }
+
+    testWidgets('sent message appear in message list after debounce timeout', (tester) async {
+      await setupMessageListPage(tester,
+        narrow: eg.topicNarrow(stream.streamId, 'topic'), streams: [stream],
+        messages: []);
+      await sendMessageAndSucceed(tester);
       check(outboxMessageFinder).findsNothing();
 
       await tester.pump(kLocalEchoDebounceDuration);
       check(outboxMessageFinder).findsOne();
+      check(find.descendant(
+        of: find.byType(MessageItem),
+        matching: find.byType(LinearProgressIndicator))).findsOne();
 
       await store.handleEvent(eg.messageEvent(
         eg.streamMessage(stream: stream, topic: 'topic'),
         localMessageId: store.outboxMessages.keys.single));
     });
+
+    testWidgets('failed to send message, retrieve the content to compose box', (tester) async {
+      await setupMessageListPage(tester,
+        narrow: eg.topicNarrow(stream.streamId, 'topic'), streams: [stream],
+        messages: []);
+      await sendMessageAndFail(tester);
+
+      final controller = tester.state<ComposeBoxState>(find.byType(ComposeBox)).controller;
+      check(controller.content).text.isNotNull().isEmpty();
+
+      // Tap the message.  This should put its content back into the compose box
+      // and remove it.
+      await tester.tap(outboxMessageFinder);
+      await tester.pump();
+      check(outboxMessageFinder).findsNothing();
+      check(controller.content).text.equals('$content\n\n');
+
+      await tester.pump(kLocalEchoDebounceDuration);
+    });
+
+    testWidgets('tapping does nothing if compose box is not offered', (tester) async {
+      final messages = [eg.streamMessage(stream: stream, topic: 'some topic')];
+      await setupMessageListPage(tester,
+        narrow: const CombinedFeedNarrow(), streams: [stream], subscriptions: [eg.subscription(stream)],
+        messages: messages);
+
+      // Navigate to a message list page in a topic narrow,
+      // which has a compose box.
+      connection.prepare(json:
+        eg.newestGetMessagesResult(foundOldest: true, messages: messages).toJson());
+      await tester.tap(find.text('some topic'));
+      await tester.pump(); // handle tap
+      await tester.pump(); // wait for navigation
+      await sendMessageAndFail(tester);
+
+      // Navigate back to the message list page without a compose box,
+      // where the failed to send message should still be visible.
+      await tester.pageBack();
+      await tester.pump(); // handle tap
+      await tester.pump(); // wait for navigation
+      check(contentInputFinder).findsNothing();
+      check(outboxMessageFinder).findsOne();
+      check(messageIsntSentErrorFinder).findsOne();
+
+      // Tap the failed to send message.
+      // This should not remove it from the message list.
+      await tester.tap(outboxMessageFinder);
+      await tester.pump();
+      check(outboxMessageFinder).findsOne();
+    });
+
+    testWidgets('tapping does nothing if message is still being sent', (tester) async {
+      await setupMessageListPage(tester,
+        narrow: eg.topicNarrow(stream.streamId, 'topic'), streams: [stream],
+        messages: []);
+      final controller = tester.state<ComposeBoxState>(find.byType(ComposeBox)).controller;
+
+      // Send a message and wait until the debounce timer expires but before
+      // the message is successfully sent.
+      await sendMessageAndSucceed(tester,
+        delay: kLocalEchoDebounceDuration + Duration(seconds: 1));
+      await tester.pump(kLocalEchoDebounceDuration);
+      check(controller.content).text.isNotNull().isEmpty();
+
+      await tester.tap(outboxMessageFinder);
+      await tester.pump();
+      check(outboxMessageFinder).findsOne();
+      check(controller.content).text.isNotNull().isEmpty();
+
+      // Wait till the send request completes.  The outbox message should
+      // remain visible because the message event didn't arrive.
+      await tester.pump(Duration(seconds: 1));
+      check(outboxMessageFinder).findsOne();
+      check(controller.content).text.isNotNull().isEmpty();
+
+      // Dispose pending timers from the message store.
+      store.dispose();
+    });
+
+    testWidgets('tapping does nothing if message was successfully sent and before message event arrives', (tester) async {
+      await setupMessageListPage(tester,
+        narrow: eg.topicNarrow(stream.streamId, 'topic'), streams: [stream],
+        messages: []);
+      final controller = tester.state<ComposeBoxState>(find.byType(ComposeBox)).controller;
+
+      // Send a message and wait until the debounce timer expires.
+      await sendMessageAndSucceed(tester);
+      await tester.pump(kLocalEchoDebounceDuration);
+      check(controller.content).text.isNotNull().isEmpty();
+
+      await tester.tap(outboxMessageFinder);
+      await tester.pump();
+      check(outboxMessageFinder).findsOne();
+      check(controller.content).text.isNotNull().isEmpty();
+
+      // Dispose pending timers from the message store.
+      store.dispose();
+    });
   });
 
   group('Starred messages', () {