Skip to content

Commit 66614ca

Browse files
committed
squash! implement retry UX
See if the tests can be simplified
1 parent 5f76af5 commit 66614ca

11 files changed

+219
-17
lines changed

Diff for: assets/l10n/app_en.arb

+4
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.",

Diff for: lib/generated/l10n/zulip_localizations.dart

+6
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:

Diff for: lib/generated/l10n/zulip_localizations_ar.dart

+3
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)';

Diff for: lib/generated/l10n/zulip_localizations_en.dart

+3
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)';

Diff for: lib/generated/l10n/zulip_localizations_ja.dart

+3
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)';

Diff for: lib/generated/l10n/zulip_localizations_nb.dart

+3
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)';

Diff for: lib/generated/l10n/zulip_localizations_pl.dart

+3
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)';

Diff for: lib/generated/l10n/zulip_localizations_ru.dart

+3
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)';

Diff for: lib/generated/l10n/zulip_localizations_sk.dart

+3
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)';

Diff for: lib/widgets/message_list.dart

+60-13
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';
@@ -1481,21 +1482,67 @@ class OutboxMessageWithPossibleSender extends StatelessWidget {
14811482

14821483
final MessageListOutboxMessageItem item;
14831484

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

Diff for: test/widgets/message_list_test.dart

+128-4
Original file line numberDiff line numberDiff line change
@@ -1359,9 +1359,12 @@ void main() {
13591359
final contentInputFinder = find.byWidgetPredicate(
13601360
(widget) => widget is TextField && widget.controller is ComposeContentController);
13611361

1362-
Finder findTextInMessages(String text) => find.descendant(
1362+
Finder outboxMessageFinder = find.descendant(
13631363
of: find.byType(MessageItem),
1364-
matching: find.text(content, findRichText: true));
1364+
matching: find.text(content, findRichText: true)).hitTestable();
1365+
1366+
Finder messageIsntSentErrorFinder = find.text(
1367+
'MESSAGE ISN\'T SENT. CHECK YOUR CONNECTION.').hitTestable();
13651368

13661369
testWidgets('sent message appear in message list after debounce timeout', (tester) async {
13671370
await setupMessageListPage(tester,
@@ -1372,10 +1375,131 @@ void main() {
13721375
await tester.enterText(contentInputFinder, content);
13731376
await tester.tap(find.byIcon(ZulipIcons.send));
13741377
await tester.pump(Duration.zero);
1375-
check(findTextInMessages(content)).findsNothing();
1378+
check(outboxMessageFinder).findsNothing();
13761379

13771380
await tester.pump(kLocalEchoDebounceDuration);
1378-
check(findTextInMessages(content)).findsOne();
1381+
check(outboxMessageFinder).findsOne();
1382+
check(find.descendant(
1383+
of: find.byType(MessageItem),
1384+
matching: find.byType(LinearProgressIndicator))).findsOne();
1385+
});
1386+
1387+
testWidgets('failed to send message, retrieve the content to compose box', (tester) async {
1388+
await setupMessageListPage(tester,
1389+
narrow: eg.topicNarrow(stream.streamId, 'topic'), streams: [stream],
1390+
messages: []);
1391+
1392+
// Send a message and fail. Dismiss the error dialog as it pops up.
1393+
connection.prepare(apiException: eg.apiBadRequest());
1394+
await tester.enterText(contentInputFinder, content);
1395+
await tester.tap(find.byIcon(ZulipIcons.send));
1396+
await tester.pump(Duration.zero);
1397+
await tester.tap(find.byWidget(
1398+
checkErrorDialog(tester, expectedTitle: 'Message not sent')));
1399+
await tester.pump();
1400+
check(outboxMessageFinder).findsOne();
1401+
check(messageIsntSentErrorFinder).findsOne();
1402+
1403+
final controller = tester.state<ComposeBoxState>(find.byType(ComposeBox)).controller;
1404+
check(controller.content).text.isNotNull().isEmpty();
1405+
1406+
// Tap the message. This should put its content back into the compose box
1407+
// and remove it.
1408+
await tester.tap(outboxMessageFinder);
1409+
await tester.pump();
1410+
check(outboxMessageFinder).findsNothing();
1411+
check(controller.content).text.equals('$content\n\n');
1412+
1413+
await tester.pump(kLocalEchoDebounceDuration);
1414+
});
1415+
1416+
testWidgets('tapping does nothing if compose box is not offered', (tester) async {
1417+
final messages = [eg.streamMessage(stream: stream, topic: 'some topic')];
1418+
await setupMessageListPage(tester,
1419+
narrow: const CombinedFeedNarrow(), streams: [stream], subscriptions: [eg.subscription(stream)],
1420+
messages: messages);
1421+
1422+
// Navigate to a message list page in a topic narrow,
1423+
// which has a compose box.
1424+
connection.prepare(json:
1425+
eg.newestGetMessagesResult(foundOldest: true, messages: messages).toJson());
1426+
await tester.tap(find.text('some topic'));
1427+
await tester.pump(); // handle tap
1428+
await tester.pump(); // wait for navigation
1429+
1430+
// Send a message and fail. Dismiss the error dialog as it pops up.
1431+
connection.prepare(apiException: eg.apiBadRequest());
1432+
await tester.enterText(contentInputFinder, content);
1433+
await tester.tap(find.byIcon(ZulipIcons.send));
1434+
await tester.pump(Duration.zero);
1435+
await tester.tap(find.byWidget(
1436+
checkErrorDialog(tester, expectedTitle: 'Message not sent')));
1437+
await tester.pump(); // handle tap
1438+
check(outboxMessageFinder).findsOne();
1439+
check(messageIsntSentErrorFinder).findsOne();
1440+
1441+
// Navigate back to the message list page without a compose box,
1442+
// where the failed to send message should still be visible.
1443+
await tester.pageBack();
1444+
await tester.pump(); // handle tap
1445+
await tester.pump(); // wait for navigation
1446+
check(contentInputFinder).findsNothing();
1447+
check(outboxMessageFinder).findsOne();
1448+
check(messageIsntSentErrorFinder).findsOne();
1449+
1450+
// Tap the failed to send message.
1451+
// This should not remove it from the message list.
1452+
await tester.tap(outboxMessageFinder);
1453+
await tester.pump();
1454+
check(outboxMessageFinder).findsOne();
1455+
});
1456+
1457+
testWidgets('tapping does nothing if message was successfully sent', (tester) async {
1458+
await setupMessageListPage(tester,
1459+
narrow: eg.topicNarrow(stream.streamId, 'topic'), streams: [stream],
1460+
messages: []);
1461+
final controller = tester.state<ComposeBoxState>(find.byType(ComposeBox)).controller;
1462+
1463+
// Send a message and wait until the debounce timer expires.
1464+
connection.prepare(json: SendMessageResult(id: 1).toJson());
1465+
await tester.enterText(contentInputFinder, content);
1466+
await tester.tap(find.byIcon(ZulipIcons.send));
1467+
await tester.pump(Duration.zero);
1468+
await tester.pump(kLocalEchoDebounceDuration);
1469+
check(controller.content).text.isNotNull().isEmpty();
1470+
1471+
await tester.tap(outboxMessageFinder);
1472+
await tester.pump();
1473+
check(outboxMessageFinder).findsOne();
1474+
check(controller.content).text.isNotNull().isEmpty();
1475+
});
1476+
1477+
testWidgets('tapping does nothing if message is still being sent', (tester) async {
1478+
await setupMessageListPage(tester,
1479+
narrow: eg.topicNarrow(stream.streamId, 'topic'), streams: [stream],
1480+
messages: []);
1481+
final controller = tester.state<ComposeBoxState>(find.byType(ComposeBox)).controller;
1482+
1483+
// Send a message and wait until the debounce timer expires but before
1484+
// the message is successfully sent.
1485+
connection.prepare(json: SendMessageResult(id: 1).toJson(),
1486+
delay: kLocalEchoDebounceDuration + Duration(seconds: 1));
1487+
await tester.enterText(contentInputFinder, content);
1488+
await tester.tap(find.byIcon(ZulipIcons.send));
1489+
await tester.pump(Duration.zero);
1490+
await tester.pump(kLocalEchoDebounceDuration);
1491+
check(controller.content).text.isNotNull().isEmpty();
1492+
1493+
await tester.tap(outboxMessageFinder);
1494+
await tester.pump();
1495+
check(outboxMessageFinder).findsOne();
1496+
check(controller.content).text.isNotNull().isEmpty();
1497+
1498+
// Wait till the send request completes. The outbox message should
1499+
// remain visible because the message event didn't arrive.
1500+
await tester.pump(Duration(seconds: 1));
1501+
check(outboxMessageFinder).findsOne();
1502+
check(controller.content).text.isNotNull().isEmpty();
13791503
});
13801504
});
13811505

0 commit comments

Comments
 (0)