Skip to content

action_sheet: Add "Mark as unread from here" button #779

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Aug 13, 2024
Merged
19 changes: 19 additions & 0 deletions assets/l10n/app_en.arb
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@
"@actionSheetOptionCopyMessageLink": {
"description": "Label for copy message link button on action sheet."
},
"actionSheetOptionMarkAsUnread": "Mark as unread from here",
"@actionSheetOptionMarkAsUnread": {
"description": "Label for mark as unread button on action sheet."
},
"actionSheetOptionShare": "Share",
"@actionSheetOptionShare": {
"description": "Label for share button on action sheet."
Expand Down Expand Up @@ -436,6 +440,21 @@
"@errorMarkAsReadFailedTitle": {
"description": "Error title when mark as read action failed."
},
"markAsUnreadComplete": "Marked {num, plural, =1{1 message} other{{num} messages}} as unread.",
"@markAsUnreadComplete": {
"description": "Message when marking messages as unread has completed.",
"placeholders": {
"num": {"type": "int", "example": "4"}
}
},
"markAsUnreadInProgress": "Marking messages as unread...",
"@markAsUnreadInProgress": {
"description": "Progress message when marking messages as unread."
},
"errorMarkAsUnreadFailedTitle": "Mark as unread failed",
"@errorMarkAsUnreadFailedTitle": {
"description": "Error title when mark as unread action failed."
},
"today": "Today",
"@today": {
"description": "Term to use to reference the current day."
Expand Down
33 changes: 33 additions & 0 deletions lib/widgets/action_sheet.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import '../api/model/model.dart';
import '../api/route/messages.dart';
import '../model/internal_link.dart';
import '../model/narrow.dart';
import 'actions.dart';
import 'clipboard.dart';
import 'compose_box.dart';
import 'dialog.dart';
Expand All @@ -28,6 +29,10 @@ void showMessageActionSheet({required BuildContext context, required Message mes
// any message list, so that's fine.
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;

final hasThumbsUpReactionVote = message.reactions
?.aggregated.any((reactionWithVotes) =>
Expand All @@ -46,6 +51,11 @@ void showMessageActionSheet({required BuildContext context, required Message mes
message: message,
messageListContext: 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),
Expand Down Expand Up @@ -278,6 +288,29 @@ class QuoteAndReplyButton extends MessageActionSheetMenuItemButton {
}
}

class MarkAsUnreadButton extends MessageActionSheetMenuItemButton {
MarkAsUnreadButton({
super.key,
required super.message,
required super.messageListContext,
required this.narrow,
});

final Narrow narrow;

@override IconData get icon => Icons.mark_chat_unread_outlined;

@override
String label(ZulipLocalizations zulipLocalizations) {
return zulipLocalizations.actionSheetOptionMarkAsUnread;
}

@override void onPressed(BuildContext context) async {
Navigator.of(context).pop();
markNarrowAsUnreadFromMessage(messageListContext, message, narrow);
}
}

class CopyMessageTextButton extends MessageActionSheetMenuItemButton {
CopyMessageTextButton({
super.key,
Expand Down
239 changes: 156 additions & 83 deletions lib/widgets/actions.dart
Original file line number Diff line number Diff line change
Expand Up @@ -16,101 +16,174 @@ import '../model/narrow.dart';
import 'dialog.dart';
import 'store.dart';

Future<void> markNarrowAsRead(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: see previous commits for what summary-line prefixes are used on a given file, and match those

msg_list: Ensure markNarrowAsRead passes before acting on it

See https://github.com/zulip/zulip-mobile/blob/main/docs/howto/git.md for tips on reading history, including filtering to changes touching a file.

BuildContext context,
Narrow narrow,
bool useLegacy, // TODO(server-6)
) async {
Future<void> markNarrowAsRead(BuildContext context, Narrow narrow) async {
final store = PerAccountStoreWidget.of(context);
final connection = store.connection;
final zulipLocalizations = ZulipLocalizations.of(context);
final useLegacy = connection.zulipFeatureLevel! < 155; // TODO(server-6)
if (useLegacy) {
return await _legacyMarkNarrowAsRead(context, narrow);
try {
await _legacyMarkNarrowAsRead(context, narrow);
return;
} catch (e) {
if (!context.mounted) return;
await showErrorDialog(context: context,
title: zulipLocalizations.errorMarkAsReadFailedTitle,
message: e.toString()); // TODO(#741): extract user-facing message better
return;
}
}

final didPass = await updateMessageFlagsStartingFromAnchor(
context: context,
// Include `is:unread` in the narrow. That has a database index, so
// this can be an important optimization in narrows with a lot of history.
// The server applies the same optimization within the (deprecated)
// specialized endpoints for marking messages as read; see
// `do_mark_stream_messages_as_read` in `zulip:zerver/actions/message_flags.py`.
apiNarrow: narrow.apiEncode()..add(ApiNarrowIsUnread()),
// Use [AnchorCode.oldest], because [AnchorCode.firstUnread]
// will be the oldest non-muted unread message, which would
// result in muted unreads older than the first unread not
// being processed.
anchor: AnchorCode.oldest,
// [AnchorCode.oldest] is an anchor ID lower than any valid
// message ID.
includeAnchor: false,
op: UpdateMessageFlagsOp.add,
flag: MessageFlag.read,
onCompletedMessage: zulipLocalizations.markAsReadComplete,
progressMessage: zulipLocalizations.markAsReadInProgress,
onFailedTitle: zulipLocalizations.errorMarkAsReadFailedTitle);

if (!didPass || !context.mounted) return;
if (narrow is CombinedFeedNarrow) {
PerAccountStoreWidget.of(context).unreads.handleAllMessagesReadSuccess();
}
}

// Compare web's `mark_all_as_read` in web/src/unread_ops.js
// and zulip-mobile's `markAsUnreadFromMessage` in src/action-sheets/index.js .
Future<void> markNarrowAsUnreadFromMessage(
BuildContext context,
Message message,
Narrow narrow,
) async {
final connection = PerAccountStoreWidget.of(context).connection;
assert(connection.zulipFeatureLevel! >= 155); // TODO(server-6)
final zulipLocalizations = ZulipLocalizations.of(context);
final scaffoldMessenger = ScaffoldMessenger.of(context);
// Use [AnchorCode.oldest], because [AnchorCode.firstUnread]
// will be the oldest non-muted unread message, which would
// result in muted unreads older than the first unread not
// being processed.
Anchor anchor = AnchorCode.oldest;
int responseCount = 0;
int updatedCount = 0;
await updateMessageFlagsStartingFromAnchor(
context: context,
apiNarrow: narrow.apiEncode(),
anchor: NumericAnchor(message.id),
includeAnchor: true,
op: UpdateMessageFlagsOp.remove,
flag: MessageFlag.read,
onCompletedMessage: zulipLocalizations.markAsUnreadComplete,
progressMessage: zulipLocalizations.markAsUnreadInProgress,
onFailedTitle: zulipLocalizations.errorMarkAsUnreadFailedTitle);
}

// Include `is:unread` in the narrow. That has a database index, so
// this can be an important optimization in narrows with a lot of history.
// The server applies the same optimization within the (deprecated)
// specialized endpoints for marking messages as read; see
// `do_mark_stream_messages_as_read` in `zulip:zerver/actions/message_flags.py`.
final apiNarrow = narrow.apiEncode()..add(ApiNarrowIsUnread());
/// Add or remove the given flag from the anchor to the end of the narrow,
/// showing feedback to the user on progress or failure.
///
/// This has the semantics of [updateMessageFlagsForNarrow]
/// (see https://zulip.com/api/update-message-flags-for-narrow)
/// with `numBefore: 0` and infinite `numAfter`. It operates by calling that
/// endpoint with a finite `numAfter` as a batch size, in a loop.
///
/// If the operation requires more than one batch, the user is shown progress
/// feedback through [SnackBar], using [progressMessage] and [onCompletedMessage].
/// If the operation fails, the user is shown an error dialog box with title
/// [onFailedTitle].
///
/// Returns true just if the operation finished successfully.
Future<bool> updateMessageFlagsStartingFromAnchor({
required BuildContext context,
required List<ApiNarrowElement> apiNarrow,
required Anchor anchor,
required bool includeAnchor,
required UpdateMessageFlagsOp op,
required MessageFlag flag,
required String Function(int) onCompletedMessage,
required String progressMessage,
required String onFailedTitle,
}) async {
try {
final store = PerAccountStoreWidget.of(context);
final connection = store.connection;
final scaffoldMessenger = ScaffoldMessenger.of(context);

while (true) {
final result = await updateMessageFlagsForNarrow(connection,
anchor: anchor,
// [AnchorCode.oldest] is an anchor ID lower than any valid
// message ID; and follow-up requests will have already
// processed the anchor ID, so we just want this to be
// unconditionally false.
includeAnchor: false,
// There is an upper limit of 5000 messages per batch
// (numBefore + numAfter <= 5000) enforced on the server.
// See `update_message_flags_in_narrow` in zerver/views/message_flags.py .
// zulip-mobile uses `numAfter` of 5000, but web uses 1000
// for more responsive feedback. See zulip@f0d87fcf6.
numBefore: 0,
numAfter: 1000,
narrow: apiNarrow,
op: UpdateMessageFlagsOp.add,
flag: MessageFlag.read);
if (!context.mounted) {
scaffoldMessenger.clearSnackBars();
return;
}
responseCount++;
updatedCount += result.updatedCount;
// Compare web's `mark_all_as_read` in web/src/unread_ops.js
// and zulip-mobile's `markAsUnreadFromMessage` in src/action-sheets/index.js .
int responseCount = 0;
int updatedCount = 0;
while (true) {
final result = await updateMessageFlagsForNarrow(connection,
anchor: anchor,
includeAnchor: includeAnchor,
// There is an upper limit of 5000 messages per batch
// (numBefore + numAfter <= 5000) enforced on the server.
// See `update_message_flags_in_narrow` in zerver/views/message_flags.py .
// zulip-mobile uses `numAfter` of 5000, but web uses 1000
// for more responsive feedback. See zulip@f0d87fcf6.
numBefore: 0,
numAfter: 1000,
narrow: apiNarrow,
op: op,
flag: flag);
if (!context.mounted) {
scaffoldMessenger.clearSnackBars();
return false;
}
responseCount++;
updatedCount += result.updatedCount;

if (result.foundNewest) {
if (responseCount > 1) {
// We previously showed an in-progress [SnackBar], so say we're done.
// There may be a backlog of [SnackBar]s accumulated in the queue
// so be sure to clear them out here.
scaffoldMessenger
..clearSnackBars()
..showSnackBar(SnackBar(behavior: SnackBarBehavior.floating,
content: Text(zulipLocalizations.markAsReadComplete(updatedCount))));
if (result.foundNewest) {
if (responseCount > 1) {
// We previously showed an in-progress [SnackBar], so say we're done.
// There may be a backlog of [SnackBar]s accumulated in the queue
// so be sure to clear them out here.
scaffoldMessenger
..clearSnackBars()
..showSnackBar(SnackBar(behavior: SnackBarBehavior.floating,
content: Text(onCompletedMessage(updatedCount))));
}
return true;
}
return;
}

if (result.lastProcessedId == null) {
// No messages were in the range of the request.
// This should be impossible given that `foundNewest` was false
// (and that our `numAfter` was positive.)
await showErrorDialog(context: context,
title: zulipLocalizations.errorMarkAsReadFailedTitle,
message: zulipLocalizations.errorInvalidResponse);
return;
}
anchor = NumericAnchor(result.lastProcessedId!);
if (result.lastProcessedId == null) {
final zulipLocalizations = ZulipLocalizations.of(context);
// No messages were in the range of the request.
// This should be impossible given that `foundNewest` was false
// (and that our `numAfter` was positive.)
showErrorDialog(context: context,
title: onFailedTitle,
message: zulipLocalizations.errorInvalidResponse);
return false;
}
anchor = NumericAnchor(result.lastProcessedId!);
includeAnchor = false;

// The task is taking a while, so tell the user we're working on it.
// No need to say how many messages, as the [MarkAsUnread] widget
// should follow along.
// TODO: Ideally we'd have a progress widget here that showed up based
// on actual time elapsed -- so it could appear before the first
// batch returns, if that takes a while -- and that then stuck
// around continuously until the task ends. For now we use a
// series of [SnackBar]s, which may feel a bit janky.
// There is complexity in tracking the status of each [SnackBar],
// due to having no way to determine which is currently active,
// or if there is an active one at all. Resetting the [SnackBar] here
// results in the same message popping in and out and the user experience
// is better for now if we allow them to run their timer through
// and clear the backlog later.
scaffoldMessenger.showSnackBar(SnackBar(behavior: SnackBarBehavior.floating,
content: Text(zulipLocalizations.markAsReadInProgress)));
// The task is taking a while, so tell the user we're working on it.
// TODO: Ideally we'd have a progress widget here that showed up based
// on actual time elapsed -- so it could appear before the first
// batch returns, if that takes a while -- and that then stuck
// around continuously until the task ends. For now we use a
// series of [SnackBar]s, which may feel a bit janky.
// There is complexity in tracking the status of each [SnackBar],
// due to having no way to determine which is currently active,
// or if there is an active one at all. Resetting the [SnackBar] here
// results in the same message popping in and out and the user experience
// is better for now if we allow them to run their timer through
// and clear the backlog later.
scaffoldMessenger.showSnackBar(SnackBar(behavior: SnackBarBehavior.floating,
content: Text(progressMessage)));
}
} catch (e) {
if (!context.mounted) return false;
showErrorDialog(context: context,
title: onFailedTitle,
message: e.toString()); // TODO(#741): extract user-facing message better
return false;
}
}

Expand Down
24 changes: 2 additions & 22 deletions lib/widgets/message_list.dart
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import 'actions.dart';
import 'app_bar.dart';
import 'compose_box.dart';
import 'content.dart';
import 'dialog.dart';
import 'emoji_reaction.dart';
import 'icons.dart';
import 'page.dart';
Expand Down Expand Up @@ -709,28 +708,9 @@ class _MarkAsReadWidgetState extends State<MarkAsReadWidget> {

void _handlePress(BuildContext context) async {
if (!context.mounted) return;

final store = PerAccountStoreWidget.of(context);
final connection = store.connection;
final useLegacy = connection.zulipFeatureLevel! < 155;
setState(() => _loading = true);

try {
await markNarrowAsRead(context, widget.narrow, useLegacy);
} catch (e) {
if (!context.mounted) return;
final zulipLocalizations = ZulipLocalizations.of(context);
showErrorDialog(context: context,
title: zulipLocalizations.errorMarkAsReadFailedTitle,
message: e.toString()); // TODO(#741): extract user-facing message better
return;
} finally {
setState(() => _loading = false);
}
if (!context.mounted) return;
if (widget.narrow is CombinedFeedNarrow && !useLegacy) {
PerAccountStoreWidget.of(context).unreads.handleAllMessagesReadSuccess();
}
await markNarrowAsRead(context, widget.narrow);
setState(() => _loading = false);
}

@override
Expand Down
Loading