Skip to content

Commit

Permalink
action_sheet: Implement resolve/unresolve in topic action sheet
Browse files Browse the repository at this point in the history
Fixes: zulip#744
  • Loading branch information
chrisbobbe committed Feb 7, 2025
1 parent 432edd5 commit 76f8a16
Show file tree
Hide file tree
Showing 14 changed files with 359 additions and 10 deletions.
16 changes: 16 additions & 0 deletions assets/l10n/app_en.arb
Original file line number Diff line number Diff line change
Expand Up @@ -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."
Expand Down
6 changes: 6 additions & 0 deletions lib/api/model/model.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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, ''));
Expand Down
24 changes: 24 additions & 0 deletions lib/generated/l10n/zulip_localizations.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
12 changes: 12 additions & 0 deletions lib/generated/l10n/zulip_localizations_ar.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';

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

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

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

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

Expand Down
12 changes: 12 additions & 0 deletions lib/generated/l10n/zulip_localizations_ru.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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 => 'Скопировать текст сообщения';

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

Expand Down
84 changes: 84 additions & 0 deletions lib/widgets/action_sheet.dart
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,13 @@ 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 pageContext = PageRoot.contextOf(context);

Expand Down Expand Up @@ -245,6 +249,12 @@ void showTopicActionSheet(BuildContext 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
Expand Down Expand Up @@ -377,6 +387,80 @@ 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.
Expand Down
7 changes: 5 additions & 2 deletions lib/widgets/inbox.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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]!;
Expand All @@ -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),
Expand Down
20 changes: 17 additions & 3 deletions lib/widgets/message_list.dart
Original file line number Diff line number Diff line change
Expand Up @@ -404,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,
Expand Down Expand Up @@ -1127,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(
Expand Down
Loading

0 comments on commit 76f8a16

Please sign in to comment.