Skip to content

Commit b389434

Browse files
PIG208PIG208
committed
msglist: Support retrieving failed outbox message content
1 parent 35eb9ed commit b389434

12 files changed

+249
-21
lines changed

‎assets/l10n/app_en.arb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -801,6 +801,10 @@
801801
"@messageIsMovedLabel": {
802802
"description": "Label for a moved message. (Use ALL CAPS for cased alphabets: Latin, Greek, Cyrillic, etc.)"
803803
},
804+
"messageIsntSentLabel": "MESSAGE ISN'T SENT. CHECK YOUR CONNECTION.",
805+
"@messageIsntSentLabel": {
806+
"description": "Label for a message that isn't sent. (Use ALL CAPS for cased alphabets: Latin, Greek, Cyrillic, etc.)"
807+
},
804808
"pollVoterNames": "({voterNames})",
805809
"@pollVoterNames": {
806810
"description": "The list of people who voted for a poll option, wrapped in parentheses.",

‎lib/generated/l10n/zulip_localizations.dart

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1167,6 +1167,12 @@ abstract class ZulipLocalizations {
11671167
/// **'MOVED'**
11681168
String get messageIsMovedLabel;
11691169

1170+
/// Label for a message that isn't sent. (Use ALL CAPS for cased alphabets: Latin, Greek, Cyrillic, etc.)
1171+
///
1172+
/// In en, this message translates to:
1173+
/// **'MESSAGE ISN\'T SENT. CHECK YOUR CONNECTION.'**
1174+
String get messageIsntSentLabel;
1175+
11701176
/// The list of people who voted for a poll option, wrapped in parentheses.
11711177
///
11721178
/// In en, this message translates to:

‎lib/generated/l10n/zulip_localizations_ar.dart

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -625,6 +625,9 @@ class ZulipLocalizationsAr extends ZulipLocalizations {
625625
@override
626626
String get messageIsMovedLabel => 'MOVED';
627627

628+
@override
629+
String get messageIsntSentLabel => 'MESSAGE ISN\'T SENT. CHECK YOUR CONNECTION.';
630+
628631
@override
629632
String pollVoterNames(String voterNames) {
630633
return '($voterNames)';

‎lib/generated/l10n/zulip_localizations_en.dart

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -625,6 +625,9 @@ class ZulipLocalizationsEn extends ZulipLocalizations {
625625
@override
626626
String get messageIsMovedLabel => 'MOVED';
627627

628+
@override
629+
String get messageIsntSentLabel => 'MESSAGE ISN\'T SENT. CHECK YOUR CONNECTION.';
630+
628631
@override
629632
String pollVoterNames(String voterNames) {
630633
return '($voterNames)';

‎lib/generated/l10n/zulip_localizations_ja.dart

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -625,6 +625,9 @@ class ZulipLocalizationsJa extends ZulipLocalizations {
625625
@override
626626
String get messageIsMovedLabel => 'MOVED';
627627

628+
@override
629+
String get messageIsntSentLabel => 'MESSAGE ISN\'T SENT. CHECK YOUR CONNECTION.';
630+
628631
@override
629632
String pollVoterNames(String voterNames) {
630633
return '($voterNames)';

‎lib/generated/l10n/zulip_localizations_nb.dart

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -625,6 +625,9 @@ class ZulipLocalizationsNb extends ZulipLocalizations {
625625
@override
626626
String get messageIsMovedLabel => 'MOVED';
627627

628+
@override
629+
String get messageIsntSentLabel => 'MESSAGE ISN\'T SENT. CHECK YOUR CONNECTION.';
630+
628631
@override
629632
String pollVoterNames(String voterNames) {
630633
return '($voterNames)';

‎lib/generated/l10n/zulip_localizations_pl.dart

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -625,6 +625,9 @@ class ZulipLocalizationsPl extends ZulipLocalizations {
625625
@override
626626
String get messageIsMovedLabel => 'PRZENIESIONO';
627627

628+
@override
629+
String get messageIsntSentLabel => 'MESSAGE ISN\'T SENT. CHECK YOUR CONNECTION.';
630+
628631
@override
629632
String pollVoterNames(String voterNames) {
630633
return '($voterNames)';

‎lib/generated/l10n/zulip_localizations_ru.dart

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -625,6 +625,9 @@ class ZulipLocalizationsRu extends ZulipLocalizations {
625625
@override
626626
String get messageIsMovedLabel => 'ПЕРЕМЕЩЕНО';
627627

628+
@override
629+
String get messageIsntSentLabel => 'MESSAGE ISN\'T SENT. CHECK YOUR CONNECTION.';
630+
628631
@override
629632
String pollVoterNames(String voterNames) {
630633
return '($voterNames)';

‎lib/generated/l10n/zulip_localizations_sk.dart

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -625,6 +625,9 @@ class ZulipLocalizationsSk extends ZulipLocalizations {
625625
@override
626626
String get messageIsMovedLabel => 'PRESUNUTÉ';
627627

628+
@override
629+
String get messageIsntSentLabel => 'MESSAGE ISN\'T SENT. CHECK YOUR CONNECTION.';
630+
628631
@override
629632
String pollVoterNames(String voterNames) {
630633
return '($voterNames)';

‎lib/model/message.dart

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -438,9 +438,8 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore {
438438
if (outboxMessages.containsKey(localMessageId)) {
439439
removeOutboxMessage(localMessageId);
440440
}
441-
// The outbox message can be missing if the user removes it (to be
442-
// implemented in #1441) before the event arrives.
443-
// Nothing to do in that case.
441+
// The outbox message can be missing if the user removes it before the
442+
// event arrives. Nothing to do in that case.
444443
}
445444

446445
for (final view in _messageListViews) {

‎lib/widgets/message_list.dart

Lines changed: 61 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import 'package:intl/intl.dart' hide TextDirection;
88

99
import '../api/model/model.dart';
1010
import '../generated/l10n/zulip_localizations.dart';
11+
import '../model/message.dart';
1112
import '../model/message_list.dart';
1213
import '../model/narrow.dart';
1314
import '../model/store.dart';
@@ -1483,21 +1484,68 @@ class OutboxMessageWithPossibleSender extends StatelessWidget {
14831484

14841485
final MessageListOutboxMessageItem item;
14851486

1487+
// TODO should we restore the topic as well?
1488+
void _handlePress(BuildContext context) {
1489+
final content = item.message.content.endsWith('\n')
1490+
? item.message.content : '${item.message.content}\n';
1491+
1492+
final composeBoxController =
1493+
MessageListPage.ancestorOf(context).composeBoxController;
1494+
composeBoxController!.content.insertPadded(content);
1495+
if (!composeBoxController.contentFocusNode.hasFocus) {
1496+
composeBoxController.contentFocusNode.requestFocus();
1497+
}
1498+
1499+
final store = PerAccountStoreWidget.of(context);
1500+
assert(store.outboxMessages.containsKey(item.message.localMessageId));
1501+
store.removeOutboxMessage(item.message.localMessageId);
1502+
}
1503+
14861504
@override
14871505
Widget build(BuildContext context) {
1506+
final designVariables = DesignVariables.of(context);
1507+
final zulipLocalizations = ZulipLocalizations.of(context);
14881508
final message = item.message;
1489-
return Padding(
1490-
padding: const EdgeInsets.symmetric(vertical: 4),
1491-
child: Column(children: [
1492-
if (item.showSender) _SenderRow(message: message, showTimestamp: false),
1493-
Padding(
1494-
padding: const EdgeInsets.symmetric(horizontal: 16),
1495-
// This is adapated from [MessageContent].
1496-
// TODO(#576): Offer InheritedMessage ancestor once we are ready
1497-
// to support local echoing images and lightbox.
1498-
child: DefaultTextStyle(
1499-
style: ContentTheme.of(context).textStylePlainParagraph,
1500-
child: BlockContentList(nodes: item.content.nodes))),
1501-
]));
1509+
final opacity = message.state == OutboxMessageLifecycle.failed ? 0.6 : 1.0;
1510+
1511+
final isComposeBoxOffered =
1512+
MessageListPage.ancestorOf(context).composeBoxController != null;
1513+
1514+
return GestureDetector(
1515+
onTap: isComposeBoxOffered && message.state == OutboxMessageLifecycle.failed
1516+
? () => _handlePress(context) : null,
1517+
behavior: HitTestBehavior.opaque,
1518+
child: Padding(
1519+
padding: const EdgeInsets.symmetric(vertical: 4),
1520+
child: Column(children: [
1521+
if (item.showSender) Opacity(opacity: opacity,
1522+
child: _SenderRow(message: message, showTimestamp: false)),
1523+
Padding(
1524+
padding: const EdgeInsets.symmetric(horizontal: 16),
1525+
child: Column(crossAxisAlignment: CrossAxisAlignment.stretch,
1526+
children: [
1527+
Opacity(opacity: opacity,
1528+
// This is adapated from [MessageContent].
1529+
// TODO(#576): Offer InheritedMessage ancestor once we are ready
1530+
// to support local echoing images and lightbox.
1531+
child: DefaultTextStyle(
1532+
style: ContentTheme.of(context).textStylePlainParagraph,
1533+
child: BlockContentList(nodes: item.content.nodes))),
1534+
1535+
if (message.state == OutboxMessageLifecycle.failed)
1536+
Text(zulipLocalizations.messageIsntSentLabel,
1537+
textAlign: TextAlign.end,
1538+
style: TextStyle(
1539+
color: designVariables.btnLabelAttLowIntDanger,
1540+
fontSize: 12,
1541+
height: 12 / 12,
1542+
letterSpacing: proportionalLetterSpacing(
1543+
context, 0.006, baseFontSize: 12),
1544+
).merge(weightVariableTextStyle(context, wght: 400)))
1545+
else LinearProgressIndicator(minHeight: 2,
1546+
color: designVariables.foreground.withFadedAlpha(0.5),
1547+
backgroundColor: designVariables.foreground.withFadedAlpha(0.2)),
1548+
])),
1549+
])));
15021550
}
15031551
}

‎test/widgets/message_list_test.dart

Lines changed: 155 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1366,24 +1366,174 @@ void main() {
13661366
of: find.byType(MessageItem),
13671367
matching: find.text(content, findRichText: true)).hitTestable();
13681368

1369-
testWidgets('sent message appear in message list after debounce timeout', (tester) async {
1370-
await setupMessageListPage(tester,
1371-
narrow: eg.topicNarrow(stream.streamId, 'topic'), streams: [stream],
1372-
messages: []);
1369+
Finder messageIsntSentErrorFinder = find.text(
1370+
'MESSAGE ISN\'T SENT. CHECK YOUR CONNECTION.').hitTestable();
13731371

1374-
connection.prepare(json: SendMessageResult(id: 1).toJson());
1372+
Future<void> sendMessageAndSucceed(WidgetTester tester, {
1373+
Duration delay = Duration.zero,
1374+
}) async {
1375+
connection.prepare(json: SendMessageResult(id: 1).toJson(), delay: delay);
1376+
await tester.enterText(contentInputFinder, content);
1377+
await tester.tap(find.byIcon(ZulipIcons.send));
1378+
await tester.pump(Duration.zero);
1379+
}
1380+
1381+
Future<void> sendMessageAndFail(WidgetTester tester) async {
1382+
// Send a message and fail. Dismiss the error dialog as it pops up.
1383+
connection.prepare(apiException: eg.apiBadRequest());
13751384
await tester.enterText(contentInputFinder, content);
13761385
await tester.tap(find.byIcon(ZulipIcons.send));
13771386
await tester.pump(Duration.zero);
1387+
await tester.tap(find.byWidget(
1388+
checkErrorDialog(tester, expectedTitle: 'Message not sent')));
1389+
await tester.pump();
1390+
check(outboxMessageFinder).findsOne();
1391+
check(messageIsntSentErrorFinder).findsOne();
1392+
}
1393+
1394+
testWidgets('sent message appear in message list after debounce timeout', (tester) async {
1395+
await setupMessageListPage(tester,
1396+
narrow: eg.topicNarrow(stream.streamId, 'topic'), streams: [stream],
1397+
messages: []);
1398+
await sendMessageAndSucceed(tester);
13781399
check(outboxMessageFinder).findsNothing();
13791400

13801401
await tester.pump(kLocalEchoDebounceDuration);
13811402
check(outboxMessageFinder).findsOne();
1403+
check(find.descendant(
1404+
of: find.byType(MessageItem),
1405+
matching: find.byType(LinearProgressIndicator))).findsOne();
13821406

13831407
await store.handleEvent(eg.messageEvent(
13841408
eg.streamMessage(stream: stream, topic: 'topic'),
13851409
localMessageId: store.outboxMessages.keys.single));
13861410
});
1411+
1412+
testWidgets('failed to send message, retrieve the content to compose box', (tester) async {
1413+
await setupMessageListPage(tester,
1414+
narrow: eg.topicNarrow(stream.streamId, 'topic'), streams: [stream],
1415+
messages: []);
1416+
await sendMessageAndFail(tester);
1417+
1418+
final controller = tester.state<ComposeBoxState>(find.byType(ComposeBox)).controller;
1419+
check(controller.content).text.isNotNull().isEmpty();
1420+
1421+
// Tap the message. This should put its content back into the compose box
1422+
// and remove it.
1423+
await tester.tap(outboxMessageFinder);
1424+
await tester.pump();
1425+
check(outboxMessageFinder).findsNothing();
1426+
check(controller.content).text.equals('$content\n\n');
1427+
1428+
await tester.pump(kLocalEchoDebounceDuration);
1429+
});
1430+
1431+
testWidgets('message sent, reaches time limit and fail, retrieve the content to compose box, then message event arrives', (tester) async {
1432+
await setupMessageListPage(tester,
1433+
narrow: eg.topicNarrow(stream.streamId, 'topic'), streams: [stream],
1434+
messages: []);
1435+
await sendMessageAndSucceed(tester);
1436+
await tester.pump(kSendMessageTimeLimit);
1437+
check(messageIsntSentErrorFinder).findsOne();
1438+
final localMessageId = store.outboxMessages.keys.single;
1439+
1440+
final controller = tester.state<ComposeBoxState>(find.byType(ComposeBox)).controller;
1441+
check(controller.content).text.isNotNull().isEmpty();
1442+
1443+
// Tap the message. This should put its content back into the compose box
1444+
// and remove it.
1445+
await tester.tap(outboxMessageFinder);
1446+
await tester.pump();
1447+
check(outboxMessageFinder).findsNothing();
1448+
check(controller.content).text.equals('$content\n\n');
1449+
check(store.outboxMessages).isEmpty();
1450+
1451+
// While `localMessageId` is no longer in store, there should be no error
1452+
// when a message event refers to it.
1453+
await store.handleEvent(eg.messageEvent(
1454+
eg.streamMessage(stream: stream, topic: 'topic'),
1455+
localMessageId: localMessageId));
1456+
});
1457+
1458+
1459+
testWidgets('tapping does nothing if compose box is not offered', (tester) async {
1460+
final messages = [eg.streamMessage(stream: stream, topic: 'some topic')];
1461+
await setupMessageListPage(tester,
1462+
narrow: const CombinedFeedNarrow(), streams: [stream], subscriptions: [eg.subscription(stream)],
1463+
messages: messages);
1464+
1465+
// Navigate to a message list page in a topic narrow,
1466+
// which has a compose box.
1467+
connection.prepare(json:
1468+
eg.newestGetMessagesResult(foundOldest: true, messages: messages).toJson());
1469+
await tester.tap(find.text('some topic'));
1470+
await tester.pump(); // handle tap
1471+
await tester.pump(); // wait for navigation
1472+
await sendMessageAndFail(tester);
1473+
1474+
// Navigate back to the message list page without a compose box,
1475+
// where the failed to send message should still be visible.
1476+
await tester.pageBack();
1477+
await tester.pump(); // handle tap
1478+
await tester.pump(); // wait for navigation
1479+
check(contentInputFinder).findsNothing();
1480+
check(outboxMessageFinder).findsOne();
1481+
check(messageIsntSentErrorFinder).findsOne();
1482+
1483+
// Tap the failed to send message.
1484+
// This should not remove it from the message list.
1485+
await tester.tap(outboxMessageFinder);
1486+
await tester.pump();
1487+
check(outboxMessageFinder).findsOne();
1488+
});
1489+
1490+
testWidgets('tapping does nothing if message is still being sent', (tester) async {
1491+
await setupMessageListPage(tester,
1492+
narrow: eg.topicNarrow(stream.streamId, 'topic'), streams: [stream],
1493+
messages: []);
1494+
final controller = tester.state<ComposeBoxState>(find.byType(ComposeBox)).controller;
1495+
1496+
// Send a message and wait until the debounce timer expires but before
1497+
// the message is successfully sent.
1498+
await sendMessageAndSucceed(tester,
1499+
delay: kLocalEchoDebounceDuration + Duration(seconds: 1));
1500+
await tester.pump(kLocalEchoDebounceDuration);
1501+
check(controller.content).text.isNotNull().isEmpty();
1502+
1503+
await tester.tap(outboxMessageFinder);
1504+
await tester.pump();
1505+
check(outboxMessageFinder).findsOne();
1506+
check(controller.content).text.isNotNull().isEmpty();
1507+
1508+
// Wait till the send request completes. The outbox message should
1509+
// remain visible because the message event didn't arrive.
1510+
await tester.pump(Duration(seconds: 1));
1511+
check(outboxMessageFinder).findsOne();
1512+
check(controller.content).text.isNotNull().isEmpty();
1513+
1514+
// Dispose pending timers from the message store.
1515+
store.dispose();
1516+
});
1517+
1518+
testWidgets('tapping does nothing if message was successfully sent and before message event arrives', (tester) async {
1519+
await setupMessageListPage(tester,
1520+
narrow: eg.topicNarrow(stream.streamId, 'topic'), streams: [stream],
1521+
messages: []);
1522+
final controller = tester.state<ComposeBoxState>(find.byType(ComposeBox)).controller;
1523+
1524+
// Send a message and wait until the debounce timer expires.
1525+
await sendMessageAndSucceed(tester);
1526+
await tester.pump(kLocalEchoDebounceDuration);
1527+
check(controller.content).text.isNotNull().isEmpty();
1528+
1529+
await tester.tap(outboxMessageFinder);
1530+
await tester.pump();
1531+
check(outboxMessageFinder).findsOne();
1532+
check(controller.content).text.isNotNull().isEmpty();
1533+
1534+
// Dispose pending timers from the message store.
1535+
store.dispose();
1536+
});
13871537
});
13881538

13891539
group('Starred messages', () {

0 commit comments

Comments
 (0)