From 779fb4ec2ed80ab37437cb761253f78640860db0 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Mon, 4 Nov 2024 16:32:57 -0800 Subject: [PATCH 01/10] action_sheet [nfc]: Factor out findMessageListPage on button widget --- lib/widgets/action_sheet.dart | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/lib/widgets/action_sheet.dart b/lib/widgets/action_sheet.dart index d06a57995f..df5a769de7 100644 --- a/lib/widgets/action_sheet.dart +++ b/lib/widgets/action_sheet.dart @@ -13,7 +13,6 @@ import '../model/internal_link.dart'; import '../model/narrow.dart'; import 'actions.dart'; import 'clipboard.dart'; -import 'compose_box.dart'; import 'dialog.dart'; import 'icons.dart'; import 'inset_shadow.dart'; @@ -105,6 +104,16 @@ abstract class MessageActionSheetMenuItemButton extends StatelessWidget { final Message message; final BuildContext messageListContext; + /// The [MessageListPageState] this action sheet was triggered from. + /// + /// Uses the inefficient [BuildContext.findAncestorStateOfType]; + /// don't call this in a build method. + MessageListPageState findMessageListPage() { + assert(messageListContext.mounted, + 'findMessageListPage should be called only when messageListContext is known to still be mounted'); + return MessageListPage.ancestorOf(messageListContext); + } + @override Widget build(BuildContext context) { final designVariables = DesignVariables.of(context); @@ -315,8 +324,7 @@ class QuoteAndReplyButton extends MessageActionSheetMenuItemButton { // This will be null only if the compose box disappeared after the // message action sheet opened, and before "Quote and reply" was pressed. // Currently a compose box can't ever disappear, so this is impossible. - ComposeBoxController composeBoxController = - MessageListPage.ancestorOf(messageListContext).composeBoxController!; + var composeBoxController = findMessageListPage().composeBoxController!; final topicController = composeBoxController.topicController; if ( topicController != null @@ -341,8 +349,7 @@ class QuoteAndReplyButton extends MessageActionSheetMenuItemButton { // This will be null only if the compose box disappeared during the // quotation request. Currently a compose box can't ever disappear, // so this is impossible. - composeBoxController = - MessageListPage.ancestorOf(messageListContext).composeBoxController!; + composeBoxController = findMessageListPage().composeBoxController!; composeBoxController.contentController .registerQuoteAndReplyEnd(PerAccountStoreWidget.of(messageListContext), tag, message: message, From 6b127bf627b10c187f0c05dbfaf9c76c79005a37 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Mon, 4 Nov 2024 16:34:59 -0800 Subject: [PATCH 02/10] action_sheet [nfc]: Rename pageContext field on button widgets Most of the ways we use this field (specifically, all the ways we use it other than in findMessageListPage) aren't related to it being the message list in particular. When we add action sheets other than the message action sheet, they'll still need a page context. So prepare the way for those by giving this a name that can generalize. --- lib/widgets/action_sheet.dart | 91 ++++++++++++++++++----------------- 1 file changed, 47 insertions(+), 44 deletions(-) diff --git a/lib/widgets/action_sheet.dart b/lib/widgets/action_sheet.dart index df5a769de7..f7a63546d0 100644 --- a/lib/widgets/action_sheet.dart +++ b/lib/widgets/action_sheet.dart @@ -47,15 +47,15 @@ void showMessageActionSheet({required BuildContext context, required Message mes final optionButtons = [ if (!hasThumbsUpReactionVote) - AddThumbsUpButton(message: message, messageListContext: context), - StarButton(message: message, messageListContext: context), + AddThumbsUpButton(message: message, pageContext: context), + StarButton(message: message, pageContext: context), if (isComposeBoxOffered) - QuoteAndReplyButton(message: message, messageListContext: context), + QuoteAndReplyButton(message: message, pageContext: context), if (showMarkAsUnreadButton) - MarkAsUnreadButton(message: message, messageListContext: context, narrow: narrow), - CopyMessageTextButton(message: message, messageListContext: context), - CopyMessageLinkButton(message: message, messageListContext: context), - ShareButton(message: message, messageListContext: context), + MarkAsUnreadButton(message: message, pageContext: context, narrow: narrow), + CopyMessageTextButton(message: message, pageContext: context), + CopyMessageLinkButton(message: message, pageContext: context), + ShareButton(message: message, pageContext: context), ]; showModalBottomSheet( @@ -94,24 +94,27 @@ abstract class MessageActionSheetMenuItemButton extends StatelessWidget { MessageActionSheetMenuItemButton({ super.key, required this.message, - required this.messageListContext, - }) : assert(messageListContext.findAncestorWidgetOfExactType() != null); + required this.pageContext, + }) : assert(pageContext.findAncestorWidgetOfExactType() != null); IconData get icon; String label(ZulipLocalizations zulipLocalizations); void onPressed(BuildContext context); final Message message; - final BuildContext messageListContext; + + /// A context within the [MessageListPage] this action sheet was + /// triggered from. + final BuildContext pageContext; /// The [MessageListPageState] this action sheet was triggered from. /// /// Uses the inefficient [BuildContext.findAncestorStateOfType]; /// don't call this in a build method. MessageListPageState findMessageListPage() { - assert(messageListContext.mounted, - 'findMessageListPage should be called only when messageListContext is known to still be mounted'); - return MessageListPage.ancestorOf(messageListContext); + assert(pageContext.mounted, + 'findMessageListPage should be called only when pageContext is known to still be mounted'); + return MessageListPage.ancestorOf(pageContext); } @override @@ -166,7 +169,7 @@ class AddThumbsUpButton extends MessageActionSheetMenuItemButton { AddThumbsUpButton({ super.key, required super.message, - required super.messageListContext, + required super.pageContext, }); @override IconData get icon => ZulipIcons.smile; @@ -180,14 +183,14 @@ class AddThumbsUpButton extends MessageActionSheetMenuItemButton { Navigator.of(context).pop(); String? errorMessage; try { - await addReaction(PerAccountStoreWidget.of(messageListContext).connection, + await addReaction(PerAccountStoreWidget.of(pageContext).connection, messageId: message.id, reactionType: ReactionType.unicodeEmoji, emojiCode: '1f44d', emojiName: '+1', ); } catch (e) { - if (!messageListContext.mounted) return; + if (!pageContext.mounted) return; switch (e) { case ZulipApiException(): @@ -207,7 +210,7 @@ class StarButton extends MessageActionSheetMenuItemButton { StarButton({ super.key, required super.message, - required super.messageListContext, + required super.pageContext, }); @override IconData get icon => _isStarred ? ZulipIcons.star_filled : ZulipIcons.star; @@ -223,17 +226,17 @@ class StarButton extends MessageActionSheetMenuItemButton { @override void onPressed(BuildContext context) async { Navigator.of(context).pop(); - final zulipLocalizations = ZulipLocalizations.of(messageListContext); + final zulipLocalizations = ZulipLocalizations.of(pageContext); final op = message.flags.contains(MessageFlag.starred) ? UpdateMessageFlagsOp.remove : UpdateMessageFlagsOp.add; try { - final connection = PerAccountStoreWidget.of(messageListContext).connection; + final connection = PerAccountStoreWidget.of(pageContext).connection; await updateMessageFlags(connection, messages: [message.id], op: op, flag: MessageFlag.starred); } catch (e) { - if (!messageListContext.mounted) return; + if (!pageContext.mounted) return; String? errorMessage; switch (e) { @@ -244,7 +247,7 @@ class StarButton extends MessageActionSheetMenuItemButton { default: } - showErrorDialog(context: messageListContext, + showErrorDialog(context: pageContext, title: switch(op) { UpdateMessageFlagsOp.remove => zulipLocalizations.errorUnstarMessageFailedTitle, UpdateMessageFlagsOp.add => zulipLocalizations.errorStarMessageFailedTitle, @@ -305,7 +308,7 @@ class QuoteAndReplyButton extends MessageActionSheetMenuItemButton { QuoteAndReplyButton({ super.key, required super.message, - required super.messageListContext, + required super.pageContext, }); @override IconData get icon => ZulipIcons.format_quote; @@ -319,7 +322,7 @@ class QuoteAndReplyButton extends MessageActionSheetMenuItemButton { // Close the message action sheet. We'll show the request progress // in the compose-box content input with a "[Quoting…]" placeholder. Navigator.of(context).pop(); - final zulipLocalizations = ZulipLocalizations.of(messageListContext); + final zulipLocalizations = ZulipLocalizations.of(pageContext); // This will be null only if the compose box disappeared after the // message action sheet opened, and before "Quote and reply" was pressed. @@ -334,24 +337,24 @@ class QuoteAndReplyButton extends MessageActionSheetMenuItemButton { topicController.value = TextEditingValue(text: message.topic); } final tag = composeBoxController.contentController - .registerQuoteAndReplyStart(PerAccountStoreWidget.of(messageListContext), + .registerQuoteAndReplyStart(PerAccountStoreWidget.of(pageContext), message: message, ); final rawContent = await fetchRawContentWithFeedback( - context: messageListContext, + context: pageContext, messageId: message.id, errorDialogTitle: zulipLocalizations.errorQuotationFailed, ); - if (!messageListContext.mounted) return; + if (!pageContext.mounted) return; // This will be null only if the compose box disappeared during the // quotation request. Currently a compose box can't ever disappear, // so this is impossible. composeBoxController = findMessageListPage().composeBoxController!; composeBoxController.contentController - .registerQuoteAndReplyEnd(PerAccountStoreWidget.of(messageListContext), tag, + .registerQuoteAndReplyEnd(PerAccountStoreWidget.of(pageContext), tag, message: message, rawContent: rawContent, ); @@ -365,7 +368,7 @@ class MarkAsUnreadButton extends MessageActionSheetMenuItemButton { MarkAsUnreadButton({ super.key, required super.message, - required super.messageListContext, + required super.pageContext, required this.narrow, }); @@ -380,7 +383,7 @@ class MarkAsUnreadButton extends MessageActionSheetMenuItemButton { @override void onPressed(BuildContext context) async { Navigator.of(context).pop(); - unawaited(markNarrowAsUnreadFromMessage(messageListContext, message, narrow)); + unawaited(markNarrowAsUnreadFromMessage(pageContext, message, narrow)); } } @@ -388,7 +391,7 @@ class CopyMessageTextButton extends MessageActionSheetMenuItemButton { CopyMessageTextButton({ super.key, required super.message, - required super.messageListContext, + required super.pageContext, }); @override IconData get icon => ZulipIcons.copy; @@ -403,19 +406,19 @@ class CopyMessageTextButton extends MessageActionSheetMenuItemButton { // but hopefully it won't take long at all, and // fetchRawContentWithFeedback has a TODO for giving feedback if it does. Navigator.of(context).pop(); - final zulipLocalizations = ZulipLocalizations.of(messageListContext); + final zulipLocalizations = ZulipLocalizations.of(pageContext); final rawContent = await fetchRawContentWithFeedback( - context: messageListContext, + context: pageContext, messageId: message.id, errorDialogTitle: zulipLocalizations.errorCopyingFailed, ); if (rawContent == null) return; - if (!messageListContext.mounted) return; + if (!pageContext.mounted) return; - copyWithPopup(context: messageListContext, + copyWithPopup(context: pageContext, successContent: Text(zulipLocalizations.successMessageTextCopied), data: ClipboardData(text: rawContent)); } @@ -425,7 +428,7 @@ class CopyMessageLinkButton extends MessageActionSheetMenuItemButton { CopyMessageLinkButton({ super.key, required super.message, - required super.messageListContext, + required super.pageContext, }); @override IconData get icon => Icons.link; @@ -437,16 +440,16 @@ class CopyMessageLinkButton extends MessageActionSheetMenuItemButton { @override void onPressed(BuildContext context) { Navigator.of(context).pop(); - final zulipLocalizations = ZulipLocalizations.of(messageListContext); + final zulipLocalizations = ZulipLocalizations.of(pageContext); - final store = PerAccountStoreWidget.of(messageListContext); + final store = PerAccountStoreWidget.of(pageContext); final messageLink = narrowLink( store, SendableNarrow.ofMessage(message, selfUserId: store.selfUserId), nearMessageId: message.id, ); - copyWithPopup(context: messageListContext, + copyWithPopup(context: pageContext, successContent: Text(zulipLocalizations.successMessageLinkCopied), data: ClipboardData(text: messageLink.toString())); } @@ -456,7 +459,7 @@ class ShareButton extends MessageActionSheetMenuItemButton { ShareButton({ super.key, required super.message, - required super.messageListContext, + required super.pageContext, }); @override @@ -479,17 +482,17 @@ class ShareButton extends MessageActionSheetMenuItemButton { // `showMessageActionSheet` call) and cover a large part of the // share sheet. Navigator.of(context).pop(); - final zulipLocalizations = ZulipLocalizations.of(messageListContext); + final zulipLocalizations = ZulipLocalizations.of(pageContext); final rawContent = await fetchRawContentWithFeedback( - context: messageListContext, + context: pageContext, messageId: message.id, errorDialogTitle: zulipLocalizations.errorSharingFailed, ); if (rawContent == null) return; - if (!messageListContext.mounted) return; + if (!pageContext.mounted) return; // TODO: to support iPads, we're asked to give a // `sharePositionOrigin` param, or risk crashing / hanging: @@ -502,8 +505,8 @@ class ShareButton extends MessageActionSheetMenuItemButton { // The plugin isn't very helpful: "The status can not be determined". // Until we learn otherwise, assume something wrong happened. case ShareResultStatus.unavailable: - if (!messageListContext.mounted) return; - showErrorDialog(context: messageListContext, + if (!pageContext.mounted) return; + showErrorDialog(context: pageContext, title: zulipLocalizations.errorSharingFailed); case ShareResultStatus.success: case ShareResultStatus.dismissed: From 3f72ef362b96550e6aec92106272ed3746d2e1eb Mon Sep 17 00:00:00 2001 From: Greg Price Date: Mon, 4 Nov 2024 16:44:44 -0800 Subject: [PATCH 03/10] action_sheet: Fix straggler use of potentially-dead sheet context And add a bit of docs clarifying the expected practice. --- lib/widgets/action_sheet.dart | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/widgets/action_sheet.dart b/lib/widgets/action_sheet.dart index f7a63546d0..6234b7b14f 100644 --- a/lib/widgets/action_sheet.dart +++ b/lib/widgets/action_sheet.dart @@ -99,6 +99,13 @@ abstract class MessageActionSheetMenuItemButton extends StatelessWidget { IconData get icon; String label(ZulipLocalizations zulipLocalizations); + + /// Called when the button is pressed. + /// + /// Generally this method's implementation should begin by dismissing the + /// action sheet with [NavigatorState.pop]. + /// After that step, the given `context` may no longer be mounted; + /// operations that need a [BuildContext] should typically use [pageContext]. void onPressed(BuildContext context); final Message message; @@ -200,7 +207,7 @@ class AddThumbsUpButton extends MessageActionSheetMenuItemButton { default: } - showErrorDialog(context: context, + showErrorDialog(context: pageContext, title: 'Adding reaction failed', message: errorMessage); } } From 0c5e6550e669ac2c057439f1a6e0d0bf4fb186be Mon Sep 17 00:00:00 2001 From: Greg Price Date: Mon, 4 Nov 2024 16:49:01 -0800 Subject: [PATCH 04/10] action_sheet [nfc]: Update TODO comment about keyboard vs. iOS share sheet We closed issue #24 because it was apparently resolved by an OS update. There's still a milder issue; mention that instead. --- lib/widgets/action_sheet.dart | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/widgets/action_sheet.dart b/lib/widgets/action_sheet.dart index 6234b7b14f..9b30ae54c1 100644 --- a/lib/widgets/action_sheet.dart +++ b/lib/widgets/action_sheet.dart @@ -484,10 +484,9 @@ class ShareButton extends MessageActionSheetMenuItemButton { // sheet. (We could do this after the sharing Future settles // with [ShareResultStatus.success], but on iOS I get impatient with // how slowly our action sheet dismisses in that case.) - // TODO(#24): Fix iOS bug where this call causes the keyboard to - // reopen (if it was open at the time of this - // `showMessageActionSheet` call) and cover a large part of the - // share sheet. + // TODO(#591): Fix iOS bug where if the keyboard was open before the call + // to `showMessageActionSheet`, it reappears briefly between + // the `pop` of the action sheet and the appearance of the share sheet. Navigator.of(context).pop(); final zulipLocalizations = ZulipLocalizations.of(pageContext); From 9b18810ae4a3a6773a54f5fa3412db1658b91612 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Mon, 4 Nov 2024 17:00:13 -0800 Subject: [PATCH 05/10] action_sheet [nfc]: Avoid passing footgun context to onPressed methods The usual Flutter idiom, which we use across the rest of the app, would call for using the parameter called `context` wherever a BuildContext is required: for getting the store or localizations, or passing to a method like showErrorDialog, and so on. That idiom doesn't work here, because that context belongs to the action sheet itself, which gets dismissed at the start of the method. Worse, it will sometimes seem to work, nondeterministically: the context only gets unmounted after the animation to dismiss the action sheet is complete, which takes something like 200ms. So the temptation to use it has been a recurring source of small bugs. Fix the problem by not passing the action sheet's context to these methods at all. --- lib/widgets/action_sheet.dart | 64 +++++++++++++++++++---------------- 1 file changed, 35 insertions(+), 29 deletions(-) diff --git a/lib/widgets/action_sheet.dart b/lib/widgets/action_sheet.dart index 9b30ae54c1..3b448413f3 100644 --- a/lib/widgets/action_sheet.dart +++ b/lib/widgets/action_sheet.dart @@ -100,13 +100,13 @@ abstract class MessageActionSheetMenuItemButton extends StatelessWidget { IconData get icon; String label(ZulipLocalizations zulipLocalizations); - /// Called when the button is pressed. + /// Called when the button is pressed, after dismissing the action sheet. /// - /// Generally this method's implementation should begin by dismissing the - /// action sheet with [NavigatorState.pop]. - /// After that step, the given `context` may no longer be mounted; - /// operations that need a [BuildContext] should typically use [pageContext]. - void onPressed(BuildContext context); + /// If the action may take a long time, this method is responsible for + /// arranging any form of progress feedback that may be desired. + /// + /// For operations that need a [BuildContext], see [pageContext]. + void onPressed(); final Message message; @@ -124,6 +124,15 @@ abstract class MessageActionSheetMenuItemButton extends StatelessWidget { return MessageListPage.ancestorOf(pageContext); } + void _handlePressed(BuildContext context) { + // Dismiss the enclosing action sheet immediately, + // for swift UI feedback that the user's selection was received. + Navigator.of(context).pop(); + + assert(pageContext.mounted); + onPressed(); + } + @override Widget build(BuildContext context) { final designVariables = DesignVariables.of(context); @@ -137,7 +146,7 @@ abstract class MessageActionSheetMenuItemButton extends StatelessWidget { ).copyWith(backgroundColor: WidgetStateColor.resolveWith((states) => designVariables.contextMenuItemBg.withValues( alpha: states.contains(WidgetState.pressed) ? 0.20 : 0.12))), - onPressed: () => onPressed(context), + onPressed: () => _handlePressed(context), child: Text(label(zulipLocalizations), style: const TextStyle(fontSize: 20, height: 24 / 20) .merge(weightVariableTextStyle(context, wght: 600)), @@ -186,8 +195,7 @@ class AddThumbsUpButton extends MessageActionSheetMenuItemButton { return 'React with 👍'; // TODO(i18n) skip translation for now } - @override void onPressed(BuildContext context) async { - Navigator.of(context).pop(); + @override void onPressed() async { String? errorMessage; try { await addReaction(PerAccountStoreWidget.of(pageContext).connection, @@ -231,8 +239,7 @@ class StarButton extends MessageActionSheetMenuItemButton { : zulipLocalizations.actionSheetOptionStarMessage; } - @override void onPressed(BuildContext context) async { - Navigator.of(context).pop(); + @override void onPressed() async { final zulipLocalizations = ZulipLocalizations.of(pageContext); final op = message.flags.contains(MessageFlag.starred) ? UpdateMessageFlagsOp.remove @@ -325,10 +332,7 @@ class QuoteAndReplyButton extends MessageActionSheetMenuItemButton { return zulipLocalizations.actionSheetOptionQuoteAndReply; } - @override void onPressed(BuildContext context) async { - // Close the message action sheet. We'll show the request progress - // in the compose-box content input with a "[Quoting…]" placeholder. - Navigator.of(context).pop(); + @override void onPressed() async { final zulipLocalizations = ZulipLocalizations.of(pageContext); // This will be null only if the compose box disappeared after the @@ -343,6 +347,9 @@ class QuoteAndReplyButton extends MessageActionSheetMenuItemButton { ) { topicController.value = TextEditingValue(text: message.topic); } + + // This inserts a "[Quoting…]" placeholder into the content input, + // giving the user a form of progress feedback. final tag = composeBoxController.contentController .registerQuoteAndReplyStart(PerAccountStoreWidget.of(pageContext), message: message, @@ -388,8 +395,7 @@ class MarkAsUnreadButton extends MessageActionSheetMenuItemButton { return zulipLocalizations.actionSheetOptionMarkAsUnread; } - @override void onPressed(BuildContext context) async { - Navigator.of(context).pop(); + @override void onPressed() async { unawaited(markNarrowAsUnreadFromMessage(pageContext, message, narrow)); } } @@ -408,11 +414,11 @@ class CopyMessageTextButton extends MessageActionSheetMenuItemButton { return zulipLocalizations.actionSheetOptionCopyMessageText; } - @override void onPressed(BuildContext context) async { - // Close the message action sheet. We won't be showing request progress, - // but hopefully it won't take long at all, and + @override void onPressed() async { + // This action doesn't show request progress. + // But hopefully it won't take long at all; and // fetchRawContentWithFeedback has a TODO for giving feedback if it does. - Navigator.of(context).pop(); + final zulipLocalizations = ZulipLocalizations.of(pageContext); final rawContent = await fetchRawContentWithFeedback( @@ -445,8 +451,7 @@ class CopyMessageLinkButton extends MessageActionSheetMenuItemButton { return zulipLocalizations.actionSheetOptionCopyMessageLink; } - @override void onPressed(BuildContext context) { - Navigator.of(context).pop(); + @override void onPressed() { final zulipLocalizations = ZulipLocalizations.of(pageContext); final store = PerAccountStoreWidget.of(pageContext); @@ -479,15 +484,16 @@ class ShareButton extends MessageActionSheetMenuItemButton { return zulipLocalizations.actionSheetOptionShare; } - @override void onPressed(BuildContext context) async { - // Close the message action sheet; we're about to show the share - // sheet. (We could do this after the sharing Future settles - // with [ShareResultStatus.success], but on iOS I get impatient with - // how slowly our action sheet dismisses in that case.) + @override void onPressed() async { // TODO(#591): Fix iOS bug where if the keyboard was open before the call // to `showMessageActionSheet`, it reappears briefly between // the `pop` of the action sheet and the appearance of the share sheet. - Navigator.of(context).pop(); + // + // (Alternatively we could delay the [NavigatorState.pop] that + // dismisses the action sheet until after the sharing Future settles + // with [ShareResultStatus.success]. But on iOS one gets impatient with + // how slowly our action sheet dismisses in that case.) + final zulipLocalizations = ZulipLocalizations.of(pageContext); final rawContent = await fetchRawContentWithFeedback( From 21caff2507563ff844d32c959c469178e5dd40a6 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Wed, 6 Nov 2024 19:09:20 -0800 Subject: [PATCH 06/10] action_sheet test: Use realistic result for fetch-older request --- test/widgets/action_sheet_test.dart | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/test/widgets/action_sheet_test.dart b/test/widgets/action_sheet_test.dart index 3081a6aa9a..0eca36483a 100644 --- a/test/widgets/action_sheet_test.dart +++ b/test/widgets/action_sheet_test.dart @@ -25,6 +25,7 @@ import '../api/fake_api.dart'; import '../example_data.dart' as eg; import '../flutter_checks.dart'; import '../model/binding.dart'; +import '../model/message_list_test.dart'; import '../model/test_store.dart'; import '../stdlib_checks.dart'; import '../test_clipboard.dart'; @@ -52,16 +53,8 @@ Future setupToMessageActionSheet(WidgetTester tester, { } connection = store.connection as FakeApiConnection; - // prepare message list data - connection.prepare(json: GetMessagesResult( - anchor: message.id, - foundNewest: true, - foundOldest: true, - foundAnchor: true, - historyLimited: false, - messages: [message], - ).toJson()); - + connection.prepare(json: newestResult( + foundOldest: true, messages: [message]).toJson()); await tester.pumpWidget(TestZulipApp(accountId: eg.selfAccount.id, child: MessageListPage(initNarrow: narrow))); From e25877327a6b6491f0998df8a6f222e12dc2c9c2 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Mon, 4 Nov 2024 17:13:42 -0800 Subject: [PATCH 07/10] action_sheet: Mark as unread in current narrow, vs. from when action sheet opened The narrow of a given MessageListPage can change over time, in particular if viewing a topic that gets moved. If it does, the mark-unread action should follow along. Otherwise we'd have a frustrating race where mark-unread just didn't work if, for example, someone happened to resolve the topic while you were in the middle of trying to mark as unread. Fixes: #1045 --- lib/widgets/action_sheet.dart | 13 ++++----- test/widgets/action_sheet_test.dart | 44 +++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 7 deletions(-) diff --git a/lib/widgets/action_sheet.dart b/lib/widgets/action_sheet.dart index 3b448413f3..20102572b0 100644 --- a/lib/widgets/action_sheet.dart +++ b/lib/widgets/action_sheet.dart @@ -29,11 +29,12 @@ void showMessageActionSheet({required BuildContext context, required Message mes // The UI that's conditioned on this won't live-update during this appearance // of the action sheet (we avoid calling composeBoxControllerOf in a build - // method; see its doc). But currently it will be constant through the life of - // any message list, so that's fine. + // method; see its doc). + // So we rely on the fact that isComposeBoxOffered for any given message list + // will be constant through the page's life. final messageListPage = MessageListPage.ancestorOf(context); final isComposeBoxOffered = messageListPage.composeBoxController != null; - final narrow = messageListPage.narrow; + final isMessageRead = message.flags.contains(MessageFlag.read); final markAsUnreadSupported = store.connection.zulipFeatureLevel! >= 155; // TODO(server-6) final showMarkAsUnreadButton = markAsUnreadSupported && isMessageRead; @@ -52,7 +53,7 @@ void showMessageActionSheet({required BuildContext context, required Message mes if (isComposeBoxOffered) QuoteAndReplyButton(message: message, pageContext: context), if (showMarkAsUnreadButton) - MarkAsUnreadButton(message: message, pageContext: context, narrow: narrow), + MarkAsUnreadButton(message: message, pageContext: context), CopyMessageTextButton(message: message, pageContext: context), CopyMessageLinkButton(message: message, pageContext: context), ShareButton(message: message, pageContext: context), @@ -383,11 +384,8 @@ class MarkAsUnreadButton extends MessageActionSheetMenuItemButton { super.key, required super.message, required super.pageContext, - required this.narrow, }); - final Narrow narrow; - @override IconData get icon => Icons.mark_chat_unread_outlined; @override @@ -396,6 +394,7 @@ class MarkAsUnreadButton extends MessageActionSheetMenuItemButton { } @override void onPressed() async { + final narrow = findMessageListPage().narrow; unawaited(markNarrowAsUnreadFromMessage(pageContext, message, narrow)); } } diff --git a/test/widgets/action_sheet_test.dart b/test/widgets/action_sheet_test.dart index 0eca36483a..ca855ace29 100644 --- a/test/widgets/action_sheet_test.dart +++ b/test/widgets/action_sheet_test.dart @@ -5,6 +5,7 @@ import 'package:flutter/material.dart'; import 'package:flutter/services.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:http/http.dart' as http; +import 'package:zulip/api/model/events.dart'; import 'package:zulip/api/model/model.dart'; import 'package:zulip/api/route/channels.dart'; import 'package:zulip/api/route/messages.dart'; @@ -438,6 +439,49 @@ void main() { }); }); + testWidgets('on topic move, acts on new topic', (tester) async { + final stream = eg.stream(); + const topic = 'old topic'; + final message = eg.streamMessage(flags: [MessageFlag.read], + stream: stream, topic: topic); + await setupToMessageActionSheet(tester, message: message, + narrow: TopicNarrow.ofMessage(message)); + + // Get the action sheet fully deployed while the old narrow applies. + // (This way we maximize the range of potential bugs this test can catch, + // by giving the code maximum opportunity to latch onto the old topic.) + await tester.pumpAndSettle(); + + final store = await testBinding.globalStore.perAccount(eg.selfAccount.id); + final newStream = eg.stream(); + const newTopic = 'other topic'; + // This result isn't quite realistic for this request: it should get + // the updated channel/stream ID and topic, because we don't even + // start the request until after we get the move event. + // But constructing the right result is annoying at the moment, and + // it doesn't matter anyway: [MessageStoreImpl.reconcileMessages] will + // keep the version updated by the event. If that somehow changes in + // some future refactor, it'll cause this test to fail. + connection.prepare(json: newestResult( + foundOldest: true, messages: [message]).toJson()); + await store.handleEvent(eg.updateMessageEventMoveFrom( + newStreamId: newStream.streamId, newTopic: newTopic, + propagateMode: PropagateMode.changeAll, + origMessages: [message])); + + connection.prepare(json: UpdateMessageFlagsForNarrowResult( + processedCount: 11, updatedCount: 3, + firstProcessedId: 1, lastProcessedId: 1980, + foundOldest: true, foundNewest: true).toJson()); + await tester.tap(find.byIcon(Icons.mark_chat_unread_outlined, skipOffstage: false)); + await tester.pumpAndSettle(); + check(connection.lastRequest).isA() + ..method.equals('POST') + ..url.path.equals('/api/v1/messages/flags/narrow') + ..bodyFields['narrow'].equals( + jsonEncode(TopicNarrow(newStream.streamId, newTopic).apiEncode())); + }); + testWidgets('shows error when fails', (tester) async { final message = eg.streamMessage(flags: [MessageFlag.read]); await setupToMessageActionSheet(tester, message: message, narrow: TopicNarrow.ofMessage(message)); From f741a8c777747cd4ef2cefeb7f81da05f1e102b4 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Mon, 4 Nov 2024 18:06:42 -0800 Subject: [PATCH 08/10] action_sheet [nfc]: Tighten button constructors onto one line This is a nice small bonus from renaming the parameter that was `messageListContext` to the shorter name `pageContext`. Some of these don't quite fit in 80 columns. But the details of the parameter list are boring (because they're all the same), so it's OK that some of them go a little over. --- lib/widgets/action_sheet.dart | 42 ++++++----------------------------- 1 file changed, 7 insertions(+), 35 deletions(-) diff --git a/lib/widgets/action_sheet.dart b/lib/widgets/action_sheet.dart index 20102572b0..844f820c13 100644 --- a/lib/widgets/action_sheet.dart +++ b/lib/widgets/action_sheet.dart @@ -183,11 +183,7 @@ class MessageActionSheetCancelButton extends StatelessWidget { // This button is very temporary, to complete #125 before we have a way to // choose an arbitrary reaction (#388). So, skipping i18n. class AddThumbsUpButton extends MessageActionSheetMenuItemButton { - AddThumbsUpButton({ - super.key, - required super.message, - required super.pageContext, - }); + AddThumbsUpButton({super.key, required super.message, required super.pageContext}); @override IconData get icon => ZulipIcons.smile; @@ -223,11 +219,7 @@ class AddThumbsUpButton extends MessageActionSheetMenuItemButton { } class StarButton extends MessageActionSheetMenuItemButton { - StarButton({ - super.key, - required super.message, - required super.pageContext, - }); + StarButton({super.key, required super.message, required super.pageContext}); @override IconData get icon => _isStarred ? ZulipIcons.star_filled : ZulipIcons.star; @@ -320,11 +312,7 @@ Future fetchRawContentWithFeedback({ } class QuoteAndReplyButton extends MessageActionSheetMenuItemButton { - QuoteAndReplyButton({ - super.key, - required super.message, - required super.pageContext, - }); + QuoteAndReplyButton({super.key, required super.message, required super.pageContext}); @override IconData get icon => ZulipIcons.format_quote; @@ -380,11 +368,7 @@ class QuoteAndReplyButton extends MessageActionSheetMenuItemButton { } class MarkAsUnreadButton extends MessageActionSheetMenuItemButton { - MarkAsUnreadButton({ - super.key, - required super.message, - required super.pageContext, - }); + MarkAsUnreadButton({super.key, required super.message, required super.pageContext}); @override IconData get icon => Icons.mark_chat_unread_outlined; @@ -400,11 +384,7 @@ class MarkAsUnreadButton extends MessageActionSheetMenuItemButton { } class CopyMessageTextButton extends MessageActionSheetMenuItemButton { - CopyMessageTextButton({ - super.key, - required super.message, - required super.pageContext, - }); + CopyMessageTextButton({super.key, required super.message, required super.pageContext}); @override IconData get icon => ZulipIcons.copy; @@ -437,11 +417,7 @@ class CopyMessageTextButton extends MessageActionSheetMenuItemButton { } class CopyMessageLinkButton extends MessageActionSheetMenuItemButton { - CopyMessageLinkButton({ - super.key, - required super.message, - required super.pageContext, - }); + CopyMessageLinkButton({super.key, required super.message, required super.pageContext}); @override IconData get icon => Icons.link; @@ -467,11 +443,7 @@ class CopyMessageLinkButton extends MessageActionSheetMenuItemButton { } class ShareButton extends MessageActionSheetMenuItemButton { - ShareButton({ - super.key, - required super.message, - required super.pageContext, - }); + ShareButton({super.key, required super.message, required super.pageContext}); @override IconData get icon => defaultTargetPlatform == TargetPlatform.iOS From 2f6b8e0ab4dde711d493effcdd331745dc193515 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Mon, 4 Nov 2024 17:50:26 -0800 Subject: [PATCH 09/10] action_sheet test [nfc]: Share store as a late variable These tests are all already relying on the detail that the setupToMessageActionSheet helper uses `eg.selfAccount` for the account. Better to encapsulate that detail, and have them rely on it giving the test cases a reference to the store directly. --- test/widgets/action_sheet_test.dart | 30 ++--------------------------- 1 file changed, 2 insertions(+), 28 deletions(-) diff --git a/test/widgets/action_sheet_test.dart b/test/widgets/action_sheet_test.dart index ca855ace29..62581bbaa8 100644 --- a/test/widgets/action_sheet_test.dart +++ b/test/widgets/action_sheet_test.dart @@ -35,6 +35,7 @@ import 'compose_box_checks.dart'; import 'dialog_checks.dart'; import 'test_app.dart'; +late PerAccountStore store; late FakeApiConnection connection; /// Simulates loading a [MessageListPage] and long-pressing on [message]. @@ -45,7 +46,7 @@ Future setupToMessageActionSheet(WidgetTester tester, { addTearDown(testBinding.reset); await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); - final store = await testBinding.globalStore.perAccount(eg.selfAccount.id); + store = await testBinding.globalStore.perAccount(eg.selfAccount.id); await store.addUser(eg.user(userId: message.senderId)); if (message is StreamMessage) { final stream = eg.stream(streamId: message.streamId); @@ -103,9 +104,7 @@ void main() { testWidgets('success', (tester) async { final message = eg.streamMessage(); await setupToMessageActionSheet(tester, message: message, narrow: TopicNarrow.ofMessage(message)); - final store = await testBinding.globalStore.perAccount(eg.selfAccount.id); - final connection = store.connection as FakeApiConnection; connection.prepare(json: {}); await tapButton(tester); await tester.pump(Duration.zero); @@ -123,9 +122,6 @@ void main() { testWidgets('request has an error', (tester) async { final message = eg.streamMessage(); await setupToMessageActionSheet(tester, message: message, narrow: TopicNarrow.ofMessage(message)); - final store = await testBinding.globalStore.perAccount(eg.selfAccount.id); - - final connection = store.connection as FakeApiConnection; connection.prepare(httpStatus: 400, json: { 'code': 'BAD_REQUEST', @@ -157,9 +153,7 @@ void main() { testWidgets('star success', (tester) async { final message = eg.streamMessage(flags: []); await setupToMessageActionSheet(tester, message: message, narrow: TopicNarrow.ofMessage(message)); - final store = await testBinding.globalStore.perAccount(eg.selfAccount.id); - final connection = store.connection as FakeApiConnection; connection.prepare(json: {}); await tapButton(tester); await tester.pump(Duration.zero); @@ -177,9 +171,7 @@ void main() { testWidgets('unstar success', (tester) async { final message = eg.streamMessage(flags: [MessageFlag.starred]); await setupToMessageActionSheet(tester, message: message, narrow: TopicNarrow.ofMessage(message)); - final store = await testBinding.globalStore.perAccount(eg.selfAccount.id); - final connection = store.connection as FakeApiConnection; connection.prepare(json: {}); await tapButton(tester, starred: true); await tester.pump(Duration.zero); @@ -197,11 +189,8 @@ void main() { testWidgets('star request has an error', (tester) async { final message = eg.streamMessage(flags: []); await setupToMessageActionSheet(tester, message: message, narrow: TopicNarrow.ofMessage(message)); - final store = await testBinding.globalStore.perAccount(eg.selfAccount.id); final zulipLocalizations = GlobalLocalizations.zulipLocalizations; - final connection = store.connection as FakeApiConnection; - connection.prepare(httpStatus: 400, json: { 'code': 'BAD_REQUEST', 'msg': 'Invalid message(s)', @@ -218,11 +207,8 @@ void main() { testWidgets('unstar request has an error', (tester) async { final message = eg.streamMessage(flags: [MessageFlag.starred]); await setupToMessageActionSheet(tester, message: message, narrow: TopicNarrow.ofMessage(message)); - final store = await testBinding.globalStore.perAccount(eg.selfAccount.id); final zulipLocalizations = GlobalLocalizations.zulipLocalizations; - final connection = store.connection as FakeApiConnection; - connection.prepare(httpStatus: 400, json: { 'code': 'BAD_REQUEST', 'msg': 'Invalid message(s)', @@ -291,7 +277,6 @@ void main() { testWidgets('in channel narrow', (tester) async { final message = eg.streamMessage(); await setupToMessageActionSheet(tester, message: message, narrow: ChannelNarrow(message.streamId)); - final store = await testBinding.globalStore.perAccount(eg.selfAccount.id); final composeBoxController = findComposeBoxController(tester)!; final contentController = composeBoxController.contentController; @@ -315,7 +300,6 @@ void main() { testWidgets('in topic narrow', (tester) async { final message = eg.streamMessage(); await setupToMessageActionSheet(tester, message: message, narrow: TopicNarrow.ofMessage(message)); - final store = await testBinding.globalStore.perAccount(eg.selfAccount.id); final composeBoxController = findComposeBoxController(tester)!; final contentController = composeBoxController.contentController; @@ -334,7 +318,6 @@ void main() { final message = eg.dmMessage(from: eg.selfUser, to: [eg.otherUser]); await setupToMessageActionSheet(tester, message: message, narrow: DmNarrow.ofMessage(message, selfUserId: eg.selfUser.userId)); - final store = await testBinding.globalStore.perAccount(eg.selfAccount.id); final composeBoxController = findComposeBoxController(tester)!; final contentController = composeBoxController.contentController; @@ -352,7 +335,6 @@ void main() { testWidgets('request has an error', (tester) async { final message = eg.streamMessage(); await setupToMessageActionSheet(tester, message: message, narrow: TopicNarrow.ofMessage(message)); - final store = await testBinding.globalStore.perAccount(eg.selfAccount.id); final composeBoxController = findComposeBoxController(tester)!; final contentController = composeBoxController.contentController; @@ -452,7 +434,6 @@ void main() { // by giving the code maximum opportunity to latch onto the old topic.) await tester.pumpAndSettle(); - final store = await testBinding.globalStore.perAccount(eg.selfAccount.id); final newStream = eg.stream(); const newTopic = 'other topic'; // This result isn't quite realistic for this request: it should get @@ -516,7 +497,6 @@ void main() { testWidgets('success', (tester) async { final message = eg.streamMessage(); await setupToMessageActionSheet(tester, message: message, narrow: TopicNarrow.ofMessage(message)); - final store = await testBinding.globalStore.perAccount(eg.selfAccount.id); prepareRawContentResponseSuccess(store, message: message, rawContent: 'Hello world'); await tapCopyMessageTextButton(tester); @@ -530,7 +510,6 @@ void main() { final message = eg.streamMessage(); await setupToMessageActionSheet(tester, message: message, narrow: TopicNarrow.ofMessage(message)); - final store = await testBinding.globalStore.perAccount(eg.selfAccount.id); // Make the request take a bit of time to complete… prepareRawContentResponseSuccess(store, message: message, rawContent: 'Hello world', @@ -552,7 +531,6 @@ void main() { testWidgets('request has an error', (tester) async { final message = eg.streamMessage(); await setupToMessageActionSheet(tester, message: message, narrow: TopicNarrow.ofMessage(message)); - final store = await testBinding.globalStore.perAccount(eg.selfAccount.id); prepareRawContentResponseError(store); await tapCopyMessageTextButton(tester); @@ -584,7 +562,6 @@ void main() { final message = eg.streamMessage(); final narrow = TopicNarrow.ofMessage(message); await setupToMessageActionSheet(tester, message: message, narrow: narrow); - final store = await testBinding.globalStore.perAccount(eg.selfAccount.id); await tapCopyMessageLinkButton(tester); await tester.pump(Duration.zero); @@ -614,7 +591,6 @@ void main() { final mockSharePlus = setupMockSharePlus(); final message = eg.streamMessage(); await setupToMessageActionSheet(tester, message: message, narrow: TopicNarrow.ofMessage(message)); - final store = await testBinding.globalStore.perAccount(eg.selfAccount.id); prepareRawContentResponseSuccess(store, message: message, rawContent: 'Hello world'); await tapShareButton(tester); @@ -626,7 +602,6 @@ void main() { final mockSharePlus = setupMockSharePlus(); final message = eg.streamMessage(); await setupToMessageActionSheet(tester, message: message, narrow: TopicNarrow.ofMessage(message)); - final store = await testBinding.globalStore.perAccount(eg.selfAccount.id); prepareRawContentResponseSuccess(store, message: message, rawContent: 'Hello world'); mockSharePlus.resultString = 'dev.fluttercommunity.plus/share/unavailable'; @@ -642,7 +617,6 @@ void main() { final mockSharePlus = setupMockSharePlus(); final message = eg.streamMessage(); await setupToMessageActionSheet(tester, message: message, narrow: TopicNarrow.ofMessage(message)); - final store = await testBinding.globalStore.perAccount(eg.selfAccount.id); prepareRawContentResponseError(store); await tapShareButton(tester); From cdce5709acf8a1541158cbaebaa9dfad8bf81b5f Mon Sep 17 00:00:00 2001 From: Greg Price Date: Mon, 4 Nov 2024 17:53:47 -0800 Subject: [PATCH 10/10] =?UTF-8?q?action=5Fsheet=20test=20[nfc]:=20Simplify?= =?UTF-8?q?=20prepareRawContentResponse=E2=80=A6=20helpers?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Their callers always give them the shared store prepared by the setup helper setupToMessageActionSheet. Have these helpers just use that directly. If in the future we end up having a use case for customizing this behavior, it'll probably be most convenient to have them accept a connection instead of a store, anyway. --- test/widgets/action_sheet_test.dart | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/test/widgets/action_sheet_test.dart b/test/widgets/action_sheet_test.dart index 62581bbaa8..aaf13c0b17 100644 --- a/test/widgets/action_sheet_test.dart +++ b/test/widgets/action_sheet_test.dart @@ -73,7 +73,7 @@ void main() { TestZulipBinding.ensureInitialized(); TestWidgetsFlutterBinding.ensureInitialized(); - void prepareRawContentResponseSuccess(PerAccountStore store, { + void prepareRawContentResponseSuccess({ required Message message, required String rawContent, Duration delay = Duration.zero, @@ -81,17 +81,17 @@ void main() { // Prepare fetch-raw-Markdown response // TODO: Message should really only differ from `message` // in its content / content_type, not in `id` or anything else. - (store.connection as FakeApiConnection).prepare(delay: delay, json: + connection.prepare(delay: delay, json: GetMessageResult(message: eg.streamMessage(contentMarkdown: rawContent)).toJson()); } - void prepareRawContentResponseError(PerAccountStore store) { + void prepareRawContentResponseError() { final fakeResponseJson = { 'code': 'BAD_REQUEST', 'msg': 'Invalid message(s)', 'result': 'error', }; - (store.connection as FakeApiConnection).prepare(httpStatus: 400, json: fakeResponseJson); + connection.prepare(httpStatus: 400, json: fakeResponseJson); } group('AddThumbsUpButton', () { @@ -288,7 +288,7 @@ void main() { topicController?.value = const TextEditingValue(text: kNoTopicTopic); final valueBefore = contentController.value; - prepareRawContentResponseSuccess(store, message: message, rawContent: 'Hello world'); + prepareRawContentResponseSuccess(message: message, rawContent: 'Hello world'); await tapQuoteAndReplyButton(tester); checkLoadingState(store, contentController, valueBefore: valueBefore, message: message); await tester.pump(Duration.zero); // message is fetched; compose box updates @@ -305,7 +305,7 @@ void main() { final contentController = composeBoxController.contentController; final valueBefore = contentController.value; - prepareRawContentResponseSuccess(store, message: message, rawContent: 'Hello world'); + prepareRawContentResponseSuccess(message: message, rawContent: 'Hello world'); await tapQuoteAndReplyButton(tester); checkLoadingState(store, contentController, valueBefore: valueBefore, message: message); await tester.pump(Duration.zero); // message is fetched; compose box updates @@ -323,7 +323,7 @@ void main() { final contentController = composeBoxController.contentController; final valueBefore = contentController.value; - prepareRawContentResponseSuccess(store, message: message, rawContent: 'Hello world'); + prepareRawContentResponseSuccess(message: message, rawContent: 'Hello world'); await tapQuoteAndReplyButton(tester); checkLoadingState(store, contentController, valueBefore: valueBefore, message: message); await tester.pump(Duration.zero); // message is fetched; compose box updates @@ -340,7 +340,7 @@ void main() { final contentController = composeBoxController.contentController; final valueBefore = contentController.value = TextEditingValue.empty; - prepareRawContentResponseError(store); + prepareRawContentResponseError(); await tapQuoteAndReplyButton(tester); checkLoadingState(store, contentController, valueBefore: valueBefore, message: message); await tester.pump(Duration.zero); // error arrives; error dialog shows @@ -498,7 +498,7 @@ void main() { final message = eg.streamMessage(); await setupToMessageActionSheet(tester, message: message, narrow: TopicNarrow.ofMessage(message)); - prepareRawContentResponseSuccess(store, message: message, rawContent: 'Hello world'); + prepareRawContentResponseSuccess(message: message, rawContent: 'Hello world'); await tapCopyMessageTextButton(tester); await tester.pump(Duration.zero); check(await Clipboard.getData('text/plain')).isNotNull().text.equals('Hello world'); @@ -512,7 +512,7 @@ void main() { await setupToMessageActionSheet(tester, message: message, narrow: TopicNarrow.ofMessage(message)); // Make the request take a bit of time to complete… - prepareRawContentResponseSuccess(store, message: message, rawContent: 'Hello world', + prepareRawContentResponseSuccess(message: message, rawContent: 'Hello world', delay: const Duration(milliseconds: 500)); await tapCopyMessageTextButton(tester); // … and pump a frame to finish the NavigationState.pop animation… @@ -532,7 +532,7 @@ void main() { final message = eg.streamMessage(); await setupToMessageActionSheet(tester, message: message, narrow: TopicNarrow.ofMessage(message)); - prepareRawContentResponseError(store); + prepareRawContentResponseError(); await tapCopyMessageTextButton(tester); await tester.pump(Duration.zero); // error arrives; error dialog shows @@ -592,7 +592,7 @@ void main() { final message = eg.streamMessage(); await setupToMessageActionSheet(tester, message: message, narrow: TopicNarrow.ofMessage(message)); - prepareRawContentResponseSuccess(store, message: message, rawContent: 'Hello world'); + prepareRawContentResponseSuccess(message: message, rawContent: 'Hello world'); await tapShareButton(tester); await tester.pump(Duration.zero); check(mockSharePlus.sharedString).equals('Hello world'); @@ -603,7 +603,7 @@ void main() { final message = eg.streamMessage(); await setupToMessageActionSheet(tester, message: message, narrow: TopicNarrow.ofMessage(message)); - prepareRawContentResponseSuccess(store, message: message, rawContent: 'Hello world'); + prepareRawContentResponseSuccess(message: message, rawContent: 'Hello world'); mockSharePlus.resultString = 'dev.fluttercommunity.plus/share/unavailable'; await tapShareButton(tester); await tester.pump(Duration.zero); @@ -618,7 +618,7 @@ void main() { final message = eg.streamMessage(); await setupToMessageActionSheet(tester, message: message, narrow: TopicNarrow.ofMessage(message)); - prepareRawContentResponseError(store); + prepareRawContentResponseError(); await tapShareButton(tester); await tester.pump(Duration.zero); // error arrives; error dialog shows