Skip to content

Commit 191a571

Browse files
chimnayajithgnprice
authored andcommitted
action_sheet: Add channel action sheet with mark as read option
Fixes: #1226
1 parent 870bae6 commit 191a571

14 files changed

+327
-25
lines changed

assets/l10n/app_en.arb

+4
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,10 @@
7676
"@permissionsDeniedReadExternalStorage": {
7777
"description": "Message for dialog asking the user to grant permissions for external storage read access."
7878
},
79+
"actionSheetOptionMarkChannelAsRead": "Mark channel as read",
80+
"@actionSheetOptionMarkChannelAsRead": {
81+
"description": "Label for marking a channel as read."
82+
},
7983
"actionSheetOptionMuteTopic": "Mute topic",
8084
"@actionSheetOptionMuteTopic": {
8185
"description": "Label for muting a topic on action sheet."

lib/generated/l10n/zulip_localizations.dart

+6
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,12 @@ abstract class ZulipLocalizations {
219219
/// **'To upload files, please grant Zulip additional permissions in Settings.'**
220220
String get permissionsDeniedReadExternalStorage;
221221

222+
/// Label for marking a channel as read.
223+
///
224+
/// In en, this message translates to:
225+
/// **'Mark channel as read'**
226+
String get actionSheetOptionMarkChannelAsRead;
227+
222228
/// Label for muting a topic on action sheet.
223229
///
224230
/// In en, this message translates to:

lib/generated/l10n/zulip_localizations_ar.dart

+3
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,9 @@ class ZulipLocalizationsAr extends ZulipLocalizations {
6767
@override
6868
String get permissionsDeniedReadExternalStorage => 'To upload files, please grant Zulip additional permissions in Settings.';
6969

70+
@override
71+
String get actionSheetOptionMarkChannelAsRead => 'Mark channel as read';
72+
7073
@override
7174
String get actionSheetOptionMuteTopic => 'Mute topic';
7275

lib/generated/l10n/zulip_localizations_en.dart

+3
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,9 @@ class ZulipLocalizationsEn extends ZulipLocalizations {
6767
@override
6868
String get permissionsDeniedReadExternalStorage => 'To upload files, please grant Zulip additional permissions in Settings.';
6969

70+
@override
71+
String get actionSheetOptionMarkChannelAsRead => 'Mark channel as read';
72+
7073
@override
7174
String get actionSheetOptionMuteTopic => 'Mute topic';
7275

lib/generated/l10n/zulip_localizations_ja.dart

+3
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,9 @@ class ZulipLocalizationsJa extends ZulipLocalizations {
6767
@override
6868
String get permissionsDeniedReadExternalStorage => 'To upload files, please grant Zulip additional permissions in Settings.';
6969

70+
@override
71+
String get actionSheetOptionMarkChannelAsRead => 'Mark channel as read';
72+
7073
@override
7174
String get actionSheetOptionMuteTopic => 'Mute topic';
7275

lib/generated/l10n/zulip_localizations_nb.dart

+3
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,9 @@ class ZulipLocalizationsNb extends ZulipLocalizations {
6767
@override
6868
String get permissionsDeniedReadExternalStorage => 'To upload files, please grant Zulip additional permissions in Settings.';
6969

70+
@override
71+
String get actionSheetOptionMarkChannelAsRead => 'Mark channel as read';
72+
7073
@override
7174
String get actionSheetOptionMuteTopic => 'Mute topic';
7275

lib/generated/l10n/zulip_localizations_pl.dart

+3
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,9 @@ class ZulipLocalizationsPl extends ZulipLocalizations {
6767
@override
6868
String get permissionsDeniedReadExternalStorage => 'Aby odebrać pliki Zulip musi uzyskać dodatkowe uprawnienia w Ustawieniach.';
6969

70+
@override
71+
String get actionSheetOptionMarkChannelAsRead => 'Mark channel as read';
72+
7073
@override
7174
String get actionSheetOptionMuteTopic => 'Wycisz wątek';
7275

lib/generated/l10n/zulip_localizations_ru.dart

+3
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,9 @@ class ZulipLocalizationsRu extends ZulipLocalizations {
6767
@override
6868
String get permissionsDeniedReadExternalStorage => 'Для загрузки файлов, пожалуйста, предоставьте Zulip дополнительные разрешения в настройках.';
6969

70+
@override
71+
String get actionSheetOptionMarkChannelAsRead => 'Mark channel as read';
72+
7073
@override
7174
String get actionSheetOptionMuteTopic => 'Отключить тему';
7275

lib/generated/l10n/zulip_localizations_sk.dart

+3
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,9 @@ class ZulipLocalizationsSk extends ZulipLocalizations {
6767
@override
6868
String get permissionsDeniedReadExternalStorage => 'To upload files, please grant Zulip additional permissions in Settings.';
6969

70+
@override
71+
String get actionSheetOptionMarkChannelAsRead => 'Mark channel as read';
72+
7073
@override
7174
String get actionSheetOptionMuteTopic => 'Stlmiť tému';
7275

lib/widgets/action_sheet.dart

+51
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,57 @@ class ActionSheetCancelButton extends StatelessWidget {
163163
}
164164
}
165165

166+
/// Show a sheet of actions you can take on a channel.
167+
///
168+
/// Needs a [PageRoot] ancestor.
169+
void showChannelActionSheet(BuildContext context, {
170+
required int channelId,
171+
}) {
172+
final pageContext = PageRoot.contextOf(context);
173+
final store = PerAccountStoreWidget.of(pageContext);
174+
175+
final optionButtons = <ActionSheetMenuItemButton>[];
176+
final unreadCount = store.unreads.countInChannelNarrow(channelId);
177+
if (unreadCount > 0) {
178+
optionButtons.add(
179+
MarkChannelAsReadButton(pageContext: pageContext, channelId: channelId));
180+
}
181+
if (optionButtons.isEmpty) {
182+
// TODO(a11y): This case makes a no-op gesture handler; as a consequence,
183+
// we're presenting some UI (to people who use screen-reader software) as
184+
// though it offers a gesture interaction that it doesn't meaningfully
185+
// offer, which is confusing. The solution here is probably to remove this
186+
// is-empty case by having at least one button that's always present,
187+
// such as "copy link to channel".
188+
return;
189+
}
190+
_showActionSheet(pageContext, optionButtons: optionButtons);
191+
}
192+
193+
class MarkChannelAsReadButton extends ActionSheetMenuItemButton {
194+
const MarkChannelAsReadButton({
195+
super.key,
196+
required this.channelId,
197+
required super.pageContext,
198+
});
199+
200+
final int channelId;
201+
202+
@override
203+
IconData get icon => ZulipIcons.message_checked;
204+
205+
@override
206+
String label(ZulipLocalizations zulipLocalizations) {
207+
return zulipLocalizations.actionSheetOptionMarkChannelAsRead;
208+
}
209+
210+
@override
211+
void onPressed() async {
212+
final narrow = ChannelNarrow(channelId);
213+
await ZulipAction.markNarrowAsRead(pageContext, narrow);
214+
}
215+
}
216+
166217
/// Show a sheet of actions you can take on a topic.
167218
///
168219
/// Needs a [PageRoot] ancestor.

lib/widgets/inbox.dart

+15-1
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,9 @@ abstract class _HeaderItem extends StatelessWidget {
272272
// But that's in tension with the Figma, which gives these header rows
273273
// 40px min height.
274274
onTap: onCollapseButtonTap,
275+
onLongPress: this is _LongPressable
276+
? (this as _LongPressable).onLongPress
277+
: null,
275278
child: Row(crossAxisAlignment: CrossAxisAlignment.center, children: [
276279
Padding(padding: const EdgeInsets.all(10),
277280
child: Icon(size: 20, color: designVariables.sectionCollapseIcon,
@@ -425,7 +428,13 @@ class _DmItem extends StatelessWidget {
425428
}
426429
}
427430

428-
class _StreamHeaderItem extends _HeaderItem {
431+
mixin _LongPressable on _HeaderItem {
432+
// TODO(#1272) move to _HeaderItem base class
433+
// when DM headers become long-pressable; remove mixin
434+
Future<void> onLongPress();
435+
}
436+
437+
class _StreamHeaderItem extends _HeaderItem with _LongPressable {
429438
final Subscription subscription;
430439

431440
const _StreamHeaderItem({
@@ -458,6 +467,11 @@ class _StreamHeaderItem extends _HeaderItem {
458467
}
459468
}
460469
@override Future<void> onRowTap() => onCollapseButtonTap(); // TODO open channel narrow
470+
471+
@override
472+
Future<void> onLongPress() async {
473+
showChannelActionSheet(sectionContext, channelId: subscription.streamId);
474+
}
461475
}
462476

463477
class _StreamSection extends StatelessWidget {

lib/widgets/message_list.dart

+43-24
Original file line numberDiff line numberDiff line change
@@ -370,36 +370,54 @@ class MessageListAppBarTitle extends StatelessWidget {
370370
case ChannelNarrow(:var streamId):
371371
final store = PerAccountStoreWidget.of(context);
372372
final stream = store.streams[streamId];
373-
return _buildStreamRow(context, stream: stream);
374-
375-
case TopicNarrow(:var streamId, :var topic):
376-
final store = PerAccountStoreWidget.of(context);
377-
final stream = store.streams[streamId];
373+
final alignment = willCenterTitle
374+
? Alignment.center
375+
: AlignmentDirectional.centerStart;
378376
return SizedBox(
379377
width: double.infinity,
380378
child: GestureDetector(
381379
behavior: HitTestBehavior.translucent,
382380
onLongPress: () {
383-
final someMessage = MessageListPage.ancestorOf(context)
384-
.model?.messages.firstOrNull;
385-
// If someMessage is null, the topic action sheet won't have a
386-
// resolve/unresolve button. That seems OK; in that case we're
387-
// either still fetching messages (and the user can reopen the
388-
// sheet after that finishes) or there aren't any messages to
389-
// act on anyway.
390-
assert(someMessage == null || narrow.containsMessage(someMessage));
391-
showTopicActionSheet(context,
392-
channelId: streamId,
393-
topic: topic,
394-
someMessageIdInTopic: someMessage?.id);
381+
showChannelActionSheet(context, channelId: streamId);
395382
},
396-
child: Column(
397-
crossAxisAlignment: willCenterTitle ? CrossAxisAlignment.center
398-
: CrossAxisAlignment.start,
399-
children: [
400-
_buildStreamRow(context, stream: stream),
401-
_buildTopicRow(context, stream: stream, topic: topic),
402-
])));
383+
child: Align(alignment: alignment,
384+
child: _buildStreamRow(context, stream: stream))));
385+
386+
case TopicNarrow(:var streamId, :var topic):
387+
final store = PerAccountStoreWidget.of(context);
388+
final stream = store.streams[streamId];
389+
final alignment = willCenterTitle
390+
? Alignment.center
391+
: AlignmentDirectional.centerStart;
392+
return Column(
393+
crossAxisAlignment: CrossAxisAlignment.stretch,
394+
children: [
395+
GestureDetector(
396+
behavior: HitTestBehavior.translucent,
397+
onLongPress: () {
398+
showChannelActionSheet(context, channelId: streamId);
399+
},
400+
child: Align(alignment: alignment,
401+
child: _buildStreamRow(context, stream: stream))),
402+
GestureDetector(
403+
behavior: HitTestBehavior.translucent,
404+
onLongPress: () {
405+
final someMessage = MessageListPage.ancestorOf(context)
406+
.model?.messages.firstOrNull;
407+
// If someMessage is null, the topic action sheet won't have a
408+
// resolve/unresolve button. That seems OK; in that case we're
409+
// either still fetching messages (and the user can reopen the
410+
// sheet after that finishes) or there aren't any messages to
411+
// act on anyway.
412+
assert(someMessage == null || narrow.containsMessage(someMessage));
413+
showTopicActionSheet(context,
414+
channelId: streamId,
415+
topic: topic,
416+
someMessageIdInTopic: someMessage?.id);
417+
},
418+
child: Align(alignment: alignment,
419+
child: _buildTopicRow(context, stream: stream, topic: topic))),
420+
]);
403421

404422
case DmNarrow(:var otherRecipientIds):
405423
final store = PerAccountStoreWidget.of(context);
@@ -1061,6 +1079,7 @@ class StreamMessageRecipientHeader extends StatelessWidget {
10611079
onTap: () => Navigator.push(context,
10621080
MessageListPage.buildRoute(context: context,
10631081
narrow: ChannelNarrow(message.streamId))),
1082+
onLongPress: () => showChannelActionSheet(context, channelId: message.streamId),
10641083
child: Row(
10651084
crossAxisAlignment: CrossAxisAlignment.center,
10661085
children: [

lib/widgets/subscription_list.dart

+2
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import '../api/model/model.dart';
44
import '../generated/l10n/zulip_localizations.dart';
55
import '../model/narrow.dart';
66
import '../model/unreads.dart';
7+
import 'action_sheet.dart';
78
import 'icons.dart';
89
import 'message_list.dart';
910
import 'store.dart';
@@ -230,6 +231,7 @@ class SubscriptionItem extends StatelessWidget {
230231
MessageListPage.buildRoute(context: context,
231232
narrow: ChannelNarrow(subscription.streamId)));
232233
},
234+
onLongPress: () => showChannelActionSheet(context, channelId: subscription.streamId),
233235
child: Row(crossAxisAlignment: CrossAxisAlignment.center, children: [
234236
const SizedBox(width: 16),
235237
Padding(

0 commit comments

Comments
 (0)