Skip to content

local echo (7/7): Support simplified version of local echo #1453

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions assets/l10n/app_en.arb
Original file line number Diff line number Diff line change
@@ -385,9 +385,9 @@
"@discardDraftForEditConfirmationDialogMessage": {
"description": "Message for a confirmation dialog for discarding message text that was typed into the compose box, when editing a message."
},
"discardDraftForMessageNotSentConfirmationDialogMessage": "When you restore a message not sent, the content that was previously in the compose box is discarded.",
"@discardDraftForMessageNotSentConfirmationDialogMessage": {
"description": "Message for a confirmation dialog when restoring a message not sent, for discarding message text that was typed into the compose box."
"discardDraftForOutboxConfirmationDialogMessage": "When you restore an unsent message, the content that was previously in the compose box is discarded.",
"@discardDraftForOutboxConfirmationDialogMessage": {
"description": "Message for a confirmation dialog when restoring an outbox message, for discarding message text that was typed into the compose box."
},
"discardDraftConfirmationDialogConfirmButton": "Discard",
"@discardDraftConfirmationDialogConfirmButton": {
6 changes: 3 additions & 3 deletions lib/generated/l10n/zulip_localizations.dart
Original file line number Diff line number Diff line change
@@ -655,11 +655,11 @@ abstract class ZulipLocalizations {
/// **'When you edit a message, the content that was previously in the compose box is discarded.'**
String get discardDraftForEditConfirmationDialogMessage;

/// Message for a confirmation dialog when restoring a message not sent, for discarding message text that was typed into the compose box.
/// Message for a confirmation dialog when restoring an outbox message, for discarding message text that was typed into the compose box.
///
/// In en, this message translates to:
/// **'When you restore a message not sent, the content that was previously in the compose box is discarded.'**
String get discardDraftForMessageNotSentConfirmationDialogMessage;
/// **'When you restore an unsent message, the content that was previously in the compose box is discarded.'**
String get discardDraftForOutboxConfirmationDialogMessage;

/// Label for the 'Discard' button on a confirmation dialog for discarding message text that was typed into the compose box.
///
4 changes: 2 additions & 2 deletions lib/generated/l10n/zulip_localizations_ar.dart
Original file line number Diff line number Diff line change
@@ -325,8 +325,8 @@ class ZulipLocalizationsAr extends ZulipLocalizations {
'When you edit a message, the content that was previously in the compose box is discarded.';

@override
String get discardDraftForMessageNotSentConfirmationDialogMessage =>
'When you restore a message not sent, the content that was previously in the compose box is discarded.';
String get discardDraftForOutboxConfirmationDialogMessage =>
'When you restore an unsent message, the content that was previously in the compose box is discarded.';

@override
String get discardDraftConfirmationDialogConfirmButton => 'Discard';
4 changes: 2 additions & 2 deletions lib/generated/l10n/zulip_localizations_de.dart
Original file line number Diff line number Diff line change
@@ -325,8 +325,8 @@ class ZulipLocalizationsDe extends ZulipLocalizations {
'When you edit a message, the content that was previously in the compose box is discarded.';

@override
String get discardDraftForMessageNotSentConfirmationDialogMessage =>
'When you restore a message not sent, the content that was previously in the compose box is discarded.';
String get discardDraftForOutboxConfirmationDialogMessage =>
'When you restore an unsent message, the content that was previously in the compose box is discarded.';

@override
String get discardDraftConfirmationDialogConfirmButton => 'Discard';
4 changes: 2 additions & 2 deletions lib/generated/l10n/zulip_localizations_en.dart
Original file line number Diff line number Diff line change
@@ -325,8 +325,8 @@ class ZulipLocalizationsEn extends ZulipLocalizations {
'When you edit a message, the content that was previously in the compose box is discarded.';

@override
String get discardDraftForMessageNotSentConfirmationDialogMessage =>
'When you restore a message not sent, the content that was previously in the compose box is discarded.';
String get discardDraftForOutboxConfirmationDialogMessage =>
'When you restore an unsent message, the content that was previously in the compose box is discarded.';

@override
String get discardDraftConfirmationDialogConfirmButton => 'Discard';
4 changes: 2 additions & 2 deletions lib/generated/l10n/zulip_localizations_ja.dart
Original file line number Diff line number Diff line change
@@ -325,8 +325,8 @@ class ZulipLocalizationsJa extends ZulipLocalizations {
'When you edit a message, the content that was previously in the compose box is discarded.';

@override
String get discardDraftForMessageNotSentConfirmationDialogMessage =>
'When you restore a message not sent, the content that was previously in the compose box is discarded.';
String get discardDraftForOutboxConfirmationDialogMessage =>
'When you restore an unsent message, the content that was previously in the compose box is discarded.';

@override
String get discardDraftConfirmationDialogConfirmButton => 'Discard';
4 changes: 2 additions & 2 deletions lib/generated/l10n/zulip_localizations_nb.dart
Original file line number Diff line number Diff line change
@@ -325,8 +325,8 @@ class ZulipLocalizationsNb extends ZulipLocalizations {
'When you edit a message, the content that was previously in the compose box is discarded.';

@override
String get discardDraftForMessageNotSentConfirmationDialogMessage =>
'When you restore a message not sent, the content that was previously in the compose box is discarded.';
String get discardDraftForOutboxConfirmationDialogMessage =>
'When you restore an unsent message, the content that was previously in the compose box is discarded.';

@override
String get discardDraftConfirmationDialogConfirmButton => 'Discard';
4 changes: 2 additions & 2 deletions lib/generated/l10n/zulip_localizations_pl.dart
Original file line number Diff line number Diff line change
@@ -332,8 +332,8 @@ class ZulipLocalizationsPl extends ZulipLocalizations {
'Miej na uwadze, że przechodząc do zmiany wiadomości wyczyścisz okno nowej wiadomości.';

@override
String get discardDraftForMessageNotSentConfirmationDialogMessage =>
'When you restore a message not sent, the content that was previously in the compose box is discarded.';
String get discardDraftForOutboxConfirmationDialogMessage =>
'When you restore an unsent message, the content that was previously in the compose box is discarded.';

@override
String get discardDraftConfirmationDialogConfirmButton => 'Odrzuć';
4 changes: 2 additions & 2 deletions lib/generated/l10n/zulip_localizations_ru.dart
Original file line number Diff line number Diff line change
@@ -333,8 +333,8 @@ class ZulipLocalizationsRu extends ZulipLocalizations {
'При изменении сообщения текст из поля для редактирования удаляется.';

@override
String get discardDraftForMessageNotSentConfirmationDialogMessage =>
'When you restore a message not sent, the content that was previously in the compose box is discarded.';
String get discardDraftForOutboxConfirmationDialogMessage =>
'When you restore an unsent message, the content that was previously in the compose box is discarded.';

@override
String get discardDraftConfirmationDialogConfirmButton => 'Сбросить';
4 changes: 2 additions & 2 deletions lib/generated/l10n/zulip_localizations_sk.dart
Original file line number Diff line number Diff line change
@@ -325,8 +325,8 @@ class ZulipLocalizationsSk extends ZulipLocalizations {
'When you edit a message, the content that was previously in the compose box is discarded.';

@override
String get discardDraftForMessageNotSentConfirmationDialogMessage =>
'When you restore a message not sent, the content that was previously in the compose box is discarded.';
String get discardDraftForOutboxConfirmationDialogMessage =>
'When you restore an unsent message, the content that was previously in the compose box is discarded.';

@override
String get discardDraftConfirmationDialogConfirmButton => 'Discard';
4 changes: 2 additions & 2 deletions lib/generated/l10n/zulip_localizations_uk.dart
Original file line number Diff line number Diff line change
@@ -334,8 +334,8 @@ class ZulipLocalizationsUk extends ZulipLocalizations {
'When you edit a message, the content that was previously in the compose box is discarded.';

@override
String get discardDraftForMessageNotSentConfirmationDialogMessage =>
'When you restore a message not sent, the content that was previously in the compose box is discarded.';
String get discardDraftForOutboxConfirmationDialogMessage =>
'When you restore an unsent message, the content that was previously in the compose box is discarded.';

@override
String get discardDraftConfirmationDialogConfirmButton => 'Discard';
4 changes: 2 additions & 2 deletions lib/generated/l10n/zulip_localizations_zh.dart
Original file line number Diff line number Diff line change
@@ -325,8 +325,8 @@ class ZulipLocalizationsZh extends ZulipLocalizations {
'When you edit a message, the content that was previously in the compose box is discarded.';

@override
String get discardDraftForMessageNotSentConfirmationDialogMessage =>
'When you restore a message not sent, the content that was previously in the compose box is discarded.';
String get discardDraftForOutboxConfirmationDialogMessage =>
'When you restore an unsent message, the content that was previously in the compose box is discarded.';

@override
String get discardDraftConfirmationDialogConfirmButton => 'Discard';
7 changes: 4 additions & 3 deletions lib/model/message.dart
Original file line number Diff line number Diff line change
@@ -394,6 +394,8 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore, _OutboxMes
}
}

// TODO predict outbox message moves using propagateMode

for (final view in _messageListViews) {
view.messagesMoved(messageMove: messageMove, messageIds: event.messageIds);
}
@@ -879,9 +881,8 @@ mixin _OutboxMessageStore on PerAccountStoreBase {
void _handleMessageEventOutbox(MessageEvent event) {
if (event.localMessageId != null) {
final localMessageId = int.parse(event.localMessageId!, radix: 10);
// The outbox message can be missing if the user removes it (to be
// implemented in #1441) before the event arrives.
// Nothing to do in that case.
// The outbox message can be missing if the user removes it before the
// event arrives. Nothing to do in that case.
_outboxMessages.remove(localMessageId);
_outboxMessageDebounceTimers.remove(localMessageId)?.cancel();
_outboxMessageWaitPeriodTimers.remove(localMessageId)?.cancel();
244 changes: 224 additions & 20 deletions lib/model/message_list.dart

Large diffs are not rendered by default.

58 changes: 40 additions & 18 deletions lib/widgets/compose_box.dart
Original file line number Diff line number Diff line change
@@ -13,6 +13,7 @@ import '../api/route/messages.dart';
import '../generated/l10n/zulip_localizations.dart';
import '../model/binding.dart';
import '../model/compose.dart';
import '../model/message.dart';
import '../model/narrow.dart';
import '../model/store.dart';
import 'actions.dart';
@@ -1288,15 +1289,8 @@ class _SendButtonState extends State<_SendButton> {
final content = controller.content.textNormalized;

controller.content.clear();
// The following `stoppedComposing` call is currently redundant,
// because clearing input sends a "typing stopped" notice.
// It will be necessary once we resolve #720.
store.typingNotifier.stoppedComposing();

try {
// TODO(#720) clear content input only on success response;
// while waiting, put input(s) and send button into a disabled
// "working on it" state (letting input text be selected for copying).
await store.sendMessage(destination: widget.getDestination(), content: content);
} on ApiRequestException catch (e) {
if (!mounted) return;
@@ -1388,7 +1382,6 @@ class _ComposeBoxContainer extends StatelessWidget {
border: Border(top: BorderSide(color: designVariables.borderBar)),
boxShadow: ComposeBoxTheme.of(context).boxShadow,
),
// TODO(#720) try a Stack for the overlaid linear progress indicator
child: Material(
color: designVariables.composeBoxBg,
child: Column(
@@ -1724,10 +1717,10 @@ class _ErrorBanner extends _Banner {

@override
Widget? buildTrailing(context) {
// TODO(#720) "x" button goes here.
// 24px square with 8px touchable padding in all directions?
// and `bool get padEnd => false`; see Figma:
// https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=4031-17029&m=dev
// An "x" button can go here.
// 24px square with 8px touchable padding in all directions?
// and `bool get padEnd => false`; see Figma:
// https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=4031-17029&m=dev
return null;
}
}
@@ -1826,6 +1819,16 @@ class ComposeBox extends StatefulWidget {
abstract class ComposeBoxState extends State<ComposeBox> {
ComposeBoxController get controller;

/// Fills the compose box with the content of an [OutboxMessage]
/// for a failed [sendMessage] request.
///
/// If there is already text in the compose box, gives a confirmation dialog
/// to confirm that it is OK to discard that text.
///
/// [localMessageId], as in [OutboxMessage.localMessageId], must be present
/// in the message store.
void restoreMessageNotSent(int localMessageId);

/// Switch the compose box to editing mode.
///
/// If there is already text in the compose box, gives a confirmation dialog
@@ -1847,6 +1850,29 @@ class _ComposeBoxState extends State<ComposeBox> with PerAccountStoreAwareStateM
@override ComposeBoxController get controller => _controller!;
ComposeBoxController? _controller;

@override
void restoreMessageNotSent(int localMessageId) async {
final zulipLocalizations = ZulipLocalizations.of(context);

final abort = await _abortBecauseContentInputNotEmpty(
dialogMessage: zulipLocalizations.discardDraftForOutboxConfirmationDialogMessage);
if (abort || !mounted) return;

final store = PerAccountStoreWidget.of(context);
final outboxMessage = store.takeOutboxMessage(localMessageId);
setState(() {
_setNewController(store);
final controller = this.controller;
controller
..content.value = TextEditingValue(text: outboxMessage.contentMarkdown)
..contentFocusNode.requestFocus();
if (controller is StreamComposeBoxController) {
controller.topic.setTopic(
(outboxMessage.conversation as StreamConversation).topic);
}
});
}

@override
void startEditInteraction(int messageId) async {
final zulipLocalizations = ZulipLocalizations.of(context);
@@ -1927,7 +1953,8 @@ class _ComposeBoxState extends State<ComposeBox> with PerAccountStoreAwareStateM
// TODO timeout this request?
if (!mounted) return;
if (!identical(controller, emptyEditController)) {
// user tapped Cancel during the fetch-raw-content request
// During the fetch-raw-content request, the user tapped Cancel
// or tapped a failed message edit or failed outbox message to restore.
// TODO in this case we don't want the error dialog caused by
// ZulipAction.fetchRawContentWithFeedback; suppress that
return;
@@ -2065,11 +2092,6 @@ class _ComposeBoxState extends State<ComposeBox> with PerAccountStoreAwareStateM
}
}

// TODO(#720) dismissable message-send error, maybe something like:
// if (controller.sendMessageError.value != null) {
// errorBanner = _ErrorBanner(label:
// ZulipLocalizations.of(context).errorSendMessageTimeout);
// }
return ComposeBoxInheritedWidget.fromComposeBoxState(this,
child: _ComposeBoxContainer(body: body, banner: banner));
}
128 changes: 128 additions & 0 deletions lib/widgets/message_list.dart
Original file line number Diff line number Diff line change
@@ -5,6 +5,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';
@@ -760,6 +761,9 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat
key: ValueKey(data.message.id),
header: header,
item: data);
case MessageListOutboxMessageItem():
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This implements minimal support to display outbox message message item
widgets in the message list, without indicators for theirs states.
Retrieving content from failed sent requests and the full UI are
implemented in a later commit.

Is there — ah, I guess that was in the commits you temporarily removed as mentioned at #1453 (comment) .

final header = RecipientHeader(message: data.message, narrow: widget.narrow);
return MessageItem(header: header, item: data);
}
}
}
@@ -1071,6 +1075,7 @@ class MessageItem extends StatelessWidget {
child: Column(children: [
switch (item) {
MessageListMessageItem() => MessageWithPossibleSender(item: item),
MessageListOutboxMessageItem() => OutboxMessageWithPossibleSender(item: item),
},
// TODO refine this padding; discussion:
// https://github.com/zulip/zulip-flutter/pull/1453#discussion_r2106526985
@@ -1651,3 +1656,126 @@ class _RestoreEditMessageGestureDetector extends StatelessWidget {
child: child);
}
}

/// A "local echo" placeholder for a Zulip message to be sent by the self-user.
///
/// See also [OutboxMessage].
class OutboxMessageWithPossibleSender extends StatelessWidget {
const OutboxMessageWithPossibleSender({super.key, required this.item});

final MessageListOutboxMessageItem item;

@override
Widget build(BuildContext context) {
final message = item.message;
final localMessageId = message.localMessageId;
final state = item.message.state;

// This is adapted from [MessageContent].
// TODO(#576): Offer InheritedMessage ancestor once we are ready
// to support local echoing images and lightbox.
Widget content = DefaultTextStyle(
style: ContentTheme.of(context).textStylePlainParagraph,
child: BlockContentList(nodes: item.content.nodes));

switch (state) {
case OutboxMessageState.hidden:
throw StateError('Hidden OutboxMessage messages should not appear in message lists');
case OutboxMessageState.waiting:
break;
case OutboxMessageState.failed:
case OutboxMessageState.waitPeriodExpired:
// TODO(#576): When we support rendered-content local echo,
// use IgnorePointer along with this faded appearance,
// like we do for the failed-message-edit state
content = _RestoreOutboxMessageGestureDetector(
localMessageId: localMessageId,
child: Opacity(opacity: 0.6, child: content));
}

return Padding(
padding: const EdgeInsets.only(top: 4),
child: Column(children: [
if (item.showSender)
_SenderRow(message: message, showTimestamp: false),
Padding(
padding: const EdgeInsets.symmetric(horizontal: 16),
child: Column(crossAxisAlignment: CrossAxisAlignment.stretch,
children: [
content,
_OutboxMessageStatusRow(
localMessageId: localMessageId, outboxMessageState: state),
])),
]));
}
}

class _OutboxMessageStatusRow extends StatelessWidget {
const _OutboxMessageStatusRow({
required this.localMessageId,
required this.outboxMessageState,
});

final int localMessageId;
final OutboxMessageState outboxMessageState;

@override
Widget build(BuildContext context) {
switch (outboxMessageState) {
case OutboxMessageState.hidden:
assert(false,
'Hidden OutboxMessage messages should not appear in message lists');
return SizedBox.shrink();

case OutboxMessageState.waiting:
final designVariables = DesignVariables.of(context);
return Padding(
padding: const EdgeInsetsGeometry.only(bottom: 2),
child: LinearProgressIndicator(
minHeight: 2,
color: designVariables.foreground.withFadedAlpha(0.5),
backgroundColor: designVariables.foreground.withFadedAlpha(0.2)));

case OutboxMessageState.failed:
case OutboxMessageState.waitPeriodExpired:
Comment on lines +1739 to +1740
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it intentional to show "MESSAGE NOT SENT" in the waitPeriodExpired state? I think this could cause a confusing symptom like #1525, where the message has actually been sent (or will be) but we say it hasn't.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In v0.0.29 we'd enter this state even if we got a success response from the send request, if the event didn't then show up. I suspect that's what happened in the report in #1525.

In the current revision of this PR, a success response will put the outbox-message into waiting state indefinitely, until the event arrives. I think showing this is OK if we've gotten neither the event nor a response from the request.

(The exact status in that case is more like "tried sending the message, it's been a while, haven't heard any response yet, so it may or may not have made it through, up to you if you want to retry". Not sure there's a good concise way to communicate that.)

final designVariables = DesignVariables.of(context);
final zulipLocalizations = ZulipLocalizations.of(context);
return Padding(
padding: const EdgeInsets.only(bottom: 4),
child: _RestoreOutboxMessageGestureDetector(
localMessageId: localMessageId,
child: Text(
zulipLocalizations.messageNotSentLabel,
textAlign: TextAlign.end,
style: TextStyle(
color: designVariables.btnLabelAttLowIntDanger,
fontSize: 12,
height: 12 / 12,
letterSpacing: proportionalLetterSpacing(
context, 0.05, baseFontSize: 12)))));
}
}
}

class _RestoreOutboxMessageGestureDetector extends StatelessWidget {
const _RestoreOutboxMessageGestureDetector({
required this.localMessageId,
required this.child,
});

final int localMessageId;
final Widget child;

@override
Widget build(BuildContext context) {
return GestureDetector(
behavior: HitTestBehavior.opaque,
onTap: () {
final composeBoxState = MessageListPage.ancestorOf(context).composeBoxState;
// TODO(#1518) allow restore-outbox-message from any message-list page
if (composeBoxState == null) return;
composeBoxState.restoreMessageNotSent(localMessageId);
},
child: child);
}
}
4 changes: 4 additions & 0 deletions test/api/model/model_checks.dart
Original file line number Diff line number Diff line change
@@ -42,6 +42,10 @@ extension StreamConversationChecks on Subject<StreamConversation> {
Subject<String?> get displayRecipient => has((x) => x.displayRecipient, 'displayRecipient');
}

extension DmConversationChecks on Subject<DmConversation> {
Subject<List<int>> get allRecipientIds => has((x) => x.allRecipientIds, 'allRecipientIds');
}

extension MessageBaseChecks<T extends Conversation> on Subject<MessageBase<T>> {
Subject<int?> get id => has((e) => e.id, 'id');
Subject<int> get senderId => has((e) => e.senderId, 'senderId');
612 changes: 595 additions & 17 deletions test/model/message_list_test.dart

Large diffs are not rendered by default.

42 changes: 42 additions & 0 deletions test/model/message_test.dart
Original file line number Diff line number Diff line change
@@ -168,32 +168,40 @@ void main() {
await prepareOutboxMessage(destination: DmDestination(
userIds: [eg.selfUser.userId, eg.otherUser.userId]));
checkState().equals(OutboxMessageState.hidden);
checkNotNotified();

async.elapse(kLocalEchoDebounceDuration);
checkState().equals(OutboxMessageState.waiting);
checkNotifiedOnce();

await receiveMessage(eg.dmMessage(from: eg.selfUser, to: [eg.otherUser]));
check(store.outboxMessages).isEmpty();
checkNotifiedOnce();
}));

test('smoke stream message: hidden -> waiting -> (delete)', () => awaitFakeAsync((async) async {
await prepareOutboxMessage(destination: StreamDestination(
stream.streamId, eg.t('foo')));
checkState().equals(OutboxMessageState.hidden);
checkNotNotified();

async.elapse(kLocalEchoDebounceDuration);
checkState().equals(OutboxMessageState.waiting);
checkNotifiedOnce();

await receiveMessage(eg.streamMessage(stream: stream, topic: 'foo'));
check(store.outboxMessages).isEmpty();
checkNotifiedOnce();
}));

test('hidden -> waiting and never transition to waitPeriodExpired', () => awaitFakeAsync((async) async {
await prepareOutboxMessage();
checkState().equals(OutboxMessageState.hidden);
checkNotNotified();

async.elapse(kLocalEchoDebounceDuration);
checkState().equals(OutboxMessageState.waiting);
checkNotifiedOnce();

// Wait till we reach at least [kSendMessageOfferRestoreWaitPeriod] after
// the send request was initiated.
@@ -203,16 +211,19 @@ void main() {
// The outbox message should stay in the waiting state;
// it should not transition to waitPeriodExpired.
checkState().equals(OutboxMessageState.waiting);
checkNotNotified();
}));

test('waiting -> waitPeriodExpired', () => awaitFakeAsync((async) async {
await prepareOutboxMessageToFailAfterDelay(
kSendMessageOfferRestoreWaitPeriod + Duration(seconds: 1));
async.elapse(kLocalEchoDebounceDuration);
checkState().equals(OutboxMessageState.waiting);
checkNotifiedOnce();

async.elapse(kSendMessageOfferRestoreWaitPeriod - kLocalEchoDebounceDuration);
checkState().equals(OutboxMessageState.waitPeriodExpired);
checkNotifiedOnce();

await check(outboxMessageFailFuture).throws();
}));
@@ -230,10 +241,12 @@ void main() {
destination: streamDestination, content: 'content');
async.elapse(kSendMessageOfferRestoreWaitPeriod);
checkState().equals(OutboxMessageState.waitPeriodExpired);
checkNotified(count: 2);

// Wait till the [sendMessage] request succeeds.
await future;
checkState().equals(OutboxMessageState.waiting);
checkNotifiedOnce();

// Wait till we reach at least [kSendMessageOfferRestoreWaitPeriod] after
// returning to the waiting state.
@@ -242,15 +255,18 @@ void main() {
// The outbox message should stay in the waiting state;
// it should not transition to waitPeriodExpired.
checkState().equals(OutboxMessageState.waiting);
checkNotNotified();
}));

group('… -> failed', () {
test('hidden -> failed', () => awaitFakeAsync((async) async {
await prepareOutboxMessageToFailAfterDelay(Duration.zero);
checkState().equals(OutboxMessageState.hidden);
checkNotNotified();

await check(outboxMessageFailFuture).throws();
checkState().equals(OutboxMessageState.failed);
checkNotifiedOnce();

// Wait till we reach at least [kSendMessageOfferRestoreWaitPeriod] after
// the send request was initiated.
@@ -259,62 +275,74 @@ void main() {
// The outbox message should stay in the failed state;
// it should not transition to waitPeriodExpired.
checkState().equals(OutboxMessageState.failed);
checkNotNotified();
}));

test('waiting -> failed', () => awaitFakeAsync((async) async {
await prepareOutboxMessageToFailAfterDelay(
kLocalEchoDebounceDuration + Duration(seconds: 1));
async.elapse(kLocalEchoDebounceDuration);
checkState().equals(OutboxMessageState.waiting);
checkNotifiedOnce();

await check(outboxMessageFailFuture).throws();
checkState().equals(OutboxMessageState.failed);
checkNotifiedOnce();
}));

test('waitPeriodExpired -> failed', () => awaitFakeAsync((async) async {
await prepareOutboxMessageToFailAfterDelay(
kSendMessageOfferRestoreWaitPeriod + Duration(seconds: 1));
async.elapse(kSendMessageOfferRestoreWaitPeriod);
checkState().equals(OutboxMessageState.waitPeriodExpired);
checkNotified(count: 2);

await check(outboxMessageFailFuture).throws();
checkState().equals(OutboxMessageState.failed);
checkNotifiedOnce();
}));
});

group('… -> (delete)', () {
test('hidden -> (delete) because event received', () => awaitFakeAsync((async) async {
await prepareOutboxMessage();
checkState().equals(OutboxMessageState.hidden);
checkNotNotified();

await receiveMessage();
check(store.outboxMessages).isEmpty();
checkNotifiedOnce();
}));

test('hidden -> (delete) when event arrives before send request fails', () => awaitFakeAsync((async) async {
// Set up an error to fail `sendMessage` with a delay, leaving time for
// the message event to arrive.
await prepareOutboxMessageToFailAfterDelay(const Duration(seconds: 1));
checkState().equals(OutboxMessageState.hidden);
checkNotNotified();

// Received the message event while the message is being sent.
await receiveMessage();
check(store.outboxMessages).isEmpty();
checkNotifiedOnce();

// Complete the send request. There should be no error despite
// the send request failure, because the outbox message is not
// in the store any more.
await check(outboxMessageFailFuture).completes();
async.elapse(const Duration(seconds: 1));
checkNotNotified();
}));

test('waiting -> (delete) because event received', () => awaitFakeAsync((async) async {
await prepareOutboxMessage();
async.elapse(kLocalEchoDebounceDuration);
checkState().equals(OutboxMessageState.waiting);
checkNotifiedOnce();

await receiveMessage();
check(store.outboxMessages).isEmpty();
checkNotifiedOnce();
}));

test('waiting -> (delete) when event arrives before send request fails', () => awaitFakeAsync((async) async {
@@ -324,15 +352,18 @@ void main() {
kLocalEchoDebounceDuration + Duration(seconds: 1));
async.elapse(kLocalEchoDebounceDuration);
checkState().equals(OutboxMessageState.waiting);
checkNotifiedOnce();

// Received the message event while the message is being sent.
await receiveMessage();
check(store.outboxMessages).isEmpty();
checkNotifiedOnce();

// Complete the send request. There should be no error despite
// the send request failure, because the outbox message is not
// in the store any more.
await check(outboxMessageFailFuture).completes();
checkNotNotified();
}));

test('waitPeriodExpired -> (delete) when event arrives before send request fails', () => awaitFakeAsync((async) async {
@@ -342,15 +373,18 @@ void main() {
kSendMessageOfferRestoreWaitPeriod + Duration(seconds: 1));
async.elapse(kSendMessageOfferRestoreWaitPeriod);
checkState().equals(OutboxMessageState.waitPeriodExpired);
checkNotified(count: 2);

// Received the message event while the message is being sent.
await receiveMessage();
check(store.outboxMessages).isEmpty();
checkNotifiedOnce();

// Complete the send request. There should be no error despite
// the send request failure, because the outbox message is not
// in the store any more.
await check(outboxMessageFailFuture).completes();
checkNotNotified();
}));

test('waitPeriodExpired -> (delete) because outbox message was taken', () => awaitFakeAsync((async) async {
@@ -360,27 +394,33 @@ void main() {
kSendMessageOfferRestoreWaitPeriod + Duration(seconds: 1));
async.elapse(kSendMessageOfferRestoreWaitPeriod);
checkState().equals(OutboxMessageState.waitPeriodExpired);
checkNotified(count: 2);

store.takeOutboxMessage(store.outboxMessages.keys.single);
check(store.outboxMessages).isEmpty();
checkNotifiedOnce();
}));

test('failed -> (delete) because event received', () => awaitFakeAsync((async) async {
await prepareOutboxMessageToFailAfterDelay(Duration.zero);
await check(outboxMessageFailFuture).throws();
checkState().equals(OutboxMessageState.failed);
checkNotifiedOnce();

await receiveMessage();
check(store.outboxMessages).isEmpty();
checkNotifiedOnce();
}));

test('failed -> (delete) because outbox message was taken', () => awaitFakeAsync((async) async {
await prepareOutboxMessageToFailAfterDelay(Duration.zero);
await check(outboxMessageFailFuture).throws();
checkState().equals(OutboxMessageState.failed);
checkNotifiedOnce();

store.takeOutboxMessage(store.outboxMessages.keys.single);
check(store.outboxMessages).isEmpty();
checkNotifiedOnce();
}));
});

@@ -422,11 +462,13 @@ void main() {
await check(store.sendMessage(
destination: StreamDestination(stream.streamId, eg.t('topic')),
content: 'content')).throws();
checkNotifiedOnce();
}

final localMessageIds = store.outboxMessages.keys.toList();
store.takeOutboxMessage(localMessageIds.removeAt(5));
check(store.outboxMessages).keys.deepEquals(localMessageIds);
checkNotifiedOnce();
});

group('reconcileMessages', () {
4 changes: 4 additions & 0 deletions test/widgets/compose_box_checks.dart
Original file line number Diff line number Diff line change
@@ -11,6 +11,10 @@ extension ComposeBoxControllerChecks on Subject<ComposeBoxController> {
Subject<FocusNode> get contentFocusNode => has((c) => c.contentFocusNode, 'contentFocusNode');
}

extension StreamComposeBoxControllerChecks on Subject<StreamComposeBoxController> {
Subject<ComposeTopicController> get topic => has((c) => c.topic, 'topic');
}

extension EditMessageComposeBoxControllerChecks on Subject<EditMessageComposeBoxController> {
Subject<int> get messageId => has((c) => c.messageId, 'messageId');
Subject<String?> get originalRawContent => has((c) => c.originalRawContent, 'originalRawContent');
154 changes: 154 additions & 0 deletions test/widgets/compose_box_test.dart
Original file line number Diff line number Diff line change
@@ -1458,6 +1458,160 @@ void main() {
}
}

group('restoreMessageNotSent', () {
final channel = eg.stream();
final topic = 'topic';
final topicNarrow = eg.topicNarrow(channel.streamId, topic);

final failedMessageContent = 'failed message';
final failedMessageFinder = find.widgetWithText(
OutboxMessageWithPossibleSender, failedMessageContent, skipOffstage: true);

Future<void> prepareMessageNotSent(WidgetTester tester, {
required Narrow narrow,
List<User> otherUsers = const [],
}) async {
TypingNotifier.debugEnable = false;
addTearDown(TypingNotifier.debugReset);
await prepareComposeBox(tester,
narrow: narrow, streams: [channel], otherUsers: otherUsers);

if (narrow is ChannelNarrow) {
connection.prepare(json: GetStreamTopicsResult(topics: []).toJson());
await enterTopic(tester, narrow: narrow, topic: topic);
}
await enterContent(tester, failedMessageContent);
connection.prepare(httpException: SocketException('error'));
await tester.tap(find.byIcon(ZulipIcons.send));
await tester.pump(Duration.zero);
check(state).controller.content.text.equals('');

await tester.tap(find.byWidget(checkErrorDialog(tester,
expectedTitle: 'Message not sent')));
await tester.pump();
check(failedMessageFinder).findsOne();
}

testWidgets('restore content in DM narrow', (tester) async {
final dmNarrow = DmNarrow.withUser(
eg.otherUser.userId, selfUserId: eg.selfUser.userId);
await prepareMessageNotSent(tester, narrow: dmNarrow, otherUsers: [eg.otherUser]);

await tester.tap(failedMessageFinder);
await tester.pump();
check(state).controller.content.text.equals(failedMessageContent);
});

testWidgets('restore content in topic narrow', (tester) async {
await prepareMessageNotSent(tester, narrow: topicNarrow);

await tester.tap(failedMessageFinder);
await tester.pump();
check(state).controller.content.text.equals(failedMessageContent);
});

testWidgets('restore content and topic in channel narrow', (tester) async {
final channelNarrow = ChannelNarrow(channel.streamId);
await prepareMessageNotSent(tester, narrow: channelNarrow);

await tester.enterText(topicInputFinder, 'topic before restoring');
check(state).controller.isA<StreamComposeBoxController>()
..topic.text.equals('topic before restoring')
..content.text.isNotNull().isEmpty();

await tester.tap(failedMessageFinder);
await tester.pump();
check(state).controller.isA<StreamComposeBoxController>()
..topic.text.equals(topic)
..content.text.equals(failedMessageContent);
});

Future<void> expectAndHandleDiscardForMessageNotSentConfirmation(
WidgetTester tester, {
required bool shouldContinue,
}) {
return expectAndHandleDiscardConfirmation(tester,
expectedMessage: 'When you restore an unsent message, the content that was previously in the compose box is discarded.',
shouldContinue: shouldContinue);
}

testWidgets('interrupting new-message compose: proceed through confirmation dialog', (tester) async {
await prepareMessageNotSent(tester, narrow: topicNarrow);
await enterContent(tester, 'composing something');

await tester.tap(failedMessageFinder);
await tester.pump();
check(state).controller.content.text.equals('composing something');

await expectAndHandleDiscardForMessageNotSentConfirmation(tester,
shouldContinue: true);
await tester.pump();
check(state).controller.content.text.equals(failedMessageContent);
});

testWidgets('interrupting new-message compose: cancel confirmation dialog', (tester) async {
await prepareMessageNotSent(tester, narrow: topicNarrow);
await enterContent(tester, 'composing something');

await tester.tap(failedMessageFinder);
await tester.pump();
check(state).controller.content.text.equals('composing something');

await expectAndHandleDiscardForMessageNotSentConfirmation(tester,
shouldContinue: false);
await tester.pump();
check(state).controller.content.text.equals('composing something');
});

testWidgets('interrupting message edit: proceed through confirmation dialog', (tester) async {
await prepareMessageNotSent(tester, narrow: topicNarrow);

final messageToEdit = eg.streamMessage(
sender: eg.selfUser, stream: channel, topic: topic,
content: 'message to edit');
await store.addMessage(messageToEdit);
await tester.pump();

await startEditInteractionFromActionSheet(tester, messageId: messageToEdit.id,
originalRawContent: 'message to edit',
delay: Duration.zero);
await tester.pump(const Duration(milliseconds: 250)); // bottom-sheet animation

await tester.tap(failedMessageFinder);
await tester.pump();
check(state).controller.content.text.equals('message to edit');

await expectAndHandleDiscardForMessageNotSentConfirmation(tester,
shouldContinue: true);
await tester.pump();
check(state).controller.content.text.equals(failedMessageContent);
});

testWidgets('interrupting message edit: cancel confirmation dialog', (tester) async {
await prepareMessageNotSent(tester, narrow: topicNarrow);

final messageToEdit = eg.streamMessage(
sender: eg.selfUser, stream: channel, topic: topic,
content: 'message to edit');
await store.addMessage(messageToEdit);
await tester.pump();

await startEditInteractionFromActionSheet(tester, messageId: messageToEdit.id,
originalRawContent: 'message to edit',
delay: Duration.zero);
await tester.pump(const Duration(milliseconds: 250)); // bottom-sheet animation

await tester.tap(failedMessageFinder);
await tester.pump();
check(state).controller.content.text.equals('message to edit');

await expectAndHandleDiscardForMessageNotSentConfirmation(tester,
shouldContinue: false);
await tester.pump();
check(state).controller.content.text.equals('message to edit');
});
});

group('edit message', () {
final channel = eg.stream();
final topic = 'topic';
159 changes: 159 additions & 0 deletions test/widgets/message_list_test.dart
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import 'dart:convert';
import 'dart:io';

import 'package:checks/checks.dart';
import 'package:collection/collection.dart';
@@ -15,6 +16,7 @@ import 'package:zulip/api/route/channels.dart';
import 'package:zulip/api/route/messages.dart';
import 'package:zulip/model/actions.dart';
import 'package:zulip/model/localizations.dart';
import 'package:zulip/model/message.dart';
import 'package:zulip/model/message_list.dart';
import 'package:zulip/model/narrow.dart';
import 'package:zulip/model/store.dart';
@@ -1551,6 +1553,163 @@ void main() {
});
});

group('OutboxMessageWithPossibleSender', () {
final stream = eg.stream();
final topic = 'topic';
final topicNarrow = eg.topicNarrow(stream.streamId, topic);
const content = 'outbox message content';

final contentInputFinder = find.byWidgetPredicate(
(widget) => widget is TextField && widget.controller is ComposeContentController);

Finder outboxMessageFinder = find.widgetWithText(
OutboxMessageWithPossibleSender, content, skipOffstage: true);

Finder messageNotSentFinder = find.descendant(
of: find.byType(OutboxMessageWithPossibleSender),
matching: find.text('MESSAGE NOT SENT')).hitTestable();
Finder loadingIndicatorFinder = find.descendant(
of: find.byType(OutboxMessageWithPossibleSender),
matching: find.byType(LinearProgressIndicator)).hitTestable();

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, {
Duration delay = Duration.zero,
}) async {
connection.prepare(httpException: SocketException('error'), delay: delay);
await tester.enterText(contentInputFinder, content);
await tester.tap(find.byIcon(ZulipIcons.send));
await tester.pump(Duration.zero);
}

Future<void> dismissErrorDialog(WidgetTester tester) async {
await tester.tap(find.byWidget(
checkErrorDialog(tester, expectedTitle: 'Message not sent')));
await tester.pump(Duration(milliseconds: 250));
}

Future<void> checkTapRestoreMessage(WidgetTester tester) async {
final state = tester.state<ComposeBoxState>(find.byType(ComposeBox));
check(store.outboxMessages).values.single;
check(outboxMessageFinder).findsOne();
check(messageNotSentFinder).findsOne();
check(state).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(store.outboxMessages).isEmpty();
check(outboxMessageFinder).findsNothing();
check(state).controller.content.text.equals(content);
}

Future<void> checkTapNotRestoreMessage(WidgetTester tester) async {
check(store.outboxMessages).values.single;
check(outboxMessageFinder).findsOne();

// the message should ignore the pointer event
await tester.tap(outboxMessageFinder, warnIfMissed: false);
await tester.pump();
check(store.outboxMessages).values.single;
check(outboxMessageFinder).findsOne();
}

// State transitions are tested more thoroughly in
// test/model/message_test.dart .

testWidgets('hidden -> waiting', (tester) async {
await setupMessageListPage(tester,
narrow: topicNarrow, streams: [stream],
messages: []);

await sendMessageAndSucceed(tester);
check(outboxMessageFinder).findsNothing();

await tester.pump(kLocalEchoDebounceDuration);
check(outboxMessageFinder).findsOne();
check(loadingIndicatorFinder).findsOne();
// The outbox message is still in waiting state;
// tapping does not restore it.
await checkTapNotRestoreMessage(tester);
});

testWidgets('hidden -> failed, tapping does nothing if compose box is not offered', (tester) async {
final messages = [eg.streamMessage(
stream: stream, topic: topic, content: content)];
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.widgetWithText(RecipientHeader, topic));
await tester.pump(); // handle tap
await tester.pump(); // wait for navigation
check(contentInputFinder).findsOne();

await sendMessageAndFail(tester);
await dismissErrorDialog(tester);
// Navigate back to the message list page without a compose box,
// where the failed to send message should be visible.
await tester.pageBack();
await tester.pump(); // handle tap
await tester.pump(const Duration(milliseconds: 500)); // wait for navigation
check(contentInputFinder).findsNothing();
check(messageNotSentFinder).findsOne();

// Tap the failed to send message.
// This should not remove it from the message list.
await checkTapNotRestoreMessage(tester);
});

testWidgets('hidden -> failed, tap to restore message', (tester) async {
await setupMessageListPage(tester,
narrow: topicNarrow, streams: [stream],
messages: []);
// Send a message and fail. Dismiss the error dialog as it pops up.
await sendMessageAndFail(tester);
await dismissErrorDialog(tester);
check(messageNotSentFinder).findsOne();

await checkTapRestoreMessage(tester);
});

testWidgets('waiting -> waitPeriodExpired, tap to restore message', (tester) async {
await setupMessageListPage(tester,
narrow: topicNarrow, streams: [stream],
messages: []);
await sendMessageAndFail(tester,
delay: kSendMessageOfferRestoreWaitPeriod + const Duration(seconds: 1));
await tester.pump(kSendMessageOfferRestoreWaitPeriod);
final localMessageId = store.outboxMessages.keys.single;
check(messageNotSentFinder).findsOne();

await checkTapRestoreMessage(tester);

// While `localMessageId` is no longer in store, there should be no error
// when a message event refers to it.
await store.handleEvent(eg.messageEvent(
eg.streamMessage(stream: stream, topic: 'topic'),
localMessageId: localMessageId));

// The [sendMessage] request fails; there is no outbox message affected.
await tester.pump(Duration(seconds: 1));
check(messageNotSentFinder).findsNothing();
});
});

group('Starred messages', () {
testWidgets('unstarred message', (tester) async {
final message = eg.streamMessage(flags: []);