Skip to content

action_sheet: Implement resolve/unresolve in topic action sheet #1301

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 3 commits into from
Feb 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file modified assets/icons/ZulipIcons.ttf
Binary file not shown.
3 changes: 3 additions & 0 deletions assets/icons/check.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 4 additions & 0 deletions assets/icons/check_remove.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
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
116 changes: 103 additions & 13 deletions lib/widgets/action_sheet.dart
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import 'emoji_reaction.dart';
import 'icons.dart';
import 'inset_shadow.dart';
import 'message_list.dart';
import 'page.dart';
import 'store.dart';
import 'text.dart';
import 'theme.dart';
Expand Down Expand Up @@ -163,11 +164,19 @@ 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 store = PerAccountStoreWidget.of(context);
final pageContext = PageRoot.contextOf(context);

final store = PerAccountStoreWidget.of(pageContext);
final subscription = store.subscriptions[channelId];

final optionButtons = <ActionSheetMenuItemButton>[];
Expand Down Expand Up @@ -237,9 +246,15 @@ void showTopicActionSheet(BuildContext context, {
currentVisibilityPolicy: visibilityPolicy,
newVisibilityPolicy: to,
narrow: TopicNarrow(channelId, topic),
pageContext: 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 All @@ -250,7 +265,7 @@ void showTopicActionSheet(BuildContext context, {
return;
}

_showActionSheet(context, optionButtons: optionButtons);
_showActionSheet(pageContext, optionButtons: optionButtons);
}

class UserTopicUpdateButton extends ActionSheetMenuItemButton {
Expand Down Expand Up @@ -372,37 +387,112 @@ 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.
void showMessageActionSheet({required BuildContext context, required Message message}) {
final store = PerAccountStoreWidget.of(context);
final pageContext = PageRoot.contextOf(context);
final store = PerAccountStoreWidget.of(pageContext);

// The UI that's conditioned on this won't live-update during this appearance
// of the action sheet (we avoid calling composeBoxControllerOf in a build
// method; see its doc).
// So we rely on the fact that isComposeBoxOffered for any given message list
// will be constant through the page's life.
final messageListPage = MessageListPage.ancestorOf(context);
final messageListPage = MessageListPage.ancestorOf(pageContext);
final isComposeBoxOffered = messageListPage.composeBoxController != null;

final isMessageRead = message.flags.contains(MessageFlag.read);
final markAsUnreadSupported = store.connection.zulipFeatureLevel! >= 155; // TODO(server-6)
final showMarkAsUnreadButton = markAsUnreadSupported && isMessageRead;

final optionButtons = [
ReactionButtons(message: message, pageContext: context),
StarButton(message: message, pageContext: context),
ReactionButtons(message: message, pageContext: pageContext),
StarButton(message: message, pageContext: pageContext),
if (isComposeBoxOffered)
QuoteAndReplyButton(message: message, pageContext: context),
QuoteAndReplyButton(message: message, pageContext: pageContext),
if (showMarkAsUnreadButton)
MarkAsUnreadButton(message: message, pageContext: context),
CopyMessageTextButton(message: message, pageContext: context),
CopyMessageLinkButton(message: message, pageContext: context),
ShareButton(message: message, pageContext: context),
MarkAsUnreadButton(message: message, pageContext: pageContext),
CopyMessageTextButton(message: message, pageContext: pageContext),
CopyMessageLinkButton(message: message, pageContext: pageContext),
ShareButton(message: message, pageContext: pageContext),
];

_showActionSheet(context, optionButtons: optionButtons);
_showActionSheet(pageContext, optionButtons: optionButtons);
}

abstract class MessageActionSheetMenuItemButton extends ActionSheetMenuItemButton {
Expand Down
Loading