Skip to content

Commit

Permalink
action_sheet: Add channel action sheet with mark as read option
Browse files Browse the repository at this point in the history
Fixes: zulip#1226
  • Loading branch information
chimnayajith committed Feb 7, 2025
1 parent cd70171 commit 26973af
Show file tree
Hide file tree
Showing 13 changed files with 232 additions and 0 deletions.
4 changes: 4 additions & 0 deletions assets/l10n/app_en.arb
Original file line number Diff line number Diff line change
Expand Up @@ -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."
Expand Down
6 changes: 6 additions & 0 deletions lib/generated/l10n/zulip_localizations.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
3 changes: 3 additions & 0 deletions lib/generated/l10n/zulip_localizations_ar.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down
3 changes: 3 additions & 0 deletions lib/generated/l10n/zulip_localizations_en.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down
3 changes: 3 additions & 0 deletions lib/generated/l10n/zulip_localizations_ja.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down
3 changes: 3 additions & 0 deletions lib/generated/l10n/zulip_localizations_nb.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down
3 changes: 3 additions & 0 deletions lib/generated/l10n/zulip_localizations_pl.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down
3 changes: 3 additions & 0 deletions lib/generated/l10n/zulip_localizations_ru.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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 => 'Отключить тему';

Expand Down
3 changes: 3 additions & 0 deletions lib/generated/l10n/zulip_localizations_sk.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down
52 changes: 52 additions & 0 deletions lib/widgets/action_sheet.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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 = <ActionSheetMenuItemButton>[];
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,
Expand Down
13 changes: 13 additions & 0 deletions lib/widgets/inbox.dart
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,7 @@ abstract class _HeaderItem extends StatelessWidget {
}

Future<void> onRowTap();
Future<void> onLongPress();

@override
Widget build(BuildContext context) {
Expand All @@ -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,
Expand Down Expand Up @@ -330,6 +332,12 @@ class _AllDmsHeaderItem extends _HeaderItem {
pageState.allDmsCollapsed = !collapsed;
}
@override Future<void> onRowTap() => onCollapseButtonTap(); // TODO open all-DMs narrow?

@override
Future<void> onLongPress() async {
// TODO(#1272) action sheet for DM conversation
return;
}
}

class _AllDmsSection extends StatelessWidget {
Expand Down Expand Up @@ -464,6 +472,11 @@ class _StreamHeaderItem extends _HeaderItem {
}
}
@override Future<void> onRowTap() => onCollapseButtonTap(); // TODO open channel narrow

@override
Future<void> onLongPress() async {
showChannelActionSheet(sectionContext, streamId: subscription.streamId);
}
}

class _StreamSection extends StatelessWidget {
Expand Down
2 changes: 2 additions & 0 deletions lib/widgets/subscription_list.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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(
Expand Down
134 changes: 134 additions & 0 deletions test/widgets/action_sheet_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -1085,6 +1086,139 @@ void main() {
});
});
});

group('channel action sheet', () {
late ZulipStream someChannel;
late PerAccountStore store;

Future<void> 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<void> 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<void> 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<http.Request>()
..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<UnicodeEmojiWidget> {
Expand Down

0 comments on commit 26973af

Please sign in to comment.