diff --git a/assets/icons/ZulipIcons.ttf b/assets/icons/ZulipIcons.ttf index 15fce5b25a..0be670d5cb 100644 Binary files a/assets/icons/ZulipIcons.ttf and b/assets/icons/ZulipIcons.ttf differ diff --git a/assets/icons/check.svg b/assets/icons/check.svg new file mode 100644 index 0000000000..26332a3599 --- /dev/null +++ b/assets/icons/check.svg @@ -0,0 +1,3 @@ + + + diff --git a/assets/icons/check_remove.svg b/assets/icons/check_remove.svg new file mode 100644 index 0000000000..cc5939b04d --- /dev/null +++ b/assets/icons/check_remove.svg @@ -0,0 +1,4 @@ + + + + diff --git a/assets/l10n/app_en.arb b/assets/l10n/app_en.arb index ee7e96c35f..ba02a819b1 100644 --- a/assets/l10n/app_en.arb +++ b/assets/l10n/app_en.arb @@ -92,6 +92,22 @@ "@actionSheetOptionUnfollowTopic": { "description": "Label for unfollowing a topic on action sheet." }, + "actionSheetOptionResolveTopic": "Mark as resolved", + "@actionSheetOptionResolveTopic": { + "description": "Label for the 'Mark as resolved' button on the topic action sheet." + }, + "actionSheetOptionUnresolveTopic": "Mark as unresolved", + "@actionSheetOptionUnresolveTopic": { + "description": "Label for the 'Mark as unresolved' button on the topic action sheet." + }, + "errorResolveTopicFailedTitle": "Failed to mark topic as resolved", + "@errorResolveTopicFailedTitle": { + "description": "Error title when marking a topic as resolved failed." + }, + "errorUnresolveTopicFailedTitle": "Failed to mark topic as unresolved", + "@errorUnresolveTopicFailedTitle": { + "description": "Error title when marking a topic as unresolved failed." + }, "actionSheetOptionCopyMessageText": "Copy message text", "@actionSheetOptionCopyMessageText": { "description": "Label for copy message text button on action sheet." diff --git a/lib/api/model/model.dart b/lib/api/model/model.dart index 03af104baf..fad8ddc5bc 100644 --- a/lib/api/model/model.dart +++ b/lib/api/model/model.dart @@ -695,6 +695,12 @@ extension type const TopicName(String _value) { /// The key to use for "same topic as" comparisons. String canonicalize() => apiName.toLowerCase(); + /// Whether the topic starts with [resolvedTopicPrefix]. + bool get isResolved => _value.startsWith(resolvedTopicPrefix); + + /// This [TopicName] plus the [resolvedTopicPrefix] prefix. + TopicName resolve() => TopicName(resolvedTopicPrefix + _value); + /// A [TopicName] with [resolvedTopicPrefixRegexp] stripped if present. TopicName unresolve() => TopicName(_value.replaceFirst(resolvedTopicPrefixRegexp, '')); diff --git a/lib/generated/l10n/zulip_localizations.dart b/lib/generated/l10n/zulip_localizations.dart index b6fbb70769..9be039ef72 100644 --- a/lib/generated/l10n/zulip_localizations.dart +++ b/lib/generated/l10n/zulip_localizations.dart @@ -243,6 +243,30 @@ abstract class ZulipLocalizations { /// **'Unfollow topic'** String get actionSheetOptionUnfollowTopic; + /// Label for the 'Mark as resolved' button on the topic action sheet. + /// + /// In en, this message translates to: + /// **'Mark as resolved'** + String get actionSheetOptionResolveTopic; + + /// Label for the 'Mark as unresolved' button on the topic action sheet. + /// + /// In en, this message translates to: + /// **'Mark as unresolved'** + String get actionSheetOptionUnresolveTopic; + + /// Error title when marking a topic as resolved failed. + /// + /// In en, this message translates to: + /// **'Failed to mark topic as resolved'** + String get errorResolveTopicFailedTitle; + + /// Error title when marking a topic as unresolved failed. + /// + /// In en, this message translates to: + /// **'Failed to mark topic as unresolved'** + String get errorUnresolveTopicFailedTitle; + /// Label for copy message text button on action sheet. /// /// In en, this message translates to: diff --git a/lib/generated/l10n/zulip_localizations_ar.dart b/lib/generated/l10n/zulip_localizations_ar.dart index 025b4b1444..2e59dae8af 100644 --- a/lib/generated/l10n/zulip_localizations_ar.dart +++ b/lib/generated/l10n/zulip_localizations_ar.dart @@ -79,6 +79,18 @@ class ZulipLocalizationsAr extends ZulipLocalizations { @override String get actionSheetOptionUnfollowTopic => 'Unfollow topic'; + @override + String get actionSheetOptionResolveTopic => 'Mark as resolved'; + + @override + String get actionSheetOptionUnresolveTopic => 'Mark as unresolved'; + + @override + String get errorResolveTopicFailedTitle => 'Failed to mark topic as resolved'; + + @override + String get errorUnresolveTopicFailedTitle => 'Failed to mark topic as unresolved'; + @override String get actionSheetOptionCopyMessageText => 'Copy message text'; diff --git a/lib/generated/l10n/zulip_localizations_en.dart b/lib/generated/l10n/zulip_localizations_en.dart index 9467d33428..a0e86b7d35 100644 --- a/lib/generated/l10n/zulip_localizations_en.dart +++ b/lib/generated/l10n/zulip_localizations_en.dart @@ -79,6 +79,18 @@ class ZulipLocalizationsEn extends ZulipLocalizations { @override String get actionSheetOptionUnfollowTopic => 'Unfollow topic'; + @override + String get actionSheetOptionResolveTopic => 'Mark as resolved'; + + @override + String get actionSheetOptionUnresolveTopic => 'Mark as unresolved'; + + @override + String get errorResolveTopicFailedTitle => 'Failed to mark topic as resolved'; + + @override + String get errorUnresolveTopicFailedTitle => 'Failed to mark topic as unresolved'; + @override String get actionSheetOptionCopyMessageText => 'Copy message text'; diff --git a/lib/generated/l10n/zulip_localizations_ja.dart b/lib/generated/l10n/zulip_localizations_ja.dart index f363ee0043..6e7f1559ac 100644 --- a/lib/generated/l10n/zulip_localizations_ja.dart +++ b/lib/generated/l10n/zulip_localizations_ja.dart @@ -79,6 +79,18 @@ class ZulipLocalizationsJa extends ZulipLocalizations { @override String get actionSheetOptionUnfollowTopic => 'Unfollow topic'; + @override + String get actionSheetOptionResolveTopic => 'Mark as resolved'; + + @override + String get actionSheetOptionUnresolveTopic => 'Mark as unresolved'; + + @override + String get errorResolveTopicFailedTitle => 'Failed to mark topic as resolved'; + + @override + String get errorUnresolveTopicFailedTitle => 'Failed to mark topic as unresolved'; + @override String get actionSheetOptionCopyMessageText => 'Copy message text'; diff --git a/lib/generated/l10n/zulip_localizations_nb.dart b/lib/generated/l10n/zulip_localizations_nb.dart index 35b3e86fe5..cabd4897e9 100644 --- a/lib/generated/l10n/zulip_localizations_nb.dart +++ b/lib/generated/l10n/zulip_localizations_nb.dart @@ -79,6 +79,18 @@ class ZulipLocalizationsNb extends ZulipLocalizations { @override String get actionSheetOptionUnfollowTopic => 'Unfollow topic'; + @override + String get actionSheetOptionResolveTopic => 'Mark as resolved'; + + @override + String get actionSheetOptionUnresolveTopic => 'Mark as unresolved'; + + @override + String get errorResolveTopicFailedTitle => 'Failed to mark topic as resolved'; + + @override + String get errorUnresolveTopicFailedTitle => 'Failed to mark topic as unresolved'; + @override String get actionSheetOptionCopyMessageText => 'Copy message text'; diff --git a/lib/generated/l10n/zulip_localizations_pl.dart b/lib/generated/l10n/zulip_localizations_pl.dart index 0594722d31..0a2a2bf1d7 100644 --- a/lib/generated/l10n/zulip_localizations_pl.dart +++ b/lib/generated/l10n/zulip_localizations_pl.dart @@ -79,6 +79,18 @@ class ZulipLocalizationsPl extends ZulipLocalizations { @override String get actionSheetOptionUnfollowTopic => 'Nie śledź wątku'; + @override + String get actionSheetOptionResolveTopic => 'Mark as resolved'; + + @override + String get actionSheetOptionUnresolveTopic => 'Mark as unresolved'; + + @override + String get errorResolveTopicFailedTitle => 'Failed to mark topic as resolved'; + + @override + String get errorUnresolveTopicFailedTitle => 'Failed to mark topic as unresolved'; + @override String get actionSheetOptionCopyMessageText => 'Skopiuj tekst wiadomości'; diff --git a/lib/generated/l10n/zulip_localizations_ru.dart b/lib/generated/l10n/zulip_localizations_ru.dart index 879559fed4..99bd72c62f 100644 --- a/lib/generated/l10n/zulip_localizations_ru.dart +++ b/lib/generated/l10n/zulip_localizations_ru.dart @@ -79,6 +79,18 @@ class ZulipLocalizationsRu extends ZulipLocalizations { @override String get actionSheetOptionUnfollowTopic => 'Не отслеживать тему'; + @override + String get actionSheetOptionResolveTopic => 'Mark as resolved'; + + @override + String get actionSheetOptionUnresolveTopic => 'Mark as unresolved'; + + @override + String get errorResolveTopicFailedTitle => 'Failed to mark topic as resolved'; + + @override + String get errorUnresolveTopicFailedTitle => 'Failed to mark topic as unresolved'; + @override String get actionSheetOptionCopyMessageText => 'Скопировать текст сообщения'; diff --git a/lib/generated/l10n/zulip_localizations_sk.dart b/lib/generated/l10n/zulip_localizations_sk.dart index af87dfd949..13646eafd5 100644 --- a/lib/generated/l10n/zulip_localizations_sk.dart +++ b/lib/generated/l10n/zulip_localizations_sk.dart @@ -79,6 +79,18 @@ class ZulipLocalizationsSk extends ZulipLocalizations { @override String get actionSheetOptionUnfollowTopic => 'Prestať sledovať tému'; + @override + String get actionSheetOptionResolveTopic => 'Mark as resolved'; + + @override + String get actionSheetOptionUnresolveTopic => 'Mark as unresolved'; + + @override + String get errorResolveTopicFailedTitle => 'Failed to mark topic as resolved'; + + @override + String get errorUnresolveTopicFailedTitle => 'Failed to mark topic as unresolved'; + @override String get actionSheetOptionCopyMessageText => 'Skopírovať text správy'; diff --git a/lib/widgets/action_sheet.dart b/lib/widgets/action_sheet.dart index 3eff182bca..7c3ea6e622 100644 --- a/lib/widgets/action_sheet.dart +++ b/lib/widgets/action_sheet.dart @@ -24,6 +24,7 @@ import 'emoji_reaction.dart'; import 'icons.dart'; import 'inset_shadow.dart'; import 'message_list.dart'; +import 'page.dart'; import 'store.dart'; import 'text.dart'; import 'theme.dart'; @@ -163,11 +164,19 @@ class ActionSheetCancelButton extends StatelessWidget { } /// Show a sheet of actions you can take on a topic. +/// +/// Needs a [PageRoot] ancestor. +/// +/// The API request for resolving/unresolving a topic needs a message ID. +/// If [someMessageIdInTopic] is null, the button for that will be absent. void showTopicActionSheet(BuildContext context, { required int channelId, required TopicName topic, + required int? someMessageIdInTopic, }) { - final store = PerAccountStoreWidget.of(context); + final pageContext = PageRoot.contextOf(context); + + final store = PerAccountStoreWidget.of(pageContext); final subscription = store.subscriptions[channelId]; final optionButtons = []; @@ -237,9 +246,15 @@ void showTopicActionSheet(BuildContext context, { currentVisibilityPolicy: visibilityPolicy, newVisibilityPolicy: to, narrow: TopicNarrow(channelId, topic), - pageContext: context); + pageContext: pageContext); })); + if (someMessageIdInTopic != null) { + optionButtons.add(ResolveUnresolveButton(pageContext: pageContext, + topic: topic, + someMessageIdInTopic: someMessageIdInTopic)); + } + if (optionButtons.isEmpty) { // TODO(a11y): This case makes a no-op gesture handler; as a consequence, // we're presenting some UI (to people who use screen-reader software) as @@ -250,7 +265,7 @@ void showTopicActionSheet(BuildContext context, { return; } - _showActionSheet(context, optionButtons: optionButtons); + _showActionSheet(pageContext, optionButtons: optionButtons); } class UserTopicUpdateButton extends ActionSheetMenuItemButton { @@ -372,18 +387,93 @@ class UserTopicUpdateButton extends ActionSheetMenuItemButton { } } +class ResolveUnresolveButton extends ActionSheetMenuItemButton { + ResolveUnresolveButton({ + super.key, + required this.topic, + required this.someMessageIdInTopic, + required super.pageContext, + }) : _actionIsResolve = !topic.isResolved; + + /// The topic that the action sheet was opened for. + /// + /// There might not currently be any messages with this topic; + /// see dartdoc of [ActionSheetMenuItemButton]. + final TopicName topic; + + /// The message ID that was passed when opening the action sheet. + /// + /// The message with this ID might currently not exist, + /// or might exist with a different topic; + /// see dartdoc of [ActionSheetMenuItemButton]. + final int someMessageIdInTopic; + + final bool _actionIsResolve; + + @override + IconData get icon => _actionIsResolve ? ZulipIcons.check : ZulipIcons.check_remove; + + @override + String label(ZulipLocalizations zulipLocalizations) { + return _actionIsResolve + ? zulipLocalizations.actionSheetOptionResolveTopic + : zulipLocalizations.actionSheetOptionUnresolveTopic; + } + + @override void onPressed() async { + final zulipLocalizations = ZulipLocalizations.of(pageContext); + final store = PerAccountStoreWidget.of(pageContext); + + // We *could* check here if the topic has changed since the action sheet was + // opened (see dartdoc of [ActionSheetMenuItemButton]) and abort if so. + // We simplify by not doing so. + // There's already an inherent race that that check wouldn't help with: + // when you tap the button, an intervening topic change may already have + // happened, just not reached us in an event yet. + // Discussion, including about what web does: + // https://github.com/zulip/zulip-flutter/pull/1301#discussion_r1936181560 + + try { + await updateMessage(store.connection, + messageId: someMessageIdInTopic, + topic: _actionIsResolve ? topic.resolve() : topic.unresolve(), + propagateMode: PropagateMode.changeAll, + sendNotificationToOldThread: false, + sendNotificationToNewThread: true, + ); + } catch (e) { + if (!pageContext.mounted) return; + + String? errorMessage; + switch (e) { + case ZulipApiException(): + errorMessage = e.message; + // TODO(#741) specific messages for common errors, like network errors + // (support with reusable code) + default: + } + + final title = _actionIsResolve + ? zulipLocalizations.errorResolveTopicFailedTitle + : zulipLocalizations.errorUnresolveTopicFailedTitle; + showErrorDialog(context: pageContext, title: title, message: errorMessage); + } + } +} + /// Show a sheet of actions you can take on a message in the message list. /// /// Must have a [MessageListPage] ancestor. void showMessageActionSheet({required BuildContext context, required Message message}) { - final store = PerAccountStoreWidget.of(context); + final pageContext = PageRoot.contextOf(context); + final store = PerAccountStoreWidget.of(pageContext); // 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). // 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 messageListPage = MessageListPage.ancestorOf(pageContext); final isComposeBoxOffered = messageListPage.composeBoxController != null; final isMessageRead = message.flags.contains(MessageFlag.read); @@ -391,18 +481,18 @@ void showMessageActionSheet({required BuildContext context, required Message mes final showMarkAsUnreadButton = markAsUnreadSupported && isMessageRead; final optionButtons = [ - ReactionButtons(message: message, pageContext: context), - StarButton(message: message, pageContext: context), + ReactionButtons(message: message, pageContext: pageContext), + StarButton(message: message, pageContext: pageContext), if (isComposeBoxOffered) - QuoteAndReplyButton(message: message, pageContext: context), + QuoteAndReplyButton(message: message, pageContext: pageContext), if (showMarkAsUnreadButton) - MarkAsUnreadButton(message: message, pageContext: context), - CopyMessageTextButton(message: message, pageContext: context), - CopyMessageLinkButton(message: message, pageContext: context), - ShareButton(message: message, pageContext: context), + MarkAsUnreadButton(message: message, pageContext: pageContext), + CopyMessageTextButton(message: message, pageContext: pageContext), + CopyMessageLinkButton(message: message, pageContext: pageContext), + ShareButton(message: message, pageContext: pageContext), ]; - _showActionSheet(context, optionButtons: optionButtons); + _showActionSheet(pageContext, optionButtons: optionButtons); } abstract class MessageActionSheetMenuItemButton extends ActionSheetMenuItemButton { diff --git a/lib/widgets/icons.dart b/lib/widgets/icons.dart index 7d58305fb4..82cb83704b 100644 --- a/lib/widgets/icons.dart +++ b/lib/widgets/icons.dart @@ -42,95 +42,101 @@ abstract final class ZulipIcons { /// The Zulip custom icon "camera". static const IconData camera = IconData(0xf106, fontFamily: "Zulip Icons"); + /// The Zulip custom icon "check". + static const IconData check = IconData(0xf107, fontFamily: "Zulip Icons"); + + /// The Zulip custom icon "check_remove". + static const IconData check_remove = IconData(0xf108, fontFamily: "Zulip Icons"); + /// The Zulip custom icon "chevron_right". - static const IconData chevron_right = IconData(0xf107, fontFamily: "Zulip Icons"); + static const IconData chevron_right = IconData(0xf109, fontFamily: "Zulip Icons"); /// The Zulip custom icon "clock". - static const IconData clock = IconData(0xf108, fontFamily: "Zulip Icons"); + static const IconData clock = IconData(0xf10a, fontFamily: "Zulip Icons"); /// The Zulip custom icon "contacts". - static const IconData contacts = IconData(0xf109, fontFamily: "Zulip Icons"); + static const IconData contacts = IconData(0xf10b, fontFamily: "Zulip Icons"); /// The Zulip custom icon "copy". - static const IconData copy = IconData(0xf10a, fontFamily: "Zulip Icons"); + static const IconData copy = IconData(0xf10c, fontFamily: "Zulip Icons"); /// The Zulip custom icon "follow". - static const IconData follow = IconData(0xf10b, fontFamily: "Zulip Icons"); + static const IconData follow = IconData(0xf10d, fontFamily: "Zulip Icons"); /// The Zulip custom icon "format_quote". - static const IconData format_quote = IconData(0xf10c, fontFamily: "Zulip Icons"); + static const IconData format_quote = IconData(0xf10e, fontFamily: "Zulip Icons"); /// The Zulip custom icon "globe". - static const IconData globe = IconData(0xf10d, fontFamily: "Zulip Icons"); + static const IconData globe = IconData(0xf10f, fontFamily: "Zulip Icons"); /// The Zulip custom icon "group_dm". - static const IconData group_dm = IconData(0xf10e, fontFamily: "Zulip Icons"); + static const IconData group_dm = IconData(0xf110, fontFamily: "Zulip Icons"); /// The Zulip custom icon "hash_italic". - static const IconData hash_italic = IconData(0xf10f, fontFamily: "Zulip Icons"); + static const IconData hash_italic = IconData(0xf111, fontFamily: "Zulip Icons"); /// The Zulip custom icon "hash_sign". - static const IconData hash_sign = IconData(0xf110, fontFamily: "Zulip Icons"); + static const IconData hash_sign = IconData(0xf112, fontFamily: "Zulip Icons"); /// The Zulip custom icon "image". - static const IconData image = IconData(0xf111, fontFamily: "Zulip Icons"); + static const IconData image = IconData(0xf113, fontFamily: "Zulip Icons"); /// The Zulip custom icon "inbox". - static const IconData inbox = IconData(0xf112, fontFamily: "Zulip Icons"); + static const IconData inbox = IconData(0xf114, fontFamily: "Zulip Icons"); /// The Zulip custom icon "info". - static const IconData info = IconData(0xf113, fontFamily: "Zulip Icons"); + static const IconData info = IconData(0xf115, fontFamily: "Zulip Icons"); /// The Zulip custom icon "inherit". - static const IconData inherit = IconData(0xf114, fontFamily: "Zulip Icons"); + static const IconData inherit = IconData(0xf116, fontFamily: "Zulip Icons"); /// The Zulip custom icon "language". - static const IconData language = IconData(0xf115, fontFamily: "Zulip Icons"); + static const IconData language = IconData(0xf117, fontFamily: "Zulip Icons"); /// The Zulip custom icon "lock". - static const IconData lock = IconData(0xf116, fontFamily: "Zulip Icons"); + static const IconData lock = IconData(0xf118, fontFamily: "Zulip Icons"); /// The Zulip custom icon "menu". - static const IconData menu = IconData(0xf117, fontFamily: "Zulip Icons"); + static const IconData menu = IconData(0xf119, fontFamily: "Zulip Icons"); /// The Zulip custom icon "message_feed". - static const IconData message_feed = IconData(0xf118, fontFamily: "Zulip Icons"); + static const IconData message_feed = IconData(0xf11a, fontFamily: "Zulip Icons"); /// The Zulip custom icon "mute". - static const IconData mute = IconData(0xf119, fontFamily: "Zulip Icons"); + static const IconData mute = IconData(0xf11b, fontFamily: "Zulip Icons"); /// The Zulip custom icon "read_receipts". - static const IconData read_receipts = IconData(0xf11a, fontFamily: "Zulip Icons"); + static const IconData read_receipts = IconData(0xf11c, fontFamily: "Zulip Icons"); /// The Zulip custom icon "send". - static const IconData send = IconData(0xf11b, fontFamily: "Zulip Icons"); + static const IconData send = IconData(0xf11d, fontFamily: "Zulip Icons"); /// The Zulip custom icon "share". - static const IconData share = IconData(0xf11c, fontFamily: "Zulip Icons"); + static const IconData share = IconData(0xf11e, fontFamily: "Zulip Icons"); /// The Zulip custom icon "share_ios". - static const IconData share_ios = IconData(0xf11d, fontFamily: "Zulip Icons"); + static const IconData share_ios = IconData(0xf11f, fontFamily: "Zulip Icons"); /// The Zulip custom icon "smile". - static const IconData smile = IconData(0xf11e, fontFamily: "Zulip Icons"); + static const IconData smile = IconData(0xf120, fontFamily: "Zulip Icons"); /// The Zulip custom icon "star". - static const IconData star = IconData(0xf11f, fontFamily: "Zulip Icons"); + static const IconData star = IconData(0xf121, fontFamily: "Zulip Icons"); /// The Zulip custom icon "star_filled". - static const IconData star_filled = IconData(0xf120, fontFamily: "Zulip Icons"); + static const IconData star_filled = IconData(0xf122, fontFamily: "Zulip Icons"); /// The Zulip custom icon "three_person". - static const IconData three_person = IconData(0xf121, fontFamily: "Zulip Icons"); + static const IconData three_person = IconData(0xf123, fontFamily: "Zulip Icons"); /// The Zulip custom icon "topic". - static const IconData topic = IconData(0xf122, fontFamily: "Zulip Icons"); + static const IconData topic = IconData(0xf124, fontFamily: "Zulip Icons"); /// The Zulip custom icon "unmute". - static const IconData unmute = IconData(0xf123, fontFamily: "Zulip Icons"); + static const IconData unmute = IconData(0xf125, fontFamily: "Zulip Icons"); /// The Zulip custom icon "user". - static const IconData user = IconData(0xf124, fontFamily: "Zulip Icons"); + static const IconData user = IconData(0xf126, fontFamily: "Zulip Icons"); // END GENERATED ICON DATA } diff --git a/lib/widgets/inbox.dart b/lib/widgets/inbox.dart index 6dbe31ce04..799f763f1c 100644 --- a/lib/widgets/inbox.dart +++ b/lib/widgets/inbox.dart @@ -507,7 +507,8 @@ class _TopicItem extends StatelessWidget { @override Widget build(BuildContext context) { - final _StreamSectionTopicData(:topic, :count, :hasMention) = data; + final _StreamSectionTopicData( + :topic, :count, :hasMention, :lastUnreadId) = data; final store = PerAccountStoreWidget.of(context); final subscription = store.subscriptions[streamId]!; @@ -525,7 +526,9 @@ class _TopicItem extends StatelessWidget { MessageListPage.buildRoute(context: context, narrow: narrow)); }, onLongPress: () => showTopicActionSheet(context, - channelId: streamId, topic: topic), + channelId: streamId, + topic: topic, + someMessageIdInTopic: lastUnreadId), child: ConstrainedBox(constraints: const BoxConstraints(minHeight: 34), child: Row(crossAxisAlignment: CrossAxisAlignment.center, children: [ const SizedBox(width: 63), diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index a27a8051e9..9f21a19fb0 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -277,7 +277,9 @@ class _MessageListPageState extends State implements MessageLis narrow: ChannelNarrow(streamId))))); } - return Scaffold( + // Insert a PageRoot here, to provide a context that can be used for + // MessageListPage.ancestorOf. + return PageRoot(child: Scaffold( appBar: ZulipAppBar( buildTitle: (willCenterTitle) => MessageListAppBarTitle(narrow: narrow, willCenterTitle: willCenterTitle), @@ -318,7 +320,7 @@ class _MessageListPageState extends State implements MessageLis ))), if (ComposeBox.hasComposeBox(narrow)) ComposeBox(key: _composeBoxKey, narrow: narrow) - ]))); + ])))); } } @@ -402,8 +404,20 @@ class MessageListAppBarTitle extends StatelessWidget { width: double.infinity, child: GestureDetector( behavior: HitTestBehavior.translucent, - onLongPress: () => showTopicActionSheet(context, - channelId: streamId, topic: topic), + onLongPress: () { + final someMessage = MessageListPage.ancestorOf(context) + .model?.messages.firstOrNull; + // If someMessage is null, the topic action sheet won't have a + // resolve/unresolve button. That seems OK; in that case we're + // either still fetching messages (and the user can reopen the + // sheet after that finishes) or there aren't any messages to + // act on anyway. + assert(someMessage == null || narrow.containsMessage(someMessage)); + showTopicActionSheet(context, + channelId: streamId, + topic: topic, + someMessageIdInTopic: someMessage?.id); + }, child: Column( crossAxisAlignment: willCenterTitle ? CrossAxisAlignment.center : CrossAxisAlignment.start, @@ -1125,7 +1139,9 @@ class StreamMessageRecipientHeader extends StatelessWidget { MessageListPage.buildRoute(context: context, narrow: TopicNarrow.ofMessage(message))), onLongPress: () => showTopicActionSheet(context, - channelId: message.streamId, topic: topic), + channelId: message.streamId, + topic: topic, + someMessageIdInTopic: message.id), child: ColoredBox( color: backgroundColor, child: Row( diff --git a/lib/widgets/page.dart b/lib/widgets/page.dart index bebb37c22c..0ba65fb3d4 100644 --- a/lib/widgets/page.dart +++ b/lib/widgets/page.dart @@ -3,6 +3,30 @@ import 'package:flutter/material.dart'; import 'store.dart'; +/// An [InheritedWidget] for near the root of a page's widget subtree, +/// providing its [BuildContext]. +/// +/// Useful when needing a context that persists through the page's lifespan, +/// e.g. for a show-action-sheet function +/// whose buttons use a context to close the sheet +/// or show an error dialog / snackbar asynchronously. +/// +/// (In this scenario, it would be buggy to use the context of the element +/// that was long-pressed, +/// if the element can unmount as part of handling a Zulip event.) +class PageRoot extends InheritedWidget { + const PageRoot({super.key, required super.child}); + + @override + bool updateShouldNotify(covariant PageRoot oldWidget) => false; + + static BuildContext contextOf(BuildContext context) { + final element = context.getElementForInheritedWidgetOfExactType(); + assert(element != null, 'No PageRoot ancestor'); + return element!; + } +} + /// A page route that always builds the same widget. /// /// This is useful for making the route more transparent for a test to inspect. @@ -42,7 +66,10 @@ mixin AccountPageRouteMixin on PageRoute { accountId: accountId, placeholder: loadingPlaceholderPage ?? const LoadingPlaceholderPage(), routeToRemoveOnLogout: this, - child: super.buildPage(context, animation, secondaryAnimation)); + // PageRoot goes under PerAccountStoreWidget, so the provided context + // can be used for PerAccountStoreWidget.of. + child: PageRoot( + child: super.buildPage(context, animation, secondaryAnimation))); } } diff --git a/test/widgets/action_sheet_test.dart b/test/widgets/action_sheet_test.dart index 2bb07b4946..7da94cfd36 100644 --- a/test/widgets/action_sheet_test.dart +++ b/test/widgets/action_sheet_test.dart @@ -227,9 +227,10 @@ void main() { } checkButton('Follow topic'); + checkButton('Mark as resolved'); } - testWidgets('show from inbox', (tester) async { + testWidgets('show from inbox; message in Unreads but not in MessageStore', (tester) async { await prepare(unreadMsgs: eg.unreadMsgs(count: 1, channels: [eg.unreadChannelMsgs( streamId: someChannel.streamId, @@ -237,6 +238,17 @@ void main() { unreadMessageIds: [someMessage.id], )])); await showFromInbox(tester); + check(store.unreads.isUnread(someMessage.id)).isNotNull().isTrue(); + check(store.messages).not((it) => it.containsKey(someMessage.id)); + checkButtons(); + }); + + testWidgets('show from inbox; message in Unreads and in MessageStore', (tester) async { + await prepare(); + await store.addMessage(someMessage); + await showFromInbox(tester); + check(store.unreads.isUnread(someMessage.id)).isNotNull().isTrue(); + check(store.messages)[someMessage.id].isNotNull(); checkButtons(); }); @@ -246,6 +258,13 @@ void main() { checkButtons(); }); + testWidgets('show from app bar: resolve/unresolve not offered when msglist empty', (tester) async { + await prepare(); + await showFromAppBar(tester, messages: []); + check(findButtonForLabel('Mark as resolved')).findsNothing(); + check(findButtonForLabel('Mark as unresolved')).findsNothing(); + }); + testWidgets('show from recipient header', (tester) async { await prepare(); await showFromRecipientHeader(tester); @@ -289,10 +308,6 @@ void main() { } void checkButtons(List expectedButtonFinders) { - if (expectedButtonFinders.isEmpty) { - check(actionSheetFinder).findsNothing(); - return; - } check(actionSheetFinder).findsOne(); for (final buttonFinder in expectedButtonFinders) { @@ -450,6 +465,109 @@ void main() { } }); }); + + group('ResolveUnresolveButton', () { + void checkRequest(int messageId, String topic) { + check(connection.takeRequests()).single.isA() + ..method.equals('PATCH') + ..url.path.equals('/api/v1/messages/$messageId') + ..bodyFields.deepEquals({ + 'topic': topic, + 'propagate_mode': 'change_all', + 'send_notification_to_old_thread': 'false', + 'send_notification_to_new_thread': 'true', + }); + } + + testWidgets('resolve: happy path from inbox; message in Unreads but not MessageStore', (tester) async { + final message = eg.streamMessage(stream: someChannel, topic: 'zulip'); + await prepare( + topic: 'zulip', + unreadMsgs: eg.unreadMsgs(count: 1, + channels: [eg.unreadChannelMsgs( + streamId: someChannel.streamId, + topic: 'zulip', + unreadMessageIds: [message.id], + )])); + await showFromInbox(tester, topic: 'zulip'); + check(store.messages).not((it) => it.containsKey(message.id)); + connection.prepare(json: UpdateMessageResult().toJson()); + await tester.tap(findButtonForLabel('Mark as resolved')); + await tester.pumpAndSettle(); + + checkNoErrorDialog(tester); + checkRequest(message.id, '✔ zulip'); + }); + + testWidgets('resolve: happy path from inbox; message in Unreads and MessageStore', (tester) async { + final message = eg.streamMessage(stream: someChannel, topic: 'zulip'); + await prepare(topic: 'zulip'); + await store.addMessage(message); + await showFromInbox(tester, topic: 'zulip'); + check(store.unreads.isUnread(message.id)).isNotNull().isTrue(); + check(store.messages)[message.id].isNotNull(); + connection.prepare(json: UpdateMessageResult().toJson()); + await tester.tap(findButtonForLabel('Mark as resolved')); + await tester.pumpAndSettle(); + + checkNoErrorDialog(tester); + checkRequest(message.id, '✔ zulip'); + }); + + testWidgets('unresolve: happy path', (tester) async { + final message = eg.streamMessage(stream: someChannel, topic: '✔ zulip'); + await prepare(topic: '✔ zulip'); + await showFromAppBar(tester, topic: '✔ zulip', messages: [message]); + connection.takeRequests(); + connection.prepare(json: UpdateMessageResult().toJson()); + await tester.tap(findButtonForLabel('Mark as unresolved')); + await tester.pumpAndSettle(); + + checkNoErrorDialog(tester); + checkRequest(message.id, 'zulip'); + }); + + testWidgets('unresolve: weird prefix', (tester) async { + final message = eg.streamMessage(stream: someChannel, topic: '✔ ✔ zulip'); + await prepare(topic: '✔ ✔ zulip'); + await showFromAppBar(tester, topic: '✔ ✔ zulip', messages: [message]); + connection.takeRequests(); + connection.prepare(json: UpdateMessageResult().toJson()); + await tester.tap(findButtonForLabel('Mark as unresolved')); + await tester.pumpAndSettle(); + + checkNoErrorDialog(tester); + checkRequest(message.id, 'zulip'); + }); + + testWidgets('resolve: request fails', (tester) async { + final message = eg.streamMessage(stream: someChannel, topic: 'zulip'); + await prepare(topic: 'zulip'); + await showFromRecipientHeader(tester, message: message); + connection.takeRequests(); + connection.prepare(exception: http.ClientException('Oops')); + await tester.tap(findButtonForLabel('Mark as resolved')); + await tester.pumpAndSettle(); + checkRequest(message.id, '✔ zulip'); + + checkErrorDialog(tester, + expectedTitle: 'Failed to mark topic as resolved'); + }); + + testWidgets('unresolve: request fails', (tester) async { + final message = eg.streamMessage(stream: someChannel, topic: '✔ zulip'); + await prepare(topic: '✔ zulip'); + await showFromRecipientHeader(tester, message: message); + connection.takeRequests(); + connection.prepare(exception: http.ClientException('Oops')); + await tester.tap(findButtonForLabel('Mark as unresolved')); + await tester.pumpAndSettle(); + checkRequest(message.id, 'zulip'); + + checkErrorDialog(tester, + expectedTitle: 'Failed to mark topic as unresolved'); + }); + }); }); group('message action sheet', () { diff --git a/test/widgets/test_app.dart b/test/widgets/test_app.dart index 4c76fbb2d8..e431aeaf23 100644 --- a/test/widgets/test_app.dart +++ b/test/widgets/test_app.dart @@ -1,6 +1,7 @@ import 'package:flutter/material.dart'; import 'package:zulip/generated/l10n/zulip_localizations.dart'; +import 'package:zulip/widgets/page.dart'; import 'package:zulip/widgets/store.dart'; import 'package:zulip/widgets/theme.dart'; @@ -77,9 +78,9 @@ class TestZulipApp extends StatelessWidget { navigatorObservers: navigatorObservers ?? const [], home: accountId != null - ? PerAccountStoreWidget(accountId: accountId!, child: child) - : child, - ); + ? PerAccountStoreWidget(accountId: accountId!, + child: PageRoot(child: child)) + : PageRoot(child: child)); })); } }