diff --git a/lib/api/model/initial_snapshot.dart b/lib/api/model/initial_snapshot.dart index 5882122baa..054230a256 100644 --- a/lib/api/model/initial_snapshot.dart +++ b/lib/api/model/initial_snapshot.dart @@ -79,6 +79,9 @@ class InitialSnapshot { /// https://zulip.com/api/roles-and-permissions#determining-if-a-user-is-a-full-member final int realmWaitingPeriodThreshold; + final bool realmAllowMessageEditing; + final int? realmMessageContentEditLimitSeconds; + final Map realmDefaultExternalAccounts; final int maxFileUploadSizeMib; @@ -137,6 +140,8 @@ class InitialSnapshot { required this.realmWildcardMentionPolicy, required this.realmMandatoryTopics, required this.realmWaitingPeriodThreshold, + required this.realmAllowMessageEditing, + required this.realmMessageContentEditLimitSeconds, required this.realmDefaultExternalAccounts, required this.maxFileUploadSizeMib, required this.serverEmojiDataUrl, diff --git a/lib/api/model/initial_snapshot.g.dart b/lib/api/model/initial_snapshot.g.dart index 79cfbe5557..570d7c2bba 100644 --- a/lib/api/model/initial_snapshot.g.dart +++ b/lib/api/model/initial_snapshot.g.dart @@ -73,6 +73,9 @@ InitialSnapshot _$InitialSnapshotFromJson( realmMandatoryTopics: json['realm_mandatory_topics'] as bool, realmWaitingPeriodThreshold: (json['realm_waiting_period_threshold'] as num).toInt(), + realmAllowMessageEditing: json['realm_allow_message_editing'] as bool, + realmMessageContentEditLimitSeconds: + (json['realm_message_content_edit_limit_seconds'] as num?)?.toInt(), realmDefaultExternalAccounts: (json['realm_default_external_accounts'] as Map).map( (k, e) => MapEntry( @@ -133,6 +136,9 @@ Map _$InitialSnapshotToJson(InitialSnapshot instance) => 'realm_wildcard_mention_policy': instance.realmWildcardMentionPolicy, 'realm_mandatory_topics': instance.realmMandatoryTopics, 'realm_waiting_period_threshold': instance.realmWaitingPeriodThreshold, + 'realm_allow_message_editing': instance.realmAllowMessageEditing, + 'realm_message_content_edit_limit_seconds': + instance.realmMessageContentEditLimitSeconds, 'realm_default_external_accounts': instance.realmDefaultExternalAccounts, 'max_file_upload_size_mib': instance.maxFileUploadSizeMib, 'server_emoji_data_url': instance.serverEmojiDataUrl?.toString(), diff --git a/lib/model/message.dart b/lib/model/message.dart index 962d79eeab..fd5de1adbd 100644 --- a/lib/model/message.dart +++ b/lib/model/message.dart @@ -63,6 +63,18 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore { assert(removed); } + void _notifyMessageListViewsForOneMessage(int messageId) { + for (final view in _messageListViews) { + view.notifyListenersIfMessagePresent(messageId); + } + } + + void _notifyMessageListViews(Iterable messageIds) { + for (final view in _messageListViews) { + view.notifyListenersIfAnyMessagePresent(messageIds); + } + } + void reassemble() { for (final view in _messageListViews) { view.reassemble(); @@ -142,9 +154,7 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore { _handleUpdateMessageEventTimestamp(event); _handleUpdateMessageEventContent(event); _handleUpdateMessageEventMove(event); - for (final view in _messageListViews) { - view.notifyListenersIfAnyMessagePresent(event.messageIds); - } + _notifyMessageListViews(event.messageIds); } void _handleUpdateMessageEventTimestamp(UpdateMessageEvent event) { @@ -268,17 +278,15 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore { : message.flags.remove(event.flag); } if (anyMessageFound) { - for (final view in _messageListViews) { - view.notifyListenersIfAnyMessagePresent(event.messages); - // TODO(#818): Support MentionsNarrow live-updates when handling - // @-mention flags. - - // To make it easier to re-star a message, we opt-out from supporting - // live-updates when starred flag is removed. - // - // TODO: Support StarredMessagesNarrow live-updates when starred flag - // is added. - } + // TODO(#818): Support MentionsNarrow live-updates when handling + // @-mention flags. + + // To make it easier to re-star a message, we opt-out from supporting + // live-updates when starred flag is removed. + // + // TODO: Support StarredMessagesNarrow live-updates when starred flag + // is added. + _notifyMessageListViews(event.messages); } } } @@ -305,10 +313,7 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore { userId: event.userId, ); } - - for (final view in _messageListViews) { - view.notifyListenersIfMessagePresent(event.messageId); - } + _notifyMessageListViewsForOneMessage(event.messageId); } void handleSubmessageEvent(SubmessageEvent event) { diff --git a/lib/model/store.dart b/lib/model/store.dart index 049ee73922..939120113e 100644 --- a/lib/model/store.dart +++ b/lib/model/store.dart @@ -472,6 +472,8 @@ class PerAccountStore extends PerAccountStoreBase with ChangeNotifier, EmojiStor realmWaitingPeriodThreshold: initialSnapshot.realmWaitingPeriodThreshold, maxFileUploadSizeMib: initialSnapshot.maxFileUploadSizeMib, realmEmptyTopicDisplayName: initialSnapshot.realmEmptyTopicDisplayName, + realmAllowMessageEditing: initialSnapshot.realmAllowMessageEditing, + realmMessageContentEditLimitSeconds: initialSnapshot.realmMessageContentEditLimitSeconds, realmDefaultExternalAccounts: initialSnapshot.realmDefaultExternalAccounts, customProfileFields: _sortCustomProfileFields(initialSnapshot.customProfileFields), emailAddressVisibility: initialSnapshot.emailAddressVisibility, @@ -509,6 +511,8 @@ class PerAccountStore extends PerAccountStoreBase with ChangeNotifier, EmojiStor required this.realmWaitingPeriodThreshold, required this.maxFileUploadSizeMib, required String? realmEmptyTopicDisplayName, + required this.realmAllowMessageEditing, + required this.realmMessageContentEditLimitSeconds, required this.realmDefaultExternalAccounts, required this.customProfileFields, required this.emailAddressVisibility, @@ -563,6 +567,8 @@ class PerAccountStore extends PerAccountStoreBase with ChangeNotifier, EmojiStor final bool realmMandatoryTopics; // TODO(#668): update this realm setting /// For docs, please see [InitialSnapshot.realmWaitingPeriodThreshold]. final int realmWaitingPeriodThreshold; // TODO(#668): update this realm setting + final bool realmAllowMessageEditing; // TODO(#668): update this realm setting + final int? realmMessageContentEditLimitSeconds; // TODO(#668): update this realm setting final int maxFileUploadSizeMib; // No event for this. /// The display name to use for empty topics. diff --git a/lib/widgets/app.dart b/lib/widgets/app.dart index d2f5b36e1d..54ba92588b 100644 --- a/lib/widgets/app.dart +++ b/lib/widgets/app.dart @@ -308,16 +308,17 @@ class ChooseAccountPage extends StatelessWidget { trailing: MenuAnchor( menuChildren: [ MenuItemButton( - onPressed: () { - showSuggestedActionDialog(context: context, + onPressed: () async { + final dialog = showSuggestedActionDialog(context: context, title: zulipLocalizations.logOutConfirmationDialogTitle, message: zulipLocalizations.logOutConfirmationDialogMessage, // TODO(#1032) "destructive" style for action button - actionButtonText: zulipLocalizations.logOutConfirmationDialogConfirmButton, - onActionButtonPress: () { - // TODO error handling if db write fails? - logOutAccount(GlobalStoreWidget.of(context), accountId); - }); + actionButtonText: zulipLocalizations.logOutConfirmationDialogConfirmButton); + if (await dialog.result == true) { + if (!context.mounted) return; + // TODO error handling if db write fails? + unawaited(logOutAccount(GlobalStoreWidget.of(context), accountId)); + } }, child: Text(zulipLocalizations.chooseAccountPageLogOutButton)), ], diff --git a/lib/widgets/compose_box.dart b/lib/widgets/compose_box.dart index aabf592dae..005321719d 100644 --- a/lib/widgets/compose_box.dart +++ b/lib/widgets/compose_box.dart @@ -1,3 +1,4 @@ +import 'dart:async'; import 'dart:math'; import 'package:app_settings/app_settings.dart'; @@ -907,13 +908,13 @@ Future> _getFilePickerFiles(BuildContext context, FileType type) // If the user hasn't checked "Don't ask again", they can always dismiss // our prompt and retry, and the permissions request will reappear, // letting them grant permissions and complete the upload. - showSuggestedActionDialog(context: context, + final dialog = showSuggestedActionDialog(context: context, title: zulipLocalizations.permissionsNeededTitle, message: zulipLocalizations.permissionsDeniedReadExternalStorage, - actionButtonText: zulipLocalizations.permissionsNeededOpenSettings, - onActionButtonPress: () { - AppSettings.openAppSettings(); - }); + actionButtonText: zulipLocalizations.permissionsNeededOpenSettings); + if (await dialog.result == true) { + unawaited(AppSettings.openAppSettings()); + } } else { showErrorDialog(context: context, title: zulipLocalizations.errorDialogTitle, @@ -1008,13 +1009,13 @@ class _AttachFromCameraButton extends _AttachUploadsButton { // permission-request alert once, the first time the app wants to // use a protected resource. After that, the only way the user can // grant it is in Settings. - showSuggestedActionDialog(context: context, + final dialog = showSuggestedActionDialog(context: context, title: zulipLocalizations.permissionsNeededTitle, message: zulipLocalizations.permissionsDeniedCameraAccess, - actionButtonText: zulipLocalizations.permissionsNeededOpenSettings, - onActionButtonPress: () { - AppSettings.openAppSettings(); - }); + actionButtonText: zulipLocalizations.permissionsNeededOpenSettings); + if (await dialog.result == true) { + unawaited(AppSettings.openAppSettings()); + } } else { showErrorDialog(context: context, title: zulipLocalizations.errorDialogTitle, @@ -1400,7 +1401,13 @@ abstract class _Banner extends StatelessWidget { Color getLabelColor(DesignVariables designVariables); Color getBackgroundColor(DesignVariables designVariables); - /// A trailing element, with no outer padding for spacing/positioning. + /// A trailing element, with vertical but not horizontal outer padding + /// for spacing/positioning. + /// + /// An interactive element's touchable area should have height at least 44px, + /// with some of that as "slop" vertical outer padding above and below + /// what gets painted: + /// https://github.com/zulip/zulip-flutter/pull/1432#discussion_r2023907300 /// /// To control the element's distance from the end edge, override [padEnd]. Widget? buildTrailing(BuildContext context); @@ -1431,13 +1438,15 @@ abstract class _Banner extends StatelessWidget { // (SafeArea.minimum doesn't take an EdgeInsetsDirectional) .resolve(Directionality.of(context)), child: Padding( - padding: const EdgeInsetsDirectional.fromSTEB(8, 5, 0, 5), + padding: const EdgeInsetsDirectional.only(start: 8), child: Row( children: [ Expanded( child: Padding( - padding: const EdgeInsets.symmetric(vertical: 4), - child: Text(style: labelTextStyle, + padding: const EdgeInsets.symmetric(vertical: 9), + child: Text( + style: labelTextStyle, + textScaler: MediaQuery.textScalerOf(context).clamp(maxScaleFactor: 1.5), getLabel(zulipLocalizations)))), if (trailing != null) ...[ const SizedBox(width: 8), diff --git a/lib/widgets/dialog.dart b/lib/widgets/dialog.dart index 7ff373db6d..0d2b4c5a7d 100644 --- a/lib/widgets/dialog.dart +++ b/lib/widgets/dialog.dart @@ -19,25 +19,41 @@ Widget _dialogActionText(String text) { /// Tracks the status of a dialog, in being still open or already closed. /// +/// Use [T] to identify the outcome of the interaction: +/// - Pass `void` for an informational dialog with just the option to dismiss. +/// - For confirmation dialogs with an option to dismiss +/// plus an option to proceed with an action, pass `bool`. +/// The action button should pass true for Navigator.pop's `result` argument. +/// - For dialogs with an option to dismiss plus multiple other options, +/// pass a custom enum. +/// For the latter two cases, a cancel button should call Navigator.pop +/// with null for the `result` argument, to match what Flutter does +/// when you dismiss the dialog by tapping outside its area. +/// /// See also: /// * [showDialog], whose return value this class is intended to wrap. -class DialogStatus { - const DialogStatus(this.closed); +class DialogStatus { + const DialogStatus(this.result); /// Resolves when the dialog is closed. - final Future closed; + /// + /// If this completes with null, the dialog was dismissed. + /// Otherwise, completes with a [T] identifying the interaction's outcome. + /// + /// See, e.g., [showSuggestedActionDialog]. + final Future result; } /// Displays an [AlertDialog] with a dismiss button /// and optional "Learn more" button. /// -/// The [DialogStatus.closed] field of the return value can be used +/// The [DialogStatus.result] field of the return value can be used /// for waiting for the dialog to be closed. // This API is inspired by [ScaffoldManager.showSnackBar]. We wrap // [showDialog]'s return value, a [Future], inside [DialogStatus] // whose documentation can be accessed. This helps avoid confusion when // intepreting the meaning of the [Future]. -DialogStatus showErrorDialog({ +DialogStatus showErrorDialog({ required BuildContext context, required String title, String? message, @@ -61,28 +77,31 @@ DialogStatus showErrorDialog({ return DialogStatus(future); } -void showSuggestedActionDialog({ +/// Displays an alert dialog with a cancel button and an action button. +/// +/// The [DialogStatus.result] Future gives true if the action button was tapped. +/// If the dialog was canceled, +/// either with the cancel button or by tapping outside the dialog's area, +/// it completes with null. +DialogStatus showSuggestedActionDialog({ required BuildContext context, required String title, required String message, required String? actionButtonText, - required VoidCallback onActionButtonPress, }) { final zulipLocalizations = ZulipLocalizations.of(context); - showDialog( + final future = showDialog( context: context, builder: (BuildContext context) => AlertDialog( title: Text(title), content: SingleChildScrollView(child: Text(message)), actions: [ TextButton( - onPressed: () => Navigator.pop(context), + onPressed: () => Navigator.pop(context, null), child: _dialogActionText(zulipLocalizations.dialogCancel)), TextButton( - onPressed: () { - onActionButtonPress(); - Navigator.pop(context); - }, + onPressed: () => Navigator.pop(context, true), child: _dialogActionText(actionButtonText ?? zulipLocalizations.dialogContinue)), ])); + return DialogStatus(future); } diff --git a/lib/widgets/lightbox.dart b/lib/widgets/lightbox.dart index 0ddad1ef38..a66e4137dd 100644 --- a/lib/widgets/lightbox.dart +++ b/lib/widgets/lightbox.dart @@ -484,7 +484,7 @@ class _VideoLightboxPageState extends State with PerAccountSt context: context, title: zulipLocalizations.errorDialogTitle, message: zulipLocalizations.errorVideoPlayerFailed); - await dialog.closed; + await dialog.result; if (!mounted) return; Navigator.pop(context); // Pops the lightbox } diff --git a/test/example_data.dart b/test/example_data.dart index afa378aa7e..f31337d303 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -916,6 +916,8 @@ InitialSnapshot initialSnapshot({ RealmWildcardMentionPolicy? realmWildcardMentionPolicy, bool? realmMandatoryTopics, int? realmWaitingPeriodThreshold, + bool? realmAllowMessageEditing, + int? realmMessageContentEditLimitSeconds, Map? realmDefaultExternalAccounts, int? maxFileUploadSizeMib, Uri? serverEmojiDataUrl, @@ -953,6 +955,8 @@ InitialSnapshot initialSnapshot({ realmWildcardMentionPolicy: realmWildcardMentionPolicy ?? RealmWildcardMentionPolicy.everyone, realmMandatoryTopics: realmMandatoryTopics ?? true, realmWaitingPeriodThreshold: realmWaitingPeriodThreshold ?? 0, + realmAllowMessageEditing: realmAllowMessageEditing ?? true, + realmMessageContentEditLimitSeconds: realmMessageContentEditLimitSeconds ?? 600, realmDefaultExternalAccounts: realmDefaultExternalAccounts ?? {}, maxFileUploadSizeMib: maxFileUploadSizeMib ?? 25, serverEmojiDataUrl: serverEmojiDataUrl diff --git a/test/widgets/dialog_test.dart b/test/widgets/dialog_test.dart index 4082e45424..c86aae478e 100644 --- a/test/widgets/dialog_test.dart +++ b/test/widgets/dialog_test.dart @@ -26,4 +26,51 @@ void main() { mode: LaunchMode.inAppBrowserView)); }); }); + + group('showSuggestedActionDialog', () { + testWidgets('tap action button', (tester) async { + addTearDown(testBinding.reset); + await tester.pumpWidget(TestZulipApp()); + await tester.pump(); + final element = tester.element(find.byType(Placeholder)); + + final dialog = showSuggestedActionDialog(context: element, + title: 'Continue?', + message: 'Do the thing?', + actionButtonText: 'Sure'); + await tester.pump(); + await tester.tap(find.text('Sure')); + await check(dialog.result).completes((it) => it.equals(true)); + }); + + testWidgets('tap cancel', (tester) async { + addTearDown(testBinding.reset); + await tester.pumpWidget(TestZulipApp()); + await tester.pump(); + final element = tester.element(find.byType(Placeholder)); + + final dialog = showSuggestedActionDialog(context: element, + title: 'Continue?', + message: 'Do the thing?', + actionButtonText: 'Sure'); + await tester.pump(); + await tester.tap(find.text('Cancel')); + await check(dialog.result).completes((it) => it.equals(null)); + }); + + testWidgets('tap outside dialog area', (tester) async { + addTearDown(testBinding.reset); + await tester.pumpWidget(TestZulipApp()); + await tester.pump(); + final element = tester.element(find.byType(Placeholder)); + + final dialog = showSuggestedActionDialog(context: element, + title: 'Continue?', + message: 'Do the thing?', + actionButtonText: 'Sure'); + await tester.pump(); + await tester.tapAt(tester.getTopLeft(find.byType(TestZulipApp))); + await check(dialog.result).completes((it) => it.equals(null)); + }); + }); }