Skip to content

msglist: Add "Copy message link" button to action sheet #713

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.

Sign up for GitHub

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 5 commits into from
Jul 2, 2024
Merged

msglist: Add "Copy message link" button to action sheet #713

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e55675e
action-sheet: Rename `CopyButton` to `CopyMessageTextButton`
sm-sayedi May 22, 2024
63b57fe
action-sheet: Pass the correct `BuildContext` for showing `SnackBar`
sm-sayedi Jun 6, 2024
aeb6a94
action-sheet test: Simplify and explain a bit more the #732 repro test
gnprice Jul 2, 2024
191f4e3
action-sheet: Move the "Share" option to the bottom of options
sm-sayedi Jun 6, 2024
22c0f3e
action-sheet: Add "Copy message link" button
sm-sayedi May 30, 2024
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
16 changes: 12 additions & 4 deletions assets/l10n/app_en.arb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,14 @@
"@permissionsDeniedReadExternalStorage": {
"description": "Message for dialog asking the user to grant permissions for external storage read access."
},
"actionSheetOptionCopy": "Copy message text",
"@actionSheetOptionCopy": {
"actionSheetOptionCopyMessageText": "Copy message text",
"@actionSheetOptionCopyMessageText": {
Comment on lines -46 to +47
Copy link
Collaborator

Choose a reason for hiding this comment

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

Cool. I think I'd like to rename successMessageCopied to successMessageTextCopied, too.

I think it would also be good to make that UI string more parallel to this existing "Copy message text" button. Instead of "Message Copied" (which has the wrong capitalization anyway), it could say "Message text copied".

Changing the user-facing text won't be NFC, so if it goes in this commit, the commit should be unmarked [nfc]. I don't think I have a preference if that change goes here or in a different commit.

"description": "Label for copy message text button on action sheet."
},
"actionSheetOptionCopyMessageLink": "Copy link to message",
"@actionSheetOptionCopyMessageLink": {
"description": "Label for copy message link button on action sheet."
},
"actionSheetOptionShare": "Share",
"@actionSheetOptionShare": {
"description": "Label for share button on action sheet."
Expand Down Expand Up @@ -168,10 +172,14 @@
"@successLinkCopied": {
"description": "Success message after copy link action completed."
},
"successMessageCopied": "Message Copied",
"@successMessageCopied": {
"successMessageTextCopied": "Message text copied",
"@successMessageTextCopied": {
"description": "Message when content of a message was copied to the user's system clipboard."
},
"successMessageLinkCopied": "Message link copied",
"@successMessageLinkCopied": {
"description": "Message when link of a message was copied to the user's system clipboard."
},
"composeBoxAttachFilesTooltip": "Attach files",
"@composeBoxAttachFilesTooltip": {
"description": "Tooltip for compose box icon to attach a file to the message."
Expand Down
162 changes: 98 additions & 64 deletions lib/widgets/action_sheet.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import 'package:share_plus/share_plus.dart';
import '../api/exception.dart';
import '../api/model/model.dart';
import '../api/route/messages.dart';
import '../model/internal_link.dart';
import '../model/narrow.dart';
import 'clipboard.dart';
import 'compose_box.dart';
import 'dialog.dart';
Expand Down Expand Up @@ -39,12 +41,13 @@ void showMessageActionSheet({required BuildContext context, required Message mes
return Column(children: [
if (!hasThumbsUpReactionVote) AddThumbsUpButton(message: message, messageListContext: context),
StarButton(message: message, messageListContext: context),
ShareButton(message: message, messageListContext: context),
if (isComposeBoxOffered) QuoteAndReplyButton(
message: message,
messageListContext: context,
),
CopyButton(message: message, messageListContext: context),
CopyMessageTextButton(message: message, messageListContext: context),
CopyMessageLinkButton(message: message, messageListContext: context),
ShareButton(message: message, messageListContext: context),
]);
});
}
Expand Down Expand Up @@ -164,63 +167,6 @@ class StarButton extends MessageActionSheetMenuItemButton {
}
}

class ShareButton extends MessageActionSheetMenuItemButton {
ShareButton({
super.key,
required super.message,
required super.messageListContext,
});

@override IconData get icon => Icons.adaptive.share;

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

@override void onPressed(BuildContext context) async {
// Close the message action sheet; we're about to show the share
// sheet. (We could do this after the sharing Future settles
// with [ShareResultStatus.success], but on iOS I get impatient with
// how slowly our action sheet dismisses in that case.)
// TODO(#24): Fix iOS bug where this call causes the keyboard to
// reopen (if it was open at the time of this
// `showMessageActionSheet` call) and cover a large part of the
// share sheet.
Navigator.of(context).pop();
final zulipLocalizations = ZulipLocalizations.of(messageListContext);

final rawContent = await fetchRawContentWithFeedback(
context: messageListContext,
messageId: message.id,
errorDialogTitle: zulipLocalizations.errorSharingFailed,
);

if (rawContent == null) return;

if (!messageListContext.mounted) return;

// TODO: to support iPads, we're asked to give a
// `sharePositionOrigin` param, or risk crashing / hanging:
// https://pub.dev/packages/share_plus#ipad
// Perhaps a wart in the API; discussion:
// https://github.com/zulip/zulip-flutter/pull/12#discussion_r1130146231
final result = await Share.share(rawContent);

switch (result.status) {
// The plugin isn't very helpful: "The status can not be determined".
// Until we learn otherwise, assume something wrong happened.
case ShareResultStatus.unavailable:
if (!messageListContext.mounted) return;
await showErrorDialog(context: messageListContext,
title: zulipLocalizations.errorSharingFailed);
case ShareResultStatus.success:
case ShareResultStatus.dismissed:
// nothing to do
}
}
}

/// Fetch and return the raw Markdown content for [messageId],
/// showing an error dialog on failure.
Future<String?> fetchRawContentWithFeedback({
Expand Down Expand Up @@ -330,8 +276,8 @@ class QuoteAndReplyButton extends MessageActionSheetMenuItemButton {
}
}

class CopyButton extends MessageActionSheetMenuItemButton {
CopyButton({
class CopyMessageTextButton extends MessageActionSheetMenuItemButton {
CopyMessageTextButton({
super.key,
required super.message,
required super.messageListContext,
Expand All @@ -341,7 +287,7 @@ class CopyButton extends MessageActionSheetMenuItemButton {

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

@override void onPressed(BuildContext context) async {
Expand All @@ -361,8 +307,96 @@ class CopyButton extends MessageActionSheetMenuItemButton {

if (!messageListContext.mounted) return;

copyWithPopup(context: context,
successContent: Text(zulipLocalizations.successMessageCopied),
copyWithPopup(context: messageListContext,
successContent: Text(zulipLocalizations.successMessageTextCopied),
data: ClipboardData(text: rawContent));
}
}

class CopyMessageLinkButton extends MessageActionSheetMenuItemButton {
CopyMessageLinkButton({
super.key,
required super.message,
required super.messageListContext,
});

@override IconData get icon => Icons.link;

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

@override void onPressed(BuildContext context) {
Navigator.of(context).pop();
final zulipLocalizations = ZulipLocalizations.of(messageListContext);

final store = PerAccountStoreWidget.of(messageListContext);
final messageLink = narrowLink(
store,
SendableNarrow.ofMessage(message, selfUserId: store.selfUserId),
nearMessageId: message.id,
);

copyWithPopup(context: messageListContext,
successContent: Text(zulipLocalizations.successMessageLinkCopied),
data: ClipboardData(text: messageLink.toString()));
}
}

class ShareButton extends MessageActionSheetMenuItemButton {
ShareButton({
super.key,
required super.message,
required super.messageListContext,
});

@override IconData get icon => Icons.adaptive.share;

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

@override void onPressed(BuildContext context) async {
// Close the message action sheet; we're about to show the share
// sheet. (We could do this after the sharing Future settles
// with [ShareResultStatus.success], but on iOS I get impatient with
// how slowly our action sheet dismisses in that case.)
// TODO(#24): Fix iOS bug where this call causes the keyboard to
// reopen (if it was open at the time of this
// `showMessageActionSheet` call) and cover a large part of the
// share sheet.
Navigator.of(context).pop();
final zulipLocalizations = ZulipLocalizations.of(messageListContext);

final rawContent = await fetchRawContentWithFeedback(
context: messageListContext,
messageId: message.id,
errorDialogTitle: zulipLocalizations.errorSharingFailed,
);

if (rawContent == null) return;

if (!messageListContext.mounted) return;

// TODO: to support iPads, we're asked to give a
// `sharePositionOrigin` param, or risk crashing / hanging:
// https://pub.dev/packages/share_plus#ipad
// Perhaps a wart in the API; discussion:
// https://github.com/zulip/zulip-flutter/pull/12#discussion_r1130146231
final result = await Share.share(rawContent);

switch (result.status) {
// The plugin isn't very helpful: "The status can not be determined".
// Until we learn otherwise, assume something wrong happened.
case ShareResultStatus.unavailable:
if (!messageListContext.mounted) return;
await showErrorDialog(context: messageListContext,
title: zulipLocalizations.errorSharingFailed);
case ShareResultStatus.success:
case ShareResultStatus.dismissed:
// nothing to do
}
}
}
Loading
Loading