Skip to content

Edit-message (4/n): Misc. preparation for edit-message feature #1471

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
Apr 18, 2025
5 changes: 5 additions & 0 deletions lib/api/model/initial_snapshot.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, RealmDefaultExternalAccount> realmDefaultExternalAccounts;

final int maxFileUploadSizeMib;
Expand Down Expand Up @@ -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,
Expand Down
6 changes: 6 additions & 0 deletions lib/api/model/initial_snapshot.g.dart

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

41 changes: 23 additions & 18 deletions lib/model/message.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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<int> messageIds) {
for (final view in _messageListViews) {
view.notifyListenersIfAnyMessagePresent(messageIds);
}
}

void reassemble() {
for (final view in _messageListViews) {
view.reassemble();
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
}
}
}
Expand All @@ -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) {
Expand Down
6 changes: 6 additions & 0 deletions lib/model/store.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down
15 changes: 8 additions & 7 deletions lib/widgets/app.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
],
Expand Down
37 changes: 23 additions & 14 deletions lib/widgets/compose_box.dart
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import 'dart:async';
import 'dart:math';

import 'package:app_settings/app_settings.dart';
Expand Down Expand Up @@ -907,13 +908,13 @@ Future<Iterable<_File>> _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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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),
Expand Down
45 changes: 32 additions & 13 deletions lib/widgets/dialog.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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<T> {
Copy link
Member

Choose a reason for hiding this comment

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

It might be helpful to document the meaning of the type parameter. Specially, from showDialog, this part seems relevant:

/// If the application has multiple [Navigator] objects, it may be necessary to
/// call `Navigator.of(context, rootNavigator: true).pop(result)` to close the
/// dialog rather than just `Navigator.pop(context, result)`.
///
/// Returns a [Future] that resolves to the value (if any) that was passed to
/// [Navigator.pop] when the dialog was closed.

We just need to adapt it to fit our wrapper class.

const DialogStatus(this.result);

/// Resolves when the dialog is closed.
final Future<void> 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<T?> 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<void> showErrorDialog({
required BuildContext context,
required String title,
String? message,
Expand All @@ -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<bool> showSuggestedActionDialog({
required BuildContext context,
required String title,
required String message,
required String? actionButtonText,
required VoidCallback onActionButtonPress,
}) {
final zulipLocalizations = ZulipLocalizations.of(context);
showDialog<void>(
final future = showDialog<bool>(
context: context,
builder: (BuildContext context) => AlertDialog(
title: Text(title),
content: SingleChildScrollView(child: Text(message)),
actions: [
TextButton(
onPressed: () => Navigator.pop(context),
onPressed: () => Navigator.pop<bool>(context, null),
child: _dialogActionText(zulipLocalizations.dialogCancel)),
TextButton(
onPressed: () {
onActionButtonPress();
Navigator.pop(context);
},
onPressed: () => Navigator.pop<bool>(context, true),
child: _dialogActionText(actionButtonText ?? zulipLocalizations.dialogContinue)),
]));
return DialogStatus(future);
}
2 changes: 1 addition & 1 deletion lib/widgets/lightbox.dart
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,7 @@ class _VideoLightboxPageState extends State<VideoLightboxPage> 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
}
Expand Down
4 changes: 4 additions & 0 deletions test/example_data.dart
Original file line number Diff line number Diff line change
Expand Up @@ -916,6 +916,8 @@ InitialSnapshot initialSnapshot({
RealmWildcardMentionPolicy? realmWildcardMentionPolicy,
bool? realmMandatoryTopics,
int? realmWaitingPeriodThreshold,
bool? realmAllowMessageEditing,
int? realmMessageContentEditLimitSeconds,
Map<String, RealmDefaultExternalAccount>? realmDefaultExternalAccounts,
int? maxFileUploadSizeMib,
Uri? serverEmojiDataUrl,
Expand Down Expand Up @@ -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
Expand Down
Loading