Skip to content

Add "Copy to clipboard" button to message action sheet #213

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 7 commits into from
Aug 1, 2023
43 changes: 43 additions & 0 deletions lib/model/binding.dart
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import 'package:device_info_plus/device_info_plus.dart' as device_info_plus;
import 'package:flutter/foundation.dart';
import 'package:url_launcher/url_launcher.dart' as url_launcher;
export 'package:url_launcher/url_launcher.dart' show LaunchMode;
Expand Down Expand Up @@ -72,6 +73,38 @@ abstract class ZulipBinding {
Uri url, {
url_launcher.LaunchMode mode = url_launcher.LaunchMode.platformDefault,
});

/// Provides device and operating system information,
/// via package:device_info_plus.
///
/// This wraps [device_info_plus.DeviceInfoPlugin.deviceInfo].
Future<BaseDeviceInfo> deviceInfo();
}

/// Like [device_info_plus.BaseDeviceInfo], but without things we don't use.
abstract class BaseDeviceInfo {
BaseDeviceInfo();
}

/// Like [device_info_plus.AndroidDeviceInfo], but without things we don't use.
class AndroidDeviceInfo extends BaseDeviceInfo {
/// The Android SDK version.
///
/// Possible values are defined in:
/// https://developer.android.com/reference/android/os/Build.VERSION_CODES.html
final int sdkInt;

AndroidDeviceInfo({required this.sdkInt});
}

/// Like [device_info_plus.IosDeviceInfo], but without things we don't use.
class IosDeviceInfo extends BaseDeviceInfo {
/// The current operating system version.
///
/// See: https://developer.apple.com/documentation/uikit/uidevice/1620043-systemversion
final String systemVersion;

IosDeviceInfo({required this.systemVersion});
}

/// A concrete binding for use in the live application.
Expand Down Expand Up @@ -103,4 +136,14 @@ class LiveZulipBinding extends ZulipBinding {
}) {
return url_launcher.launchUrl(url, mode: mode);
}

@override
Future<BaseDeviceInfo> deviceInfo() async {
final deviceInfo = await device_info_plus.DeviceInfoPlugin().deviceInfo;
return switch (deviceInfo) {
device_info_plus.AndroidDeviceInfo(:var version) => AndroidDeviceInfo(sdkInt: version.sdkInt),
device_info_plus.IosDeviceInfo(:var systemVersion) => IosDeviceInfo(systemVersion: systemVersion),
_ => throw UnimplementedError(),
};
}
}
126 changes: 89 additions & 37 deletions lib/widgets/action_sheet.dart
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import 'package:flutter/material.dart';
import 'package:flutter/services.dart';
import 'package:share_plus/share_plus.dart';

import '../api/exception.dart';
import '../api/model/model.dart';
import '../api/route/messages.dart';
import 'clipboard.dart';
import 'compose_box.dart';
import 'dialog.dart';
import 'draggable_scrollable_modal_bottom_sheet.dart';
Expand All @@ -28,6 +30,7 @@ void showMessageActionSheet({required BuildContext context, required Message mes
message: message,
messageListContext: context,
),
CopyButton(message: message, messageListContext: context),
]);
});
}
Expand Down Expand Up @@ -87,6 +90,53 @@ class ShareButton extends MessageActionSheetMenuItemButton {
};
}

/// Fetch and return the raw Markdown content for [messageId],
/// showing an error dialog on failure.
Future<String?> fetchRawContentWithFeedback({
required BuildContext context,
required int messageId,
required String errorDialogTitle,
}) async {
Message? fetchedMessage;
String? errorMessage;
// TODO, supported by reusable code:
// - (?) Retry with backoff on plausibly transient errors.
// - If request(s) take(s) a long time, show snackbar with cancel
// button, like "Still working on quote-and-reply…".
// On final failure or success, auto-dismiss the snackbar.
try {
fetchedMessage = await getMessageCompat(PerAccountStoreWidget.of(context).connection,
messageId: messageId,
applyMarkdown: false,
);
if (fetchedMessage == null) {
errorMessage = 'That message does not seem to exist.';
}
} catch (e) {
switch (e) {
case ZulipApiException():
errorMessage = e.message;
// TODO specific messages for common errors, like network errors
// (support with reusable code)
default:
errorMessage = 'Could not fetch message source.';
}
}

if (!context.mounted) return null;

if (fetchedMessage == null) {
assert(errorMessage != null);
// TODO(?) give no feedback on error conditions we expect to
// flag centrally in event polling, like invalid auth,
// user/realm deactivated. (Support with reusable code.)
await showErrorDialog(context: context,
title: errorDialogTitle, message: errorMessage);
}

return fetchedMessage?.content;
}

class QuoteAndReplyButton extends MessageActionSheetMenuItemButton {
QuoteAndReplyButton({
super.key,
Expand Down Expand Up @@ -121,42 +171,11 @@ class QuoteAndReplyButton extends MessageActionSheetMenuItemButton {
message: message,
);

Message? fetchedMessage;
String? errorMessage;
// TODO, supported by reusable code:
// - (?) Retry with backoff on plausibly transient errors.
// - If request(s) take(s) a long time, show snackbar with cancel
// button, like "Still working on quote-and-reply…".
// On final failure or success, auto-dismiss the snackbar.
try {
fetchedMessage = await getMessageCompat(PerAccountStoreWidget.of(messageListContext).connection,
messageId: message.id,
applyMarkdown: false,
);
if (fetchedMessage == null) {
errorMessage = 'That message does not seem to exist.';
}
} catch (e) {
switch (e) {
case ZulipApiException():
errorMessage = e.message;
// TODO specific messages for common errors, like network errors
// (support with reusable code)
default:
errorMessage = 'Could not fetch message source.';
}
}

if (!messageListContext.mounted) return;

if (fetchedMessage == null) {
assert(errorMessage != null);
// TODO(?) give no feedback on error conditions we expect to
// flag centrally in event polling, like invalid auth,
// user/realm deactivated. (Support with reusable code.)
await showErrorDialog(context: messageListContext,
title: 'Quotation failed', message: errorMessage);
}
final rawContent = await fetchRawContentWithFeedback(
context: messageListContext,
messageId: message.id,
errorDialogTitle: 'Quotation failed',
);

if (!messageListContext.mounted) return;

Expand All @@ -167,10 +186,43 @@ class QuoteAndReplyButton extends MessageActionSheetMenuItemButton {
composeBoxController.contentController
.registerQuoteAndReplyEnd(PerAccountStoreWidget.of(messageListContext), tag,
message: message,
rawContent: fetchedMessage?.content,
rawContent: rawContent,
);
if (!composeBoxController.contentFocusNode.hasFocus) {
composeBoxController.contentFocusNode.requestFocus();
}
};
}

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

@override get icon => Icons.copy;

@override get label => 'Copy message text';

@override get onPressed => (BuildContext context) async {
// Close the message action sheet. We won't be showing request progress,
// but hopefully it won't take long at all, and
// fetchRawContentWithFeedback has a TODO for giving feedback if it does.
Navigator.of(context).pop();

final rawContent = await fetchRawContentWithFeedback(
context: messageListContext,
messageId: message.id,
errorDialogTitle: 'Copying failed',
);

if (rawContent == null) return;

if (!messageListContext.mounted) return;

// TODO(i18n)
copyWithPopup(context: context, successContent: const Text('Message copied'),
data: ClipboardData(text: rawContent));
};
}
31 changes: 12 additions & 19 deletions lib/widgets/clipboard.dart
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import 'package:device_info_plus/device_info_plus.dart';
import 'package:flutter/material.dart';
import 'package:flutter/services.dart';

import '../model/binding.dart';

/// Copies [data] to the clipboard and shows a popup on success.
///
/// Must have a [Scaffold] ancestor.
Expand All @@ -17,26 +18,18 @@ void copyWithPopup({
required Widget successContent,
}) async {
await Clipboard.setData(data);
final deviceInfo = await DeviceInfoPlugin().deviceInfo;

// https://github.com/dart-lang/linter/issues/4007
// ignore: use_build_context_synchronously
if (!context.mounted) {
return;
}
final deviceInfo = await ZulipBinding.instance.deviceInfo();

final bool shouldShowSnackbar;
switch (deviceInfo) {
case AndroidDeviceInfo(:var version):
// Android 13+ shows its own popup on copying to the clipboard,
// so we suppress ours, following the advice at:
// https://developer.android.com/develop/ui/views/touch-and-input/copy-paste#duplicate-notifications
// TODO(android-sdk-33): Simplify this and dartdoc
shouldShowSnackbar = version.sdkInt <= 32;
default:
shouldShowSnackbar = true;
}
if (!context.mounted) return;

final shouldShowSnackbar = switch (deviceInfo) {
// Android 13+ shows its own popup on copying to the clipboard,
// so we suppress ours, following the advice at:
// https://developer.android.com/develop/ui/views/touch-and-input/copy-paste#duplicate-notifications
// TODO(android-sdk-33): Simplify this and dartdoc
AndroidDeviceInfo(:var sdkInt) => sdkInt <= 32,
_ => true,
};
if (shouldShowSnackbar) {
ScaffoldMessenger.of(context).showSnackBar(
SnackBar(behavior: SnackBarBehavior.floating, content: successContent));
Expand Down
4 changes: 4 additions & 0 deletions test/flutter_checks.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,11 @@ import 'dart:ui';
import 'package:checks/checks.dart';
import 'package:flutter/foundation.dart';
import 'package:flutter/painting.dart';
import 'package:flutter/services.dart';

extension ClipboardDataChecks on Subject<ClipboardData> {
Subject<String?> get text => has((d) => d.text, 'text');
}

extension ValueNotifierChecks<T> on Subject<ValueNotifier<T>> {
Subject<T> get value => has((c) => c.value, 'value');
Expand Down
12 changes: 12 additions & 0 deletions test/model/binding.dart
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ class TestZulipBinding extends ZulipBinding {

launchUrlResult = true;
_launchUrlCalls = null;
deviceInfoResult = _defaultDeviceInfoResult;
}

/// The current global store offered to a [GlobalStoreWidget].
Expand Down Expand Up @@ -126,4 +127,15 @@ class TestZulipBinding extends ZulipBinding {
(_launchUrlCalls ??= []).add((url: url, mode: mode));
return launchUrlResult;
}

/// The value that `ZulipBinding.instance.deviceInfo()` should return.
///
/// See also [takeDeviceInfoCalls].
BaseDeviceInfo deviceInfoResult = _defaultDeviceInfoResult;
static final _defaultDeviceInfoResult = AndroidDeviceInfo(sdkInt: 33);

@override
Future<BaseDeviceInfo> deviceInfo() {
return Future(() => deviceInfoResult);
}
}
27 changes: 27 additions & 0 deletions test/test_clipboard.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import 'package:flutter/services.dart';

// Inspired by MockClipboard in test code in the Flutter tree:
// https://github.com/flutter/flutter/blob/de26ad0a8/packages/flutter/test/widgets/clipboard_utils.dart
class MockClipboard {
MockClipboard();

dynamic clipboardData;

Future<Object?> handleMethodCall(MethodCall methodCall) async {
Copy link
Member

Choose a reason for hiding this comment

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

This is based on some piece of test code in the Flutter tree, right? Worth a brief comment pointing to there — the code is pretty gnarly-looking with all those dynamics, and that's helpful context for understanding it.

switch (methodCall.method) {
case 'Clipboard.getData':
return clipboardData;
case 'Clipboard.hasStrings':
final clipboardDataMap = clipboardData as Map<String, dynamic>?;
final text = clipboardDataMap?['text'] as String?;
return {'value': text != null && text.isNotEmpty};
case 'Clipboard.setData':
clipboardData = methodCall.arguments;
default:
if (methodCall.method.startsWith('Clipboard.')) {
throw UnimplementedError();
}
}
return null;
}
}
Loading