From 93d3199787a54dfecb222b193071cc4738aed208 Mon Sep 17 00:00:00 2001 From: chimnayajith Date: Sun, 2 Feb 2025 12:20:14 +0530 Subject: [PATCH] action_sheet: Add channel action sheet with mark as read option Fixes: #1226 --- assets/l10n/app_en.arb | 4 + lib/generated/l10n/zulip_localizations.dart | 6 + .../l10n/zulip_localizations_ar.dart | 3 + .../l10n/zulip_localizations_en.dart | 3 + .../l10n/zulip_localizations_ja.dart | 3 + .../l10n/zulip_localizations_nb.dart | 3 + .../l10n/zulip_localizations_pl.dart | 3 + .../l10n/zulip_localizations_ru.dart | 3 + .../l10n/zulip_localizations_sk.dart | 3 + lib/widgets/action_sheet.dart | 52 +++++++ lib/widgets/inbox.dart | 13 ++ lib/widgets/subscription_list.dart | 2 + test/widgets/action_sheet_test.dart | 134 ++++++++++++++++++ 13 files changed, 232 insertions(+) diff --git a/assets/l10n/app_en.arb b/assets/l10n/app_en.arb index ee7e96c35f8..15bb3f889c6 100644 --- a/assets/l10n/app_en.arb +++ b/assets/l10n/app_en.arb @@ -76,6 +76,10 @@ "@permissionsDeniedReadExternalStorage": { "description": "Message for dialog asking the user to grant permissions for external storage read access." }, + "actionSheetOptionMarkChannelAsRead": "Mark channel as read", + "@actionSheetOptionMarkChannelAsRead": { + "description": "Label for marking a channel as read." + }, "actionSheetOptionMuteTopic": "Mute topic", "@actionSheetOptionMuteTopic": { "description": "Label for muting a topic on action sheet." diff --git a/lib/generated/l10n/zulip_localizations.dart b/lib/generated/l10n/zulip_localizations.dart index b6fbb707698..803e00a6646 100644 --- a/lib/generated/l10n/zulip_localizations.dart +++ b/lib/generated/l10n/zulip_localizations.dart @@ -219,6 +219,12 @@ abstract class ZulipLocalizations { /// **'To upload files, please grant Zulip additional permissions in Settings.'** String get permissionsDeniedReadExternalStorage; + /// Label for marking a channel as read. + /// + /// In en, this message translates to: + /// **'Mark channel as read'** + String get actionSheetOptionMarkChannelAsRead; + /// Label for muting a topic 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 025b4b14441..091174ec000 100644 --- a/lib/generated/l10n/zulip_localizations_ar.dart +++ b/lib/generated/l10n/zulip_localizations_ar.dart @@ -67,6 +67,9 @@ class ZulipLocalizationsAr extends ZulipLocalizations { @override String get permissionsDeniedReadExternalStorage => 'To upload files, please grant Zulip additional permissions in Settings.'; + @override + String get actionSheetOptionMarkChannelAsRead => 'Mark channel as read'; + @override String get actionSheetOptionMuteTopic => 'Mute topic'; diff --git a/lib/generated/l10n/zulip_localizations_en.dart b/lib/generated/l10n/zulip_localizations_en.dart index 9467d334288..8c16ae80742 100644 --- a/lib/generated/l10n/zulip_localizations_en.dart +++ b/lib/generated/l10n/zulip_localizations_en.dart @@ -67,6 +67,9 @@ class ZulipLocalizationsEn extends ZulipLocalizations { @override String get permissionsDeniedReadExternalStorage => 'To upload files, please grant Zulip additional permissions in Settings.'; + @override + String get actionSheetOptionMarkChannelAsRead => 'Mark channel as read'; + @override String get actionSheetOptionMuteTopic => 'Mute topic'; diff --git a/lib/generated/l10n/zulip_localizations_ja.dart b/lib/generated/l10n/zulip_localizations_ja.dart index f363ee00433..bb48307719f 100644 --- a/lib/generated/l10n/zulip_localizations_ja.dart +++ b/lib/generated/l10n/zulip_localizations_ja.dart @@ -67,6 +67,9 @@ class ZulipLocalizationsJa extends ZulipLocalizations { @override String get permissionsDeniedReadExternalStorage => 'To upload files, please grant Zulip additional permissions in Settings.'; + @override + String get actionSheetOptionMarkChannelAsRead => 'Mark channel as read'; + @override String get actionSheetOptionMuteTopic => 'Mute topic'; diff --git a/lib/generated/l10n/zulip_localizations_nb.dart b/lib/generated/l10n/zulip_localizations_nb.dart index 35b3e86fe5f..e8797c28519 100644 --- a/lib/generated/l10n/zulip_localizations_nb.dart +++ b/lib/generated/l10n/zulip_localizations_nb.dart @@ -67,6 +67,9 @@ class ZulipLocalizationsNb extends ZulipLocalizations { @override String get permissionsDeniedReadExternalStorage => 'To upload files, please grant Zulip additional permissions in Settings.'; + @override + String get actionSheetOptionMarkChannelAsRead => 'Mark channel as read'; + @override String get actionSheetOptionMuteTopic => 'Mute topic'; diff --git a/lib/generated/l10n/zulip_localizations_pl.dart b/lib/generated/l10n/zulip_localizations_pl.dart index 0594722d31b..e15e8d5c036 100644 --- a/lib/generated/l10n/zulip_localizations_pl.dart +++ b/lib/generated/l10n/zulip_localizations_pl.dart @@ -67,6 +67,9 @@ class ZulipLocalizationsPl extends ZulipLocalizations { @override String get permissionsDeniedReadExternalStorage => 'Aby odebrać pliki Zulip musi uzyskać dodatkowe uprawnienia w Ustawieniach.'; + @override + String get actionSheetOptionMarkChannelAsRead => 'Mark channel as read'; + @override String get actionSheetOptionMuteTopic => 'Wycisz wątek'; diff --git a/lib/generated/l10n/zulip_localizations_ru.dart b/lib/generated/l10n/zulip_localizations_ru.dart index 879559fed4d..d3cf20ad87f 100644 --- a/lib/generated/l10n/zulip_localizations_ru.dart +++ b/lib/generated/l10n/zulip_localizations_ru.dart @@ -67,6 +67,9 @@ class ZulipLocalizationsRu extends ZulipLocalizations { @override String get permissionsDeniedReadExternalStorage => 'Для загрузки файлов, пожалуйста, предоставьте Zulip дополнительные разрешения в настройках.'; + @override + String get actionSheetOptionMarkChannelAsRead => 'Mark channel as read'; + @override String get actionSheetOptionMuteTopic => 'Отключить тему'; diff --git a/lib/generated/l10n/zulip_localizations_sk.dart b/lib/generated/l10n/zulip_localizations_sk.dart index af87dfd949e..9a3dbe7ec9f 100644 --- a/lib/generated/l10n/zulip_localizations_sk.dart +++ b/lib/generated/l10n/zulip_localizations_sk.dart @@ -67,6 +67,9 @@ class ZulipLocalizationsSk extends ZulipLocalizations { @override String get permissionsDeniedReadExternalStorage => 'To upload files, please grant Zulip additional permissions in Settings.'; + @override + String get actionSheetOptionMarkChannelAsRead => 'Mark channel as read'; + @override String get actionSheetOptionMuteTopic => 'Stlmiť tému'; diff --git a/lib/widgets/action_sheet.dart b/lib/widgets/action_sheet.dart index a227cd7742f..fc80f6648ea 100644 --- a/lib/widgets/action_sheet.dart +++ b/lib/widgets/action_sheet.dart @@ -162,6 +162,58 @@ class ActionSheetCancelButton extends StatelessWidget { } } +/// Show a sheet of actions you can take on a channel. +void showChannelActionSheet(BuildContext context, { + required int streamId, +}) { + final store = PerAccountStoreWidget.of(context); + + final optionButtons = []; + final unreadCount = store.unreads.countInChannelNarrow(streamId); + if (unreadCount > 0) { + optionButtons.add( + MarkChannelAsReadButton( + streamId: streamId, + pageContext: context, + ), + ); + } + 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 + // though it offers a gesture interaction that it doesn't meaningfully + // offer, which is confusing. The solution here is probably to remove this + // is-empty case by having at least one button that's always present, + // such as "copy link to channel". + return; + } + _showActionSheet(context, optionButtons: optionButtons); +} + +class MarkChannelAsReadButton extends ActionSheetMenuItemButton { + const MarkChannelAsReadButton({ + super.key, + required this.streamId, + required super.pageContext + }); + + final int streamId; + + @override + IconData get icon => ZulipIcons.message_checked; + + @override + String label(ZulipLocalizations zulipLocalizations) { + return zulipLocalizations.actionSheetOptionMarkChannelAsRead; + } + + @override + void onPressed() async { + final narrow = ChannelNarrow(streamId); + await ZulipAction.markNarrowAsRead(pageContext, narrow); + } +} + /// Show a sheet of actions you can take on a topic. void showTopicActionSheet(BuildContext context, { required int channelId, diff --git a/lib/widgets/inbox.dart b/lib/widgets/inbox.dart index 6dbe31ce04c..8d61147448d 100644 --- a/lib/widgets/inbox.dart +++ b/lib/widgets/inbox.dart @@ -255,6 +255,7 @@ abstract class _HeaderItem extends StatelessWidget { } Future onRowTap(); + Future onLongPress(); @override Widget build(BuildContext context) { @@ -272,6 +273,7 @@ abstract class _HeaderItem extends StatelessWidget { // But that's in tension with the Figma, which gives these header rows // 40px min height. onTap: onCollapseButtonTap, + onLongPress: onLongPress, child: Row(crossAxisAlignment: CrossAxisAlignment.center, children: [ Padding(padding: const EdgeInsets.all(10), child: Icon(size: 20, color: designVariables.sectionCollapseIcon, @@ -330,6 +332,12 @@ class _AllDmsHeaderItem extends _HeaderItem { pageState.allDmsCollapsed = !collapsed; } @override Future onRowTap() => onCollapseButtonTap(); // TODO open all-DMs narrow? + + @override + Future onLongPress() async { + // TODO(#1272) action sheet for DM conversation + return; + } } class _AllDmsSection extends StatelessWidget { @@ -464,6 +472,11 @@ class _StreamHeaderItem extends _HeaderItem { } } @override Future onRowTap() => onCollapseButtonTap(); // TODO open channel narrow + + @override + Future onLongPress() async { + showChannelActionSheet(sectionContext, streamId: subscription.streamId); + } } class _StreamSection extends StatelessWidget { diff --git a/lib/widgets/subscription_list.dart b/lib/widgets/subscription_list.dart index a51e722b515..6538d295c63 100644 --- a/lib/widgets/subscription_list.dart +++ b/lib/widgets/subscription_list.dart @@ -4,6 +4,7 @@ import '../api/model/model.dart'; import '../generated/l10n/zulip_localizations.dart'; import '../model/narrow.dart'; import '../model/unreads.dart'; +import 'action_sheet.dart'; import 'icons.dart'; import 'message_list.dart'; import 'store.dart'; @@ -230,6 +231,7 @@ class SubscriptionItem extends StatelessWidget { MessageListPage.buildRoute(context: context, narrow: ChannelNarrow(subscription.streamId))); }, + onLongPress: () => showChannelActionSheet(context, streamId: subscription.streamId), child: Row(crossAxisAlignment: CrossAxisAlignment.center, children: [ const SizedBox(width: 16), Padding( diff --git a/test/widgets/action_sheet_test.dart b/test/widgets/action_sheet_test.dart index 2bb07b49460..1c5e509a060 100644 --- a/test/widgets/action_sheet_test.dart +++ b/test/widgets/action_sheet_test.dart @@ -30,6 +30,7 @@ import 'package:zulip/widgets/icons.dart'; import 'package:zulip/widgets/inbox.dart'; import 'package:zulip/widgets/message_list.dart'; import 'package:share_plus_platform_interface/method_channel/method_channel_share.dart'; +import 'package:zulip/widgets/subscription_list.dart'; import '../api/fake_api.dart'; import '../example_data.dart' as eg; @@ -1085,6 +1086,139 @@ void main() { }); }); }); + + group('channel action sheet', () { + late ZulipStream someChannel; + late PerAccountStore store; + + Future prepare({UnreadMessagesSnapshot? unreadMsgs}) async { + final stream = eg.stream(); + someChannel = stream; + addTearDown(testBinding.reset); + + unreadMsgs ??= eg.unreadMsgs(channels: [ + eg.unreadChannelMsgs( + streamId: stream.streamId, + topic: 'topic', + unreadMessageIds: [1], + ), + ]); + + await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot( + realmUsers: [eg.selfUser, eg.otherUser], + streams: [someChannel], + subscriptions: [eg.subscription(someChannel)], + unreadMsgs: unreadMsgs)); + store = await testBinding.globalStore.perAccount(eg.selfAccount.id); + connection = store.connection as FakeApiConnection; + } + + Future showFromInbox(WidgetTester tester) async { + await tester.pumpWidget(TestZulipApp(accountId: eg.selfAccount.id, + child: const HomePage())); + await tester.pumpAndSettle(); + check(find.byType(InboxPageBody)).findsOne(); + + await tester.pump(); + await tester.longPress(find.text(someChannel.name).hitTestable()); + await tester.pump(const Duration(milliseconds: 250)); + } + + Future showFromSubscriptionList(WidgetTester tester) async { + await tester.pumpWidget(TestZulipApp(accountId: eg.selfAccount.id, + child: const SubscriptionListPageBody())); + await tester.pumpAndSettle(); + check(find.byType(SubscriptionListPageBody)).findsOne(); + + await tester.pump(); + await tester.longPress(find.text(someChannel.name).hitTestable()); + await tester.pump(const Duration(milliseconds: 250)); + } + + final actionSheetFinder = find.byType(BottomSheet); + Finder findButtonForLabel(String label) => + find.descendant(of: actionSheetFinder, matching: find.text(label)); + + void checkButton(String label) { + check(findButtonForLabel(label)).findsOne(); + } + + group('showChannelActionSheet', () { + void checkButtons() { + check(actionSheetFinder).findsOne(); + checkButton('Mark channel as read'); + } + + testWidgets('show from inbox', (tester) async { + await prepare(); + await showFromInbox(tester); + checkButtons(); + }); + + testWidgets('show from subscription list', (tester) async { + await prepare(); + await showFromSubscriptionList(tester); + checkButtons(); + }); + + testWidgets('show with no unread messages', (tester) async { + await prepare(unreadMsgs: eg.unreadMsgs()); + await showFromSubscriptionList(tester); + check(actionSheetFinder).findsNothing(); + }); + }); + + group('MarkChannelAsReadButton', () { + void checkRequest(int streamId) { + check(connection.takeRequests()).single.isA() + ..method.equals('POST') + ..url.path.equals('/api/v1/messages/flags/narrow') + ..bodyFields.deepEquals({ + 'anchor': 'oldest', + 'include_anchor': 'false', + 'num_before': '0', + 'num_after': '1000', + 'narrow': jsonEncode([ + {'operator': 'stream', 'operand': streamId}, + {'operator': 'is', 'operand': 'unread'} + ]), + 'op': 'add', + 'flag': 'read', + }); + } + + testWidgets('happy path from inbox', (tester) async { + await prepare(); + await showFromInbox(tester); + connection.prepare(json: UpdateMessageFlagsForNarrowResult( + processedCount: 1, updatedCount: 1, + firstProcessedId: null, lastProcessedId: null, + foundOldest: true, foundNewest: true).toJson()); + await tester.tap(findButtonForLabel('Mark channel as read')); + await tester.pumpAndSettle(); + + checkNoErrorDialog(tester); + checkRequest(someChannel.streamId); + }); + + testWidgets('request fails', (tester) async { + await prepare(); + await showFromInbox(tester); + + // Prepare error response + connection.prepare(httpStatus: 400, json: { + 'result': 'error', 'code': 'BAD_REQUEST', 'msg': ''}); + + // Tap and wait for dialog + await tester.tap(findButtonForLabel('Mark channel as read')); + await tester.pump(); // + await tester.pumpAndSettle(); // Wait for dialog animation + + checkErrorDialog(tester, + expectedTitle: "Mark as read failed"); + }); + }); + }); } extension UnicodeEmojiWidgetChecks on Subject {