From 131a4e9e6431f68ec042511ff8d52c3ec0365ad8 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 13 May 2025 22:48:45 -0700 Subject: [PATCH 01/12] compose_box test: Use full MessageListPage in compose box widget tests --- test/widgets/compose_box_test.dart | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/test/widgets/compose_box_test.dart b/test/widgets/compose_box_test.dart index f979c687de..3ee28a053e 100644 --- a/test/widgets/compose_box_test.dart +++ b/test/widgets/compose_box_test.dart @@ -20,6 +20,7 @@ import 'package:zulip/model/typing_status.dart'; import 'package:zulip/widgets/app.dart'; import 'package:zulip/widgets/color.dart'; import 'package:zulip/widgets/compose_box.dart'; +import 'package:zulip/widgets/message_list.dart'; import 'package:zulip/widgets/page.dart'; import 'package:zulip/widgets/icons.dart'; import 'package:zulip/widgets/theme.dart'; @@ -69,15 +70,12 @@ void main() { connection = store.connection as FakeApiConnection; + connection.prepare(json: + eg.newestGetMessagesResult(foundOldest: true, messages: []).toJson()); await tester.pumpWidget(TestZulipApp(accountId: selfAccount.id, - child: Column( - // This positions the compose box at the bottom of the screen, - // simulating the layout of the message list page. - children: [ - const Expanded(child: SizedBox.expand()), - ComposeBox(narrow: narrow), - ]))); + child: MessageListPage(initNarrow: narrow))); await tester.pumpAndSettle(); + connection.takeRequests(); controller = tester.state(find.byType(ComposeBox)).controller; } @@ -1334,6 +1332,14 @@ void main() { await enterContent(tester, 'some content'); checkContentInputValue(tester, 'some content'); + // Encache a new connection; prepare it for the message-list fetch + final newConnection = (testBinding.globalStore + ..clearCachedApiConnections() + ..useCachedApiConnections = true) + .apiConnectionFromAccount(store.account) as FakeApiConnection; + newConnection.prepare(json: + eg.newestGetMessagesResult(foundOldest: true, messages: []).toJson()); + store.updateMachine! ..debugPauseLoop() ..poll() @@ -1341,6 +1347,7 @@ void main() { eg.apiExceptionBadEventQueueId(queueId: store.queueId)) ..debugAdvanceLoop(); await tester.pump(); + await tester.pump(Duration.zero); final newStore = testBinding.globalStore.perAccountSync(store.accountId)!; check(newStore) From 9e27a118f1cf53830acfa2fee6f06b33856b104c Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Thu, 1 May 2025 23:34:53 -0700 Subject: [PATCH 02/12] msglist [nfc]: s/localizations/zulipLocalizations/ --- lib/widgets/message_list.dart | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index 3e8850f5cc..8c280eac1c 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -817,16 +817,16 @@ class _TypingStatusWidgetState extends State with PerAccount if (narrow is! SendableNarrow) return const SizedBox(); final store = PerAccountStoreWidget.of(context); - final localizations = ZulipLocalizations.of(context); + final zulipLocalizations = ZulipLocalizations.of(context); final typistIds = model!.typistIdsInNarrow(narrow); if (typistIds.isEmpty) return const SizedBox(); final text = switch (typistIds.length) { - 1 => localizations.onePersonTyping( + 1 => zulipLocalizations.onePersonTyping( store.userDisplayName(typistIds.first)), - 2 => localizations.twoPeopleTyping( + 2 => zulipLocalizations.twoPeopleTyping( store.userDisplayName(typistIds.first), store.userDisplayName(typistIds.last)), - _ => localizations.manyPeopleTyping, + _ => zulipLocalizations.manyPeopleTyping, }; return Padding( @@ -1452,13 +1452,13 @@ class MessageWithPossibleSender extends StatelessWidget { final designVariables = DesignVariables.of(context); final message = item.message; - final localizations = ZulipLocalizations.of(context); + final zulipLocalizations = ZulipLocalizations.of(context); String? editStateText; switch (message.editState) { case MessageEditState.edited: - editStateText = localizations.messageIsEditedLabel; + editStateText = zulipLocalizations.messageIsEditedLabel; case MessageEditState.moved: - editStateText = localizations.messageIsMovedLabel; + editStateText = zulipLocalizations.messageIsMovedLabel; case MessageEditState.none: } From bf38693bc094da7b1c002b6c452f57a7352e2096 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 5 May 2025 12:48:47 -0700 Subject: [PATCH 03/12] compose_box test [nfc]: Pull out sendButtonFinder helper --- test/widgets/compose_box_test.dart | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/test/widgets/compose_box_test.dart b/test/widgets/compose_box_test.dart index 3ee28a053e..24c4d403b3 100644 --- a/test/widgets/compose_box_test.dart +++ b/test/widgets/compose_box_test.dart @@ -115,9 +115,11 @@ void main() { .controller.isNotNull().value.text.equals(expected); } + final sendButtonFinder = find.byIcon(ZulipIcons.send); + Future tapSendButton(WidgetTester tester) async { connection.prepare(json: SendMessageResult(id: 123).toJson()); - await tester.tap(find.byIcon(ZulipIcons.send)); + await tester.tap(sendButtonFinder); await tester.pump(Duration.zero); } @@ -690,7 +692,7 @@ void main() { connection.prepare(json: {}); connection.prepare(json: SendMessageResult(id: 123).toJson()); - await tester.tap(find.byIcon(ZulipIcons.send)); + await tester.tap(sendButtonFinder); await tester.pump(Duration.zero); final requests = connection.takeRequests(); checkSetTypingStatusRequests([requests.first], [(TypingOp.stop, narrow)]); @@ -854,7 +856,7 @@ void main() { await enterTopic(tester, narrow: narrow, topic: topicInputText); await tester.enterText(contentInputFinder, 'test content'); - await tester.tap(find.byIcon(ZulipIcons.send)); + await tester.tap(sendButtonFinder); await tester.pump(); } @@ -911,7 +913,7 @@ void main() { group('uploads', () { void checkAppearsLoading(WidgetTester tester, bool expected) { final sendButtonElement = tester.element(find.ancestor( - of: find.byIcon(ZulipIcons.send), + of: sendButtonFinder, matching: find.byType(IconButton))); final sendButtonWidget = sendButtonElement.widget as IconButton; final designVariables = DesignVariables.of(sendButtonElement); From c1fbbe5e59f58020d5a295979b2a67570c25f732 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 6 May 2025 12:20:40 -0700 Subject: [PATCH 04/12] dialog: Sweep through error dialogs, giving `message` final punctuation --- assets/l10n/app_en.arb | 6 +++--- lib/generated/l10n/zulip_localizations.dart | 6 +++--- lib/generated/l10n/zulip_localizations_ar.dart | 6 +++--- lib/generated/l10n/zulip_localizations_en.dart | 6 +++--- lib/generated/l10n/zulip_localizations_ja.dart | 6 +++--- lib/generated/l10n/zulip_localizations_nb.dart | 6 +++--- lib/log.dart | 3 +++ lib/widgets/dialog.dart | 3 +++ 8 files changed, 24 insertions(+), 18 deletions(-) diff --git a/assets/l10n/app_en.arb b/assets/l10n/app_en.arb index d2d7b53033..cd5a621230 100644 --- a/assets/l10n/app_en.arb +++ b/assets/l10n/app_en.arb @@ -168,7 +168,7 @@ "server": {"type": "String", "example": "https://example.com"} } }, - "errorCouldNotFetchMessageSource": "Could not fetch message source", + "errorCouldNotFetchMessageSource": "Could not fetch message source.", "@errorCouldNotFetchMessageSource": { "description": "Error message when the source of a message could not be fetched." }, @@ -561,7 +561,7 @@ "url": {"type": "String", "example": "http://chat.example.com/"} } }, - "errorInvalidResponse": "The server sent an invalid response", + "errorInvalidResponse": "The server sent an invalid response.", "@errorInvalidResponse": { "description": "Error message when an API call returned an invalid response." }, @@ -591,7 +591,7 @@ "httpStatus": {"type": "int", "example": "500"} } }, - "errorVideoPlayerFailed": "Unable to play the video", + "errorVideoPlayerFailed": "Unable to play the video.", "@errorVideoPlayerFailed": { "description": "Error message when a video fails to play." }, diff --git a/lib/generated/l10n/zulip_localizations.dart b/lib/generated/l10n/zulip_localizations.dart index 6181c7b39b..3cd6c96960 100644 --- a/lib/generated/l10n/zulip_localizations.dart +++ b/lib/generated/l10n/zulip_localizations.dart @@ -359,7 +359,7 @@ abstract class ZulipLocalizations { /// Error message when the source of a message could not be fetched. /// /// In en, this message translates to: - /// **'Could not fetch message source'** + /// **'Could not fetch message source.'** String get errorCouldNotFetchMessageSource; /// Error message when copying the text of a message to the user's system clipboard failed. @@ -863,7 +863,7 @@ abstract class ZulipLocalizations { /// Error message when an API call returned an invalid response. /// /// In en, this message translates to: - /// **'The server sent an invalid response'** + /// **'The server sent an invalid response.'** String get errorInvalidResponse; /// Error message when a network request fails. @@ -893,7 +893,7 @@ abstract class ZulipLocalizations { /// Error message when a video fails to play. /// /// In en, this message translates to: - /// **'Unable to play the video'** + /// **'Unable to play the video.'** String get errorVideoPlayerFailed; /// Error message when URL is empty diff --git a/lib/generated/l10n/zulip_localizations_ar.dart b/lib/generated/l10n/zulip_localizations_ar.dart index f26b9d017c..04aa218ce7 100644 --- a/lib/generated/l10n/zulip_localizations_ar.dart +++ b/lib/generated/l10n/zulip_localizations_ar.dart @@ -141,7 +141,7 @@ class ZulipLocalizationsAr extends ZulipLocalizations { @override String get errorCouldNotFetchMessageSource => - 'Could not fetch message source'; + 'Could not fetch message source.'; @override String get errorCopyingFailed => 'Copying failed'; @@ -459,7 +459,7 @@ class ZulipLocalizationsAr extends ZulipLocalizations { } @override - String get errorInvalidResponse => 'The server sent an invalid response'; + String get errorInvalidResponse => 'The server sent an invalid response.'; @override String get errorNetworkRequestFailed => 'Network request failed'; @@ -480,7 +480,7 @@ class ZulipLocalizationsAr extends ZulipLocalizations { } @override - String get errorVideoPlayerFailed => 'Unable to play the video'; + String get errorVideoPlayerFailed => 'Unable to play the video.'; @override String get serverUrlValidationErrorEmpty => 'Please enter a URL.'; diff --git a/lib/generated/l10n/zulip_localizations_en.dart b/lib/generated/l10n/zulip_localizations_en.dart index bfb14645d5..8dc52afb94 100644 --- a/lib/generated/l10n/zulip_localizations_en.dart +++ b/lib/generated/l10n/zulip_localizations_en.dart @@ -141,7 +141,7 @@ class ZulipLocalizationsEn extends ZulipLocalizations { @override String get errorCouldNotFetchMessageSource => - 'Could not fetch message source'; + 'Could not fetch message source.'; @override String get errorCopyingFailed => 'Copying failed'; @@ -459,7 +459,7 @@ class ZulipLocalizationsEn extends ZulipLocalizations { } @override - String get errorInvalidResponse => 'The server sent an invalid response'; + String get errorInvalidResponse => 'The server sent an invalid response.'; @override String get errorNetworkRequestFailed => 'Network request failed'; @@ -480,7 +480,7 @@ class ZulipLocalizationsEn extends ZulipLocalizations { } @override - String get errorVideoPlayerFailed => 'Unable to play the video'; + String get errorVideoPlayerFailed => 'Unable to play the video.'; @override String get serverUrlValidationErrorEmpty => 'Please enter a URL.'; diff --git a/lib/generated/l10n/zulip_localizations_ja.dart b/lib/generated/l10n/zulip_localizations_ja.dart index 35088555e2..04cbf3e7bd 100644 --- a/lib/generated/l10n/zulip_localizations_ja.dart +++ b/lib/generated/l10n/zulip_localizations_ja.dart @@ -141,7 +141,7 @@ class ZulipLocalizationsJa extends ZulipLocalizations { @override String get errorCouldNotFetchMessageSource => - 'Could not fetch message source'; + 'Could not fetch message source.'; @override String get errorCopyingFailed => 'Copying failed'; @@ -459,7 +459,7 @@ class ZulipLocalizationsJa extends ZulipLocalizations { } @override - String get errorInvalidResponse => 'The server sent an invalid response'; + String get errorInvalidResponse => 'The server sent an invalid response.'; @override String get errorNetworkRequestFailed => 'Network request failed'; @@ -480,7 +480,7 @@ class ZulipLocalizationsJa extends ZulipLocalizations { } @override - String get errorVideoPlayerFailed => 'Unable to play the video'; + String get errorVideoPlayerFailed => 'Unable to play the video.'; @override String get serverUrlValidationErrorEmpty => 'Please enter a URL.'; diff --git a/lib/generated/l10n/zulip_localizations_nb.dart b/lib/generated/l10n/zulip_localizations_nb.dart index ae79d99ea9..1c5075cdcc 100644 --- a/lib/generated/l10n/zulip_localizations_nb.dart +++ b/lib/generated/l10n/zulip_localizations_nb.dart @@ -141,7 +141,7 @@ class ZulipLocalizationsNb extends ZulipLocalizations { @override String get errorCouldNotFetchMessageSource => - 'Could not fetch message source'; + 'Could not fetch message source.'; @override String get errorCopyingFailed => 'Copying failed'; @@ -459,7 +459,7 @@ class ZulipLocalizationsNb extends ZulipLocalizations { } @override - String get errorInvalidResponse => 'The server sent an invalid response'; + String get errorInvalidResponse => 'The server sent an invalid response.'; @override String get errorNetworkRequestFailed => 'Network request failed'; @@ -480,7 +480,7 @@ class ZulipLocalizationsNb extends ZulipLocalizations { } @override - String get errorVideoPlayerFailed => 'Unable to play the video'; + String get errorVideoPlayerFailed => 'Unable to play the video.'; @override String get serverUrlValidationErrorEmpty => 'Please enter a URL.'; diff --git a/lib/log.dart b/lib/log.dart index c85d228263..64cb409a0e 100644 --- a/lib/log.dart +++ b/lib/log.dart @@ -80,6 +80,7 @@ typedef ReportErrorCallback = void Function( /// /// If `details` is non-null, the [SnackBar] will contain a button that would /// open a dialog containing the error details. +/// Prose in `details` should have final punctuation. // This gets set in [ZulipApp]. We need this indirection to keep `lib/log.dart` // from importing widget code, because the file is a dependency for the rest of // the app. @@ -91,6 +92,8 @@ ReportErrorCancellablyCallback reportErrorToUserBriefly = defaultReportErrorToUs /// as the body. If called before the app's widget tree is ready /// (see [ZulipApp.ready]), then we give up on showing the message to the user, /// and just log the message to the console. +/// +/// Prose in `message` should have final punctuation. // This gets set in [ZulipApp]. We need this indirection to keep `lib/log.dart` // from importing widget code, because the file is a dependency for the rest of // the app. diff --git a/lib/widgets/dialog.dart b/lib/widgets/dialog.dart index 0d2b4c5a7d..08ce8f08c7 100644 --- a/lib/widgets/dialog.dart +++ b/lib/widgets/dialog.dart @@ -49,6 +49,9 @@ class DialogStatus { /// /// The [DialogStatus.result] field of the return value can be used /// for waiting for the dialog to be closed. +/// +/// Prose in [message] should have final punctuation: +/// https://github.com/zulip/zulip-flutter/pull/1498#issuecomment-2853578577 // 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 From 13a8b28156426b3fa615d6030605dff02bd48d65 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 9 May 2025 16:17:40 -0700 Subject: [PATCH 05/12] test: Support testing with poll messages in the message list When we set up the message list in tests, we do it by preparing the API response for a message fetch or new-message event, via JSON. But {Stream,Dm}Message.toJson drops poll data on the floor, which defeats setting up a poll-style message in the message list. Until now, anyway, with this workaround. A reference in a dartdoc to something called `prepareMessageWithSubmessages` was dangling; it seemed to be dangling when it first appeared, too, in 5af5c76a8; there's nothing by that name. --- lib/api/model/submessage.dart | 32 +++++++++++++++++++++++++++++--- test/model/message_test.dart | 4 ---- 2 files changed, 29 insertions(+), 7 deletions(-) diff --git a/lib/api/model/submessage.dart b/lib/api/model/submessage.dart index f338265b46..81181f061f 100644 --- a/lib/api/model/submessage.dart +++ b/lib/api/model/submessage.dart @@ -64,6 +64,7 @@ class Submessage { widgetData: widgetData, pollEventSubmessages: submessages.skip(1), messageSenderId: messageSenderId, + debugSubmessages: kDebugMode ? submessages : null, ); case UnsupportedWidgetData(): assert(debugLog('Unsupported widgetData: ${widgetData.json}')); @@ -368,11 +369,13 @@ class Poll extends ChangeNotifier { required PollWidgetData widgetData, required Iterable pollEventSubmessages, required int messageSenderId, + required List? debugSubmessages, }) { final poll = Poll._( messageSenderId: messageSenderId, question: widgetData.extraData.question, options: widgetData.extraData.options, + debugSubmessages: debugSubmessages, ); for (final submessage in pollEventSubmessages) { @@ -386,17 +389,23 @@ class Poll extends ChangeNotifier { required this.messageSenderId, required this.question, required List options, + required List? debugSubmessages, }) { for (int index = 0; index < options.length; index += 1) { // Initial poll options use a placeholder senderId. // See [PollEventSubmessage.optionKey] for details. _addOption(senderId: null, idx: index, option: options[index]); } + if (kDebugMode) { + _debugSubmessages = debugSubmessages; + } } final int messageSenderId; String question; + List? _debugSubmessages; + /// The limit of options any single user can add to a poll. /// /// See https://github.com/zulip/zulip/blob/304d948416465c1a085122af5d752f03d6797003/web/shared/src/poll_data.ts#L69-L71 @@ -417,6 +426,14 @@ class Poll extends ChangeNotifier { } _applyEvent(event.senderId, pollEventSubmessage); notifyListeners(); + + if (kDebugMode) { + assert(_debugSubmessages != null); + _debugSubmessages!.add(Submessage( + senderId: event.senderId, + msgType: event.msgType, + content: event.content)); + } } void _applyEvent(int senderId, PollEventSubmessage event) { @@ -472,9 +489,18 @@ class Poll extends ChangeNotifier { } static List toJson(Poll? poll) { - // Rather than maintaining a up-to-date submessages list, return as if it is - // empty, because we are not sending the submessages to the server anyway. - return []; + List? result; + + if (kDebugMode) { + // Useful for setting up a message list with a poll message, which goes + // through this codepath (when preparing a fetch response). + result = poll?._debugSubmessages; + } + + // In prod, rather than maintaining a up-to-date submessages list, + // return as if it is empty, because we are not sending the submessages + // to the server anyway. + return result ?? []; } } diff --git a/test/model/message_test.dart b/test/model/message_test.dart index 2ed4d99490..97c6fbc8b1 100644 --- a/test/model/message_test.dart +++ b/test/model/message_test.dart @@ -61,14 +61,10 @@ void main() { /// Perform the initial message fetch for [messageList]. /// /// The test case must have already called [prepare] to initialize the state. - /// - /// This does not support submessages. Use [prepareMessageWithSubmessages] - /// instead if needed. Future prepareMessages( List messages, { bool foundOldest = false, }) async { - assert(messages.every((message) => message.poll == null)); connection.prepare(json: eg.newestGetMessagesResult(foundOldest: foundOldest, messages: messages).toJson()); await messageList.fetchInitial(); From 18fee48fb82d34f9b929191c601cb5936bc74ff3 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 9 May 2025 13:11:32 -0700 Subject: [PATCH 06/12] action_sheet test: Silence a noisy warning in an upcoming new test We're about to add a test that renders a poll-style message, and we'd get this warning on that. --- test/widgets/action_sheet_test.dart | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/test/widgets/action_sheet_test.dart b/test/widgets/action_sheet_test.dart index 7d6a5205bb..7ab166bc44 100644 --- a/test/widgets/action_sheet_test.dart +++ b/test/widgets/action_sheet_test.dart @@ -79,10 +79,21 @@ Future setupToMessageActionSheet(WidgetTester tester, { // global store, per-account store, and message list get loaded await tester.pumpAndSettle(); - // request the message action sheet - await tester.longPress(find.byType(MessageContent)); + // Request the message action sheet. + // + // We use `warnIfMissed: false` to suppress warnings in cases where + // MessageContent itself didn't hit-test as true but the action sheet still + // opened. The action sheet still opens because the gesture handler is an + // ancestor of MessageContent, but MessageContent might not hit-test as true + // because its render box effectively has HitTestBehavior.deferToChild, and + // the long-press might land where no child hit-tests as true, + // like if it's in padding around a Paragraph. + await tester.longPress(find.byType(MessageContent), warnIfMissed: false); // sheet appears onscreen; default duration of bottom-sheet enter animation await tester.pump(const Duration(milliseconds: 250)); + // Check the action sheet did in fact open, so we don't defeat any tests that + // use simple `find.byIcon`-style checks to test presence/absence of a button. + check(find.byType(BottomSheet)).findsOne(); } void main() { From c7863c02c9fb2bcca3502fbe9c94e43bc5622f69 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 12 May 2025 18:30:31 -0700 Subject: [PATCH 07/12] compose [nfc]: Give Compose{Content,Topic}Controller a starting-text param --- lib/widgets/compose_box.dart | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/widgets/compose_box.dart b/lib/widgets/compose_box.dart index bfd91d6afd..acdfa170b5 100644 --- a/lib/widgets/compose_box.dart +++ b/lib/widgets/compose_box.dart @@ -82,6 +82,8 @@ const double _composeButtonSize = 44; /// /// Subclasses must ensure that [_update] is called in all exposed constructors. abstract class ComposeController extends TextEditingController { + ComposeController({super.text}); + int get maxLengthUnicodeCodePoints; String get textNormalized => _textNormalized; @@ -143,7 +145,7 @@ enum TopicValidationError { } class ComposeTopicController extends ComposeController { - ComposeTopicController({required this.store}) { + ComposeTopicController({super.text, required this.store}) { _update(); } @@ -226,7 +228,7 @@ enum ContentValidationError { } class ComposeContentController extends ComposeController { - ComposeContentController() { + ComposeContentController({super.text}) { _update(); } From ba65dc58b08dc5754172259ab74c944574a5577a Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Thu, 15 May 2025 13:34:33 -0700 Subject: [PATCH 08/12] test: eg.initialSnapshot.realmMessageContentEditLimitSeconds default null --- test/example_data.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/example_data.dart b/test/example_data.dart index 9577ef0e73..e0f44f9ddc 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -971,7 +971,7 @@ InitialSnapshot initialSnapshot({ realmMandatoryTopics: realmMandatoryTopics ?? true, realmWaitingPeriodThreshold: realmWaitingPeriodThreshold ?? 0, realmAllowMessageEditing: realmAllowMessageEditing ?? true, - realmMessageContentEditLimitSeconds: realmMessageContentEditLimitSeconds ?? 600, + realmMessageContentEditLimitSeconds: realmMessageContentEditLimitSeconds, realmDefaultExternalAccounts: realmDefaultExternalAccounts ?? {}, maxFileUploadSizeMib: maxFileUploadSizeMib ?? 25, serverEmojiDataUrl: serverEmojiDataUrl From 99c530acc08dbc10d800728d3760dc88a3591ecc Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Thu, 15 May 2025 13:36:41 -0700 Subject: [PATCH 09/12] test: Add FakeApiConnection.clearPreparedResponses --- test/api/fake_api.dart | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/api/fake_api.dart b/test/api/fake_api.dart index 2b230376ac..2382ab859b 100644 --- a/test/api/fake_api.dart +++ b/test/api/fake_api.dart @@ -81,6 +81,10 @@ class FakeHttpClient extends http.BaseClient { } } + void clearPreparedResponses() { + _preparedResponses.clear(); + } + @override Future send(http.BaseRequest request) { _requestHistory.add(request); @@ -278,6 +282,10 @@ class FakeApiConnection extends ApiConnection { delay: delay, ); } + + void clearPreparedResponses() { + client.clearPreparedResponses(); + } } extension FakeApiConnectionChecks on Subject { From ec811eec734823c6c6e8bc79950a581932cc5beb Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Thu, 15 May 2025 13:39:02 -0700 Subject: [PATCH 10/12] message: Include both {new,originalRaw}Content in takeFailedMessageEdit Greg points out that we'll need both values when restoring a failed message edit: originalRawContent for the eventual edit-message request (for prevContentSha256), and newContent to fill the edit-message compose box, so the user can restore the edit session to what it was before it failed. --- lib/model/message.dart | 18 +++++++++++++----- lib/model/store.dart | 2 +- test/model/message_test.dart | 4 ++-- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/lib/model/message.dart b/lib/model/message.dart index 1da91e9b0d..2573cfadc6 100644 --- a/lib/model/message.dart +++ b/lib/model/message.dart @@ -63,13 +63,18 @@ mixin MessageStore { /// /// Should only be called when there is a failed request, /// per [getEditMessageErrorStatus]. - String takeFailedMessageEdit(int messageId); + ({String originalRawContent, String newContent}) takeFailedMessageEdit(int messageId); } class _EditMessageRequestStatus { - _EditMessageRequestStatus({required this.hasError, required this.newContent}); + _EditMessageRequestStatus({ + required this.hasError, + required this.originalRawContent, + required this.newContent, + }); bool hasError; + final String originalRawContent; final String newContent; } @@ -185,7 +190,7 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore { } _editMessageRequests[messageId] = _EditMessageRequestStatus( - hasError: false, newContent: newContent); + hasError: false, originalRawContent: originalRawContent, newContent: newContent); _notifyMessageListViewsForOneMessage(messageId); try { await updateMessage(connection, @@ -210,7 +215,7 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore { } @override - String takeFailedMessageEdit(int messageId) { + ({String originalRawContent, String newContent}) takeFailedMessageEdit(int messageId) { final status = _editMessageRequests.remove(messageId); _notifyMessageListViewsForOneMessage(messageId); if (status == null) { @@ -219,7 +224,10 @@ class MessageStoreImpl extends PerAccountStoreBase with MessageStore { if (!status.hasError) { throw StateError("called takeFailedMessageEdit, but edit hasn't failed"); } - return status.newContent; + return ( + originalRawContent: status.originalRawContent, + newContent: status.newContent + ); } void handleUserTopicEvent(UserTopicEvent event) { diff --git a/lib/model/store.dart b/lib/model/store.dart index f7a4e0ca4e..5f5b19128b 100644 --- a/lib/model/store.dart +++ b/lib/model/store.dart @@ -765,7 +765,7 @@ class PerAccountStore extends PerAccountStoreBase with ChangeNotifier, EmojiStor originalRawContent: originalRawContent, newContent: newContent); } @override - String takeFailedMessageEdit(int messageId) { + ({String originalRawContent, String newContent}) takeFailedMessageEdit(int messageId) { assert(!_disposed); return _messages.takeFailedMessageEdit(messageId); } diff --git a/test/model/message_test.dart b/test/model/message_test.dart index 97c6fbc8b1..1809f0888b 100644 --- a/test/model/message_test.dart +++ b/test/model/message_test.dart @@ -252,7 +252,7 @@ void main() { check(store.getEditMessageErrorStatus(message.id)).isNotNull().isTrue(); checkNotifiedOnce(); - check(store.takeFailedMessageEdit(message.id)).equals('new content'); + check(store.takeFailedMessageEdit(message.id).newContent).equals('new content'); check(store.getEditMessageErrorStatus(message.id)).isNull(); checkNotifiedOnce(); })); @@ -338,7 +338,7 @@ void main() { async.elapse(Duration(seconds: 1)); check(store.getEditMessageErrorStatus(message.id)).isNotNull().isTrue(); checkNotifiedOnce(); - check(store.takeFailedMessageEdit(message.id)).equals('new content'); + check(store.takeFailedMessageEdit(message.id).newContent).equals('new content'); checkNotifiedOnce(); await store.handleEvent(eg.updateMessageEditEvent(message)); // no error From af36660e4081c600dffa5591c76680c55904e796 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Thu, 15 May 2025 13:53:17 -0700 Subject: [PATCH 11/12] compose: Prepare some leafward widgets to support a disabled state In the upcoming edit-message feature, the edit-message compose box will have a "Preparing..." state after you tap "Edit message" in the action sheet, while we're fetching the raw message content. The edit-message compose box shouldn't be interactable in this state. --- lib/widgets/compose_box.dart | 64 ++++++++++++++++++++++++++++++------ 1 file changed, 54 insertions(+), 10 deletions(-) diff --git a/lib/widgets/compose_box.dart b/lib/widgets/compose_box.dart index acdfa170b5..dfc55bf193 100644 --- a/lib/widgets/compose_box.dart +++ b/lib/widgets/compose_box.dart @@ -493,11 +493,13 @@ class _ContentInput extends StatelessWidget { required this.narrow, required this.controller, required this.hintText, + this.enabled = true, // ignore: unused_element_parameter }); final Narrow narrow; final ComposeBoxController controller; final String hintText; + final bool enabled; static double maxHeight(BuildContext context) { final clampingTextScaler = MediaQuery.textScalerOf(context) @@ -540,6 +542,7 @@ class _ContentInput extends StatelessWidget { top: _verticalPadding, bottom: _verticalPadding, color: designVariables.composeBoxBg, child: TextField( + enabled: enabled, controller: controller.content, focusNode: controller.contentFocusNode, // Let the content show through the `contentPadding` so that @@ -957,9 +960,10 @@ Future _uploadFiles({ } abstract class _AttachUploadsButton extends StatelessWidget { - const _AttachUploadsButton({required this.controller}); + const _AttachUploadsButton({required this.controller, required this.enabled}); final ComposeBoxController controller; + final bool enabled; IconData get icon; String tooltip(ZulipLocalizations zulipLocalizations); @@ -1001,7 +1005,7 @@ abstract class _AttachUploadsButton extends StatelessWidget { child: IconButton( icon: Icon(icon, color: designVariables.foreground.withFadedAlpha(0.5)), tooltip: tooltip(zulipLocalizations), - onPressed: () => _handlePress(context))); + onPressed: enabled ? () => _handlePress(context) : null)); } } @@ -1060,7 +1064,7 @@ Future> _getFilePickerFiles(BuildContext context, FileType type) } class _AttachFileButton extends _AttachUploadsButton { - const _AttachFileButton({required super.controller}); + const _AttachFileButton({required super.controller, required super.enabled}); @override IconData get icon => ZulipIcons.attach_file; @@ -1076,7 +1080,7 @@ class _AttachFileButton extends _AttachUploadsButton { } class _AttachMediaButton extends _AttachUploadsButton { - const _AttachMediaButton({required super.controller}); + const _AttachMediaButton({required super.controller, required super.enabled}); @override IconData get icon => ZulipIcons.image; @@ -1093,7 +1097,7 @@ class _AttachMediaButton extends _AttachUploadsButton { } class _AttachFromCameraButton extends _AttachUploadsButton { - const _AttachFromCameraButton({required super.controller}); + const _AttachFromCameraButton({required super.controller, required super.enabled}); @override IconData get icon => ZulipIcons.camera; @@ -1370,6 +1374,7 @@ abstract class _ComposeBoxBody extends StatelessWidget { Widget? buildTopicInput(); Widget buildContentInput(); + bool getComposeButtonsEnabled(BuildContext context); Widget buildSendButton(); @override @@ -1396,10 +1401,11 @@ abstract class _ComposeBoxBody extends StatelessWidget { shape: const RoundedRectangleBorder( borderRadius: BorderRadius.all(Radius.circular(4))))); + final composeButtonsEnabled = getComposeButtonsEnabled(context); final composeButtons = [ - _AttachFileButton(controller: controller), - _AttachMediaButton(controller: controller), - _AttachFromCameraButton(controller: controller), + _AttachFileButton(controller: controller, enabled: composeButtonsEnabled), + _AttachMediaButton(controller: controller, enabled: composeButtonsEnabled), + _AttachFromCameraButton(controller: controller, enabled: composeButtonsEnabled), ]; final topicInput = buildTopicInput(); @@ -1449,6 +1455,8 @@ class _StreamComposeBoxBody extends _ComposeBoxBody { controller: controller, ); + @override bool getComposeButtonsEnabled(BuildContext context) => true; + @override Widget buildSendButton() => _SendButton( controller: controller, getDestination: () => StreamDestination( @@ -1472,6 +1480,8 @@ class _FixedDestinationComposeBoxBody extends _ComposeBoxBody { controller: controller, ); + @override bool getComposeButtonsEnabled(BuildContext context) => true; + @override Widget buildSendButton() => _SendButton( controller: controller, getDestination: () => narrow.destination, @@ -1756,7 +1766,8 @@ class _ComposeBoxState extends State with PerAccountStoreAwareStateM final errorBanner = _errorBannerComposingNotAllowed(context); if (errorBanner != null) { - return _ComposeBoxContainer(body: null, banner: errorBanner); + return ComposeBoxInheritedWidget.fromComposeBoxState(this, + child: _ComposeBoxContainer(body: null, banner: errorBanner)); } final controller = this.controller; @@ -1777,6 +1788,39 @@ class _ComposeBoxState extends State with PerAccountStoreAwareStateM // errorBanner = _ErrorBanner(label: // ZulipLocalizations.of(context).errorSendMessageTimeout); // } - return _ComposeBoxContainer(body: body, banner: null); + return ComposeBoxInheritedWidget.fromComposeBoxState(this, + child: _ComposeBoxContainer(body: body, banner: null)); + } +} + +/// An [InheritedWidget] to provide data to leafward [StatelessWidget]s, +/// such as flags that should cause the upload buttons to be disabled. +class ComposeBoxInheritedWidget extends InheritedWidget { + factory ComposeBoxInheritedWidget.fromComposeBoxState( + ComposeBoxState state, { + required Widget child, + }) { + return ComposeBoxInheritedWidget._( + // TODO add fields + child: child, + ); + } + + const ComposeBoxInheritedWidget._({ + // TODO add fields + required super.child, + }); + + // TODO add fields + + @override + bool updateShouldNotify(covariant ComposeBoxInheritedWidget oldWidget) => + // TODO compare fields + false; + + static ComposeBoxInheritedWidget of(BuildContext context) { + final widget = context.dependOnInheritedWidgetOfExactType(); + assert(widget != null, 'No ComposeBoxInheritedWidget ancestor'); + return widget!; } } From 77ab9300a83045d6336946d73d55b3792c046413 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Fri, 2 May 2025 11:56:40 -0700 Subject: [PATCH 12/12] compose: Support editing an already-sent message Fixes: #126 --- assets/l10n/app_en.arb | 56 ++ lib/generated/l10n/zulip_localizations.dart | 84 +++ .../l10n/zulip_localizations_ar.dart | 45 ++ .../l10n/zulip_localizations_en.dart | 45 ++ .../l10n/zulip_localizations_ja.dart | 45 ++ .../l10n/zulip_localizations_nb.dart | 45 ++ .../l10n/zulip_localizations_pl.dart | 45 ++ .../l10n/zulip_localizations_ru.dart | 45 ++ .../l10n/zulip_localizations_sk.dart | 45 ++ .../l10n/zulip_localizations_uk.dart | 45 ++ lib/widgets/action_sheet.dart | 52 ++ lib/widgets/compose_box.dart | 310 ++++++++++- lib/widgets/message_list.dart | 98 +++- lib/widgets/theme.dart | 21 + test/flutter_checks.dart | 4 + test/widgets/action_sheet_test.dart | 173 +++++- test/widgets/compose_box_checks.dart | 15 + test/widgets/compose_box_test.dart | 505 +++++++++++++++++- test/widgets/message_list_test.dart | 79 ++- 19 files changed, 1736 insertions(+), 21 deletions(-) diff --git a/assets/l10n/app_en.arb b/assets/l10n/app_en.arb index cd5a621230..d11bf43eda 100644 --- a/assets/l10n/app_en.arb +++ b/assets/l10n/app_en.arb @@ -144,6 +144,10 @@ "@actionSheetOptionUnstarMessage": { "description": "Label for unstar button on action sheet." }, + "actionSheetOptionEditMessage": "Edit message", + "@actionSheetOptionEditMessage": { + "description": "Label for the 'Edit message' button in the message action sheet." + }, "actionSheetOptionMarkTopicAsRead": "Mark topic as read", "@actionSheetOptionMarkTopicAsRead": { "description": "Option to mark a specific topic as read in the action sheet." @@ -219,6 +223,10 @@ "@errorMessageNotSent": { "description": "Error message for compose box when a message could not be sent." }, + "errorMessageEditNotSaved": "Message not saved", + "@errorMessageEditNotSaved": { + "description": "Error message for compose box when a message edit could not be saved." + }, "errorLoginCouldNotConnect": "Failed to connect to server:\n{url}", "@errorLoginCouldNotConnect": { "description": "Error message when the app could not connect to the server.", @@ -309,6 +317,10 @@ "@errorUnstarMessageFailedTitle": { "description": "Error title when unstarring a message failed." }, + "errorCouldNotEditMessageTitle": "Could not edit message", + "@errorCouldNotEditMessageTitle": { + "description": "Error title when an exception prevented us from opening the compose box for editing a message." + }, "successLinkCopied": "Link copied", "@successLinkCopied": { "description": "Success message after copy link action completed." @@ -329,6 +341,46 @@ "@errorBannerCannotPostInChannelLabel": { "description": "Error-banner text replacing the compose box when you do not have permission to send a message to the channel." }, + "composeBoxBannerLabelEditMessage": "Edit message", + "@composeBoxBannerLabelEditMessage": { + "description": "Label text for the compose-box banner when you are editing a message." + }, + "composeBoxBannerButtonCancel": "Cancel", + "@composeBoxBannerButtonCancel": { + "description": "Label text for the 'Cancel' button in the compose-box banner when you are editing a message." + }, + "composeBoxBannerButtonSave": "Save", + "@composeBoxBannerButtonSave": { + "description": "Label text for the 'Save' button in the compose-box banner when you are editing a message." + }, + "editAlreadyInProgressTitle": "Cannot edit message", + "@editAlreadyInProgressTitle": { + "description": "Error title when a message edit cannot be saved because there is another edit already in progress." + }, + "editAlreadyInProgressMessage": "An edit is already in progress. Please wait for it to complete.", + "@editAlreadyInProgressMessage": { + "description": "Error message when a message edit cannot be saved because there is another edit already in progress." + }, + "savingMessageEditLabel": "SAVING EDIT…", + "@savingMessageEditLabel": { + "description": "Text on a message in the message list saying that a message edit request is processing. (Use ALL CAPS for cased alphabets: Latin, Greek, Cyrillic, etc.)" + }, + "savingMessageEditFailedLabel": "EDIT NOT SAVED", + "@savingMessageEditFailedLabel": { + "description": "Text on a message in the message list saying that a message edit request failed. (Use ALL CAPS for cased alphabets: Latin, Greek, Cyrillic, etc.)" + }, + "discardDraftConfirmationDialogTitle": "Discard the message you’re writing?", + "@discardDraftConfirmationDialogTitle": { + "description": "Title for a confirmation dialog for discarding message text that was typed into the compose box." + }, + "discardDraftConfirmationDialogMessage": "When you edit a message, the content that was previously in the compose box is discarded.", + "@discardDraftConfirmationDialogMessage": { + "description": "Message for a confirmation dialog for discarding message text that was typed into the compose box." + }, + "discardDraftConfirmationDialogConfirmButton": "Discard", + "@discardDraftConfirmationDialogConfirmButton": { + "description": "Label for the 'Discard' button on a confirmation dialog for discarding message text that was typed into the compose box." + }, "composeBoxAttachFilesTooltip": "Attach files", "@composeBoxAttachFilesTooltip": { "description": "Tooltip for compose box icon to attach a file to the message." @@ -367,6 +419,10 @@ "destination": {"type": "String", "example": "#channel name > topic name"} } }, + "preparingEditMessageContentInput": "Preparing…", + "@preparingEditMessageContentInput": { + "description": "Hint text for content input when the compose box is preparing to edit a message." + }, "composeBoxSendTooltip": "Send", "@composeBoxSendTooltip": { "description": "Tooltip for send button in compose box." diff --git a/lib/generated/l10n/zulip_localizations.dart b/lib/generated/l10n/zulip_localizations.dart index 3cd6c96960..75b70a0377 100644 --- a/lib/generated/l10n/zulip_localizations.dart +++ b/lib/generated/l10n/zulip_localizations.dart @@ -326,6 +326,12 @@ abstract class ZulipLocalizations { /// **'Unstar message'** String get actionSheetOptionUnstarMessage; + /// Label for the 'Edit message' button in the message action sheet. + /// + /// In en, this message translates to: + /// **'Edit message'** + String get actionSheetOptionEditMessage; + /// Option to mark a specific topic as read in the action sheet. /// /// In en, this message translates to: @@ -414,6 +420,12 @@ abstract class ZulipLocalizations { /// **'Message not sent'** String get errorMessageNotSent; + /// Error message for compose box when a message edit could not be saved. + /// + /// In en, this message translates to: + /// **'Message not saved'** + String get errorMessageEditNotSaved; + /// Error message when the app could not connect to the server. /// /// In en, this message translates to: @@ -526,6 +538,12 @@ abstract class ZulipLocalizations { /// **'Failed to unstar message'** String get errorUnstarMessageFailedTitle; + /// Error title when an exception prevented us from opening the compose box for editing a message. + /// + /// In en, this message translates to: + /// **'Could not edit message'** + String get errorCouldNotEditMessageTitle; + /// Success message after copy link action completed. /// /// In en, this message translates to: @@ -556,6 +574,66 @@ abstract class ZulipLocalizations { /// **'You do not have permission to post in this channel.'** String get errorBannerCannotPostInChannelLabel; + /// Label text for the compose-box banner when you are editing a message. + /// + /// In en, this message translates to: + /// **'Edit message'** + String get composeBoxBannerLabelEditMessage; + + /// Label text for the 'Cancel' button in the compose-box banner when you are editing a message. + /// + /// In en, this message translates to: + /// **'Cancel'** + String get composeBoxBannerButtonCancel; + + /// Label text for the 'Save' button in the compose-box banner when you are editing a message. + /// + /// In en, this message translates to: + /// **'Save'** + String get composeBoxBannerButtonSave; + + /// Error title when a message edit cannot be saved because there is another edit already in progress. + /// + /// In en, this message translates to: + /// **'Cannot edit message'** + String get editAlreadyInProgressTitle; + + /// Error message when a message edit cannot be saved because there is another edit already in progress. + /// + /// In en, this message translates to: + /// **'An edit is already in progress. Please wait for it to complete.'** + String get editAlreadyInProgressMessage; + + /// Text on a message in the message list saying that a message edit request is processing. (Use ALL CAPS for cased alphabets: Latin, Greek, Cyrillic, etc.) + /// + /// In en, this message translates to: + /// **'SAVING EDIT…'** + String get savingMessageEditLabel; + + /// Text on a message in the message list saying that a message edit request failed. (Use ALL CAPS for cased alphabets: Latin, Greek, Cyrillic, etc.) + /// + /// In en, this message translates to: + /// **'EDIT NOT SAVED'** + String get savingMessageEditFailedLabel; + + /// Title for a confirmation dialog for discarding message text that was typed into the compose box. + /// + /// In en, this message translates to: + /// **'Discard the message you’re writing?'** + String get discardDraftConfirmationDialogTitle; + + /// Message for a confirmation dialog for discarding message text that was typed into the compose box. + /// + /// In en, this message translates to: + /// **'When you edit a message, the content that was previously in the compose box is discarded.'** + String get discardDraftConfirmationDialogMessage; + + /// Label for the 'Discard' button on a confirmation dialog for discarding message text that was typed into the compose box. + /// + /// In en, this message translates to: + /// **'Discard'** + String get discardDraftConfirmationDialogConfirmButton; + /// Tooltip for compose box icon to attach a file to the message. /// /// In en, this message translates to: @@ -604,6 +682,12 @@ abstract class ZulipLocalizations { /// **'Message {destination}'** String composeBoxChannelContentHint(String destination); + /// Hint text for content input when the compose box is preparing to edit a message. + /// + /// In en, this message translates to: + /// **'Preparing…'** + String get preparingEditMessageContentInput; + /// Tooltip for send button in compose box. /// /// In en, this message translates to: diff --git a/lib/generated/l10n/zulip_localizations_ar.dart b/lib/generated/l10n/zulip_localizations_ar.dart index 04aa218ce7..98dd9a7af6 100644 --- a/lib/generated/l10n/zulip_localizations_ar.dart +++ b/lib/generated/l10n/zulip_localizations_ar.dart @@ -122,6 +122,9 @@ class ZulipLocalizationsAr extends ZulipLocalizations { @override String get actionSheetOptionUnstarMessage => 'Unstar message'; + @override + String get actionSheetOptionEditMessage => 'Edit message'; + @override String get actionSheetOptionMarkTopicAsRead => 'Mark topic as read'; @@ -191,6 +194,9 @@ class ZulipLocalizationsAr extends ZulipLocalizations { @override String get errorMessageNotSent => 'Message not sent'; + @override + String get errorMessageEditNotSaved => 'Message not saved'; + @override String errorLoginCouldNotConnect(String url) { return 'Failed to connect to server:\n$url'; @@ -262,6 +268,9 @@ class ZulipLocalizationsAr extends ZulipLocalizations { @override String get errorUnstarMessageFailedTitle => 'Failed to unstar message'; + @override + String get errorCouldNotEditMessageTitle => 'Could not edit message'; + @override String get successLinkCopied => 'Link copied'; @@ -279,6 +288,39 @@ class ZulipLocalizationsAr extends ZulipLocalizations { String get errorBannerCannotPostInChannelLabel => 'You do not have permission to post in this channel.'; + @override + String get composeBoxBannerLabelEditMessage => 'Edit message'; + + @override + String get composeBoxBannerButtonCancel => 'Cancel'; + + @override + String get composeBoxBannerButtonSave => 'Save'; + + @override + String get editAlreadyInProgressTitle => 'Cannot edit message'; + + @override + String get editAlreadyInProgressMessage => + 'An edit is already in progress. Please wait for it to complete.'; + + @override + String get savingMessageEditLabel => 'SAVING EDIT…'; + + @override + String get savingMessageEditFailedLabel => 'EDIT NOT SAVED'; + + @override + String get discardDraftConfirmationDialogTitle => + 'Discard the message you’re writing?'; + + @override + String get discardDraftConfirmationDialogMessage => + 'When you edit a message, the content that was previously in the compose box is discarded.'; + + @override + String get discardDraftConfirmationDialogConfirmButton => 'Discard'; + @override String get composeBoxAttachFilesTooltip => 'Attach files'; @@ -307,6 +349,9 @@ class ZulipLocalizationsAr extends ZulipLocalizations { return 'Message $destination'; } + @override + String get preparingEditMessageContentInput => 'Preparing…'; + @override String get composeBoxSendTooltip => 'Send'; diff --git a/lib/generated/l10n/zulip_localizations_en.dart b/lib/generated/l10n/zulip_localizations_en.dart index 8dc52afb94..0a746ff634 100644 --- a/lib/generated/l10n/zulip_localizations_en.dart +++ b/lib/generated/l10n/zulip_localizations_en.dart @@ -122,6 +122,9 @@ class ZulipLocalizationsEn extends ZulipLocalizations { @override String get actionSheetOptionUnstarMessage => 'Unstar message'; + @override + String get actionSheetOptionEditMessage => 'Edit message'; + @override String get actionSheetOptionMarkTopicAsRead => 'Mark topic as read'; @@ -191,6 +194,9 @@ class ZulipLocalizationsEn extends ZulipLocalizations { @override String get errorMessageNotSent => 'Message not sent'; + @override + String get errorMessageEditNotSaved => 'Message not saved'; + @override String errorLoginCouldNotConnect(String url) { return 'Failed to connect to server:\n$url'; @@ -262,6 +268,9 @@ class ZulipLocalizationsEn extends ZulipLocalizations { @override String get errorUnstarMessageFailedTitle => 'Failed to unstar message'; + @override + String get errorCouldNotEditMessageTitle => 'Could not edit message'; + @override String get successLinkCopied => 'Link copied'; @@ -279,6 +288,39 @@ class ZulipLocalizationsEn extends ZulipLocalizations { String get errorBannerCannotPostInChannelLabel => 'You do not have permission to post in this channel.'; + @override + String get composeBoxBannerLabelEditMessage => 'Edit message'; + + @override + String get composeBoxBannerButtonCancel => 'Cancel'; + + @override + String get composeBoxBannerButtonSave => 'Save'; + + @override + String get editAlreadyInProgressTitle => 'Cannot edit message'; + + @override + String get editAlreadyInProgressMessage => + 'An edit is already in progress. Please wait for it to complete.'; + + @override + String get savingMessageEditLabel => 'SAVING EDIT…'; + + @override + String get savingMessageEditFailedLabel => 'EDIT NOT SAVED'; + + @override + String get discardDraftConfirmationDialogTitle => + 'Discard the message you’re writing?'; + + @override + String get discardDraftConfirmationDialogMessage => + 'When you edit a message, the content that was previously in the compose box is discarded.'; + + @override + String get discardDraftConfirmationDialogConfirmButton => 'Discard'; + @override String get composeBoxAttachFilesTooltip => 'Attach files'; @@ -307,6 +349,9 @@ class ZulipLocalizationsEn extends ZulipLocalizations { return 'Message $destination'; } + @override + String get preparingEditMessageContentInput => 'Preparing…'; + @override String get composeBoxSendTooltip => 'Send'; diff --git a/lib/generated/l10n/zulip_localizations_ja.dart b/lib/generated/l10n/zulip_localizations_ja.dart index 04cbf3e7bd..74a2d4bedb 100644 --- a/lib/generated/l10n/zulip_localizations_ja.dart +++ b/lib/generated/l10n/zulip_localizations_ja.dart @@ -122,6 +122,9 @@ class ZulipLocalizationsJa extends ZulipLocalizations { @override String get actionSheetOptionUnstarMessage => 'Unstar message'; + @override + String get actionSheetOptionEditMessage => 'Edit message'; + @override String get actionSheetOptionMarkTopicAsRead => 'Mark topic as read'; @@ -191,6 +194,9 @@ class ZulipLocalizationsJa extends ZulipLocalizations { @override String get errorMessageNotSent => 'Message not sent'; + @override + String get errorMessageEditNotSaved => 'Message not saved'; + @override String errorLoginCouldNotConnect(String url) { return 'Failed to connect to server:\n$url'; @@ -262,6 +268,9 @@ class ZulipLocalizationsJa extends ZulipLocalizations { @override String get errorUnstarMessageFailedTitle => 'Failed to unstar message'; + @override + String get errorCouldNotEditMessageTitle => 'Could not edit message'; + @override String get successLinkCopied => 'Link copied'; @@ -279,6 +288,39 @@ class ZulipLocalizationsJa extends ZulipLocalizations { String get errorBannerCannotPostInChannelLabel => 'You do not have permission to post in this channel.'; + @override + String get composeBoxBannerLabelEditMessage => 'Edit message'; + + @override + String get composeBoxBannerButtonCancel => 'Cancel'; + + @override + String get composeBoxBannerButtonSave => 'Save'; + + @override + String get editAlreadyInProgressTitle => 'Cannot edit message'; + + @override + String get editAlreadyInProgressMessage => + 'An edit is already in progress. Please wait for it to complete.'; + + @override + String get savingMessageEditLabel => 'SAVING EDIT…'; + + @override + String get savingMessageEditFailedLabel => 'EDIT NOT SAVED'; + + @override + String get discardDraftConfirmationDialogTitle => + 'Discard the message you’re writing?'; + + @override + String get discardDraftConfirmationDialogMessage => + 'When you edit a message, the content that was previously in the compose box is discarded.'; + + @override + String get discardDraftConfirmationDialogConfirmButton => 'Discard'; + @override String get composeBoxAttachFilesTooltip => 'Attach files'; @@ -307,6 +349,9 @@ class ZulipLocalizationsJa extends ZulipLocalizations { return 'Message $destination'; } + @override + String get preparingEditMessageContentInput => 'Preparing…'; + @override String get composeBoxSendTooltip => 'Send'; diff --git a/lib/generated/l10n/zulip_localizations_nb.dart b/lib/generated/l10n/zulip_localizations_nb.dart index 1c5075cdcc..02913278b8 100644 --- a/lib/generated/l10n/zulip_localizations_nb.dart +++ b/lib/generated/l10n/zulip_localizations_nb.dart @@ -122,6 +122,9 @@ class ZulipLocalizationsNb extends ZulipLocalizations { @override String get actionSheetOptionUnstarMessage => 'Unstar message'; + @override + String get actionSheetOptionEditMessage => 'Edit message'; + @override String get actionSheetOptionMarkTopicAsRead => 'Mark topic as read'; @@ -191,6 +194,9 @@ class ZulipLocalizationsNb extends ZulipLocalizations { @override String get errorMessageNotSent => 'Message not sent'; + @override + String get errorMessageEditNotSaved => 'Message not saved'; + @override String errorLoginCouldNotConnect(String url) { return 'Failed to connect to server:\n$url'; @@ -262,6 +268,9 @@ class ZulipLocalizationsNb extends ZulipLocalizations { @override String get errorUnstarMessageFailedTitle => 'Failed to unstar message'; + @override + String get errorCouldNotEditMessageTitle => 'Could not edit message'; + @override String get successLinkCopied => 'Link copied'; @@ -279,6 +288,39 @@ class ZulipLocalizationsNb extends ZulipLocalizations { String get errorBannerCannotPostInChannelLabel => 'You do not have permission to post in this channel.'; + @override + String get composeBoxBannerLabelEditMessage => 'Edit message'; + + @override + String get composeBoxBannerButtonCancel => 'Cancel'; + + @override + String get composeBoxBannerButtonSave => 'Save'; + + @override + String get editAlreadyInProgressTitle => 'Cannot edit message'; + + @override + String get editAlreadyInProgressMessage => + 'An edit is already in progress. Please wait for it to complete.'; + + @override + String get savingMessageEditLabel => 'SAVING EDIT…'; + + @override + String get savingMessageEditFailedLabel => 'EDIT NOT SAVED'; + + @override + String get discardDraftConfirmationDialogTitle => + 'Discard the message you’re writing?'; + + @override + String get discardDraftConfirmationDialogMessage => + 'When you edit a message, the content that was previously in the compose box is discarded.'; + + @override + String get discardDraftConfirmationDialogConfirmButton => 'Discard'; + @override String get composeBoxAttachFilesTooltip => 'Attach files'; @@ -307,6 +349,9 @@ class ZulipLocalizationsNb extends ZulipLocalizations { return 'Message $destination'; } + @override + String get preparingEditMessageContentInput => 'Preparing…'; + @override String get composeBoxSendTooltip => 'Send'; diff --git a/lib/generated/l10n/zulip_localizations_pl.dart b/lib/generated/l10n/zulip_localizations_pl.dart index 15f61bd882..657e1757d1 100644 --- a/lib/generated/l10n/zulip_localizations_pl.dart +++ b/lib/generated/l10n/zulip_localizations_pl.dart @@ -127,6 +127,9 @@ class ZulipLocalizationsPl extends ZulipLocalizations { @override String get actionSheetOptionUnstarMessage => 'Odbierz gwiazdkę'; + @override + String get actionSheetOptionEditMessage => 'Edit message'; + @override String get actionSheetOptionMarkTopicAsRead => 'Oznacz wątek jako przeczytany'; @@ -197,6 +200,9 @@ class ZulipLocalizationsPl extends ZulipLocalizations { @override String get errorMessageNotSent => 'Nie wysłano wiadomości'; + @override + String get errorMessageEditNotSaved => 'Message not saved'; + @override String errorLoginCouldNotConnect(String url) { return 'Nie udało się połączyć z serwerem:\n$url'; @@ -269,6 +275,9 @@ class ZulipLocalizationsPl extends ZulipLocalizations { String get errorUnstarMessageFailedTitle => 'Odebranie gwiazdki bez powodzenia'; + @override + String get errorCouldNotEditMessageTitle => 'Could not edit message'; + @override String get successLinkCopied => 'Skopiowano odnośnik'; @@ -286,6 +295,39 @@ class ZulipLocalizationsPl extends ZulipLocalizations { String get errorBannerCannotPostInChannelLabel => 'Nie masz uprawnień do dodawania wpisów w tym kanale.'; + @override + String get composeBoxBannerLabelEditMessage => 'Edit message'; + + @override + String get composeBoxBannerButtonCancel => 'Cancel'; + + @override + String get composeBoxBannerButtonSave => 'Save'; + + @override + String get editAlreadyInProgressTitle => 'Cannot edit message'; + + @override + String get editAlreadyInProgressMessage => + 'An edit is already in progress. Please wait for it to complete.'; + + @override + String get savingMessageEditLabel => 'SAVING EDIT…'; + + @override + String get savingMessageEditFailedLabel => 'EDIT NOT SAVED'; + + @override + String get discardDraftConfirmationDialogTitle => + 'Discard the message you’re writing?'; + + @override + String get discardDraftConfirmationDialogMessage => + 'When you edit a message, the content that was previously in the compose box is discarded.'; + + @override + String get discardDraftConfirmationDialogConfirmButton => 'Discard'; + @override String get composeBoxAttachFilesTooltip => 'Dołącz pliki'; @@ -314,6 +356,9 @@ class ZulipLocalizationsPl extends ZulipLocalizations { return 'Wiadomość do $destination'; } + @override + String get preparingEditMessageContentInput => 'Preparing…'; + @override String get composeBoxSendTooltip => 'Wyślij'; diff --git a/lib/generated/l10n/zulip_localizations_ru.dart b/lib/generated/l10n/zulip_localizations_ru.dart index e4248179e2..5d8899290d 100644 --- a/lib/generated/l10n/zulip_localizations_ru.dart +++ b/lib/generated/l10n/zulip_localizations_ru.dart @@ -127,6 +127,9 @@ class ZulipLocalizationsRu extends ZulipLocalizations { @override String get actionSheetOptionUnstarMessage => 'Снять отметку с сообщения'; + @override + String get actionSheetOptionEditMessage => 'Edit message'; + @override String get actionSheetOptionMarkTopicAsRead => 'Отметить тему как прочитанную'; @@ -197,6 +200,9 @@ class ZulipLocalizationsRu extends ZulipLocalizations { @override String get errorMessageNotSent => 'Сообщение не отправлено'; + @override + String get errorMessageEditNotSaved => 'Message not saved'; + @override String errorLoginCouldNotConnect(String url) { return 'Не удалось подключиться к серверу:\n$url'; @@ -270,6 +276,9 @@ class ZulipLocalizationsRu extends ZulipLocalizations { String get errorUnstarMessageFailedTitle => 'Не удалось снять отметку с сообщения'; + @override + String get errorCouldNotEditMessageTitle => 'Could not edit message'; + @override String get successLinkCopied => 'Ссылка скопирована'; @@ -287,6 +296,39 @@ class ZulipLocalizationsRu extends ZulipLocalizations { String get errorBannerCannotPostInChannelLabel => 'У вас нет права писать в этом канале.'; + @override + String get composeBoxBannerLabelEditMessage => 'Edit message'; + + @override + String get composeBoxBannerButtonCancel => 'Cancel'; + + @override + String get composeBoxBannerButtonSave => 'Save'; + + @override + String get editAlreadyInProgressTitle => 'Cannot edit message'; + + @override + String get editAlreadyInProgressMessage => + 'An edit is already in progress. Please wait for it to complete.'; + + @override + String get savingMessageEditLabel => 'SAVING EDIT…'; + + @override + String get savingMessageEditFailedLabel => 'EDIT NOT SAVED'; + + @override + String get discardDraftConfirmationDialogTitle => + 'Discard the message you’re writing?'; + + @override + String get discardDraftConfirmationDialogMessage => + 'When you edit a message, the content that was previously in the compose box is discarded.'; + + @override + String get discardDraftConfirmationDialogConfirmButton => 'Discard'; + @override String get composeBoxAttachFilesTooltip => 'Прикрепить файлы'; @@ -315,6 +357,9 @@ class ZulipLocalizationsRu extends ZulipLocalizations { return 'Сообщение для $destination'; } + @override + String get preparingEditMessageContentInput => 'Preparing…'; + @override String get composeBoxSendTooltip => 'Отправить'; diff --git a/lib/generated/l10n/zulip_localizations_sk.dart b/lib/generated/l10n/zulip_localizations_sk.dart index baded578ba..3ff534eca5 100644 --- a/lib/generated/l10n/zulip_localizations_sk.dart +++ b/lib/generated/l10n/zulip_localizations_sk.dart @@ -123,6 +123,9 @@ class ZulipLocalizationsSk extends ZulipLocalizations { @override String get actionSheetOptionUnstarMessage => 'Odhviezdičkovať správu'; + @override + String get actionSheetOptionEditMessage => 'Edit message'; + @override String get actionSheetOptionMarkTopicAsRead => 'Mark topic as read'; @@ -192,6 +195,9 @@ class ZulipLocalizationsSk extends ZulipLocalizations { @override String get errorMessageNotSent => 'Správa nebola odoslaná'; + @override + String get errorMessageEditNotSaved => 'Message not saved'; + @override String errorLoginCouldNotConnect(String url) { return 'Nepodarilo sa pripojiť na server:\n$url'; @@ -262,6 +268,9 @@ class ZulipLocalizationsSk extends ZulipLocalizations { @override String get errorUnstarMessageFailedTitle => 'Failed to unstar message'; + @override + String get errorCouldNotEditMessageTitle => 'Could not edit message'; + @override String get successLinkCopied => 'Link copied'; @@ -279,6 +288,39 @@ class ZulipLocalizationsSk extends ZulipLocalizations { String get errorBannerCannotPostInChannelLabel => 'You do not have permission to post in this channel.'; + @override + String get composeBoxBannerLabelEditMessage => 'Edit message'; + + @override + String get composeBoxBannerButtonCancel => 'Cancel'; + + @override + String get composeBoxBannerButtonSave => 'Save'; + + @override + String get editAlreadyInProgressTitle => 'Cannot edit message'; + + @override + String get editAlreadyInProgressMessage => + 'An edit is already in progress. Please wait for it to complete.'; + + @override + String get savingMessageEditLabel => 'SAVING EDIT…'; + + @override + String get savingMessageEditFailedLabel => 'EDIT NOT SAVED'; + + @override + String get discardDraftConfirmationDialogTitle => + 'Discard the message you’re writing?'; + + @override + String get discardDraftConfirmationDialogMessage => + 'When you edit a message, the content that was previously in the compose box is discarded.'; + + @override + String get discardDraftConfirmationDialogConfirmButton => 'Discard'; + @override String get composeBoxAttachFilesTooltip => 'Attach files'; @@ -307,6 +349,9 @@ class ZulipLocalizationsSk extends ZulipLocalizations { return 'Message $destination'; } + @override + String get preparingEditMessageContentInput => 'Preparing…'; + @override String get composeBoxSendTooltip => 'Send'; diff --git a/lib/generated/l10n/zulip_localizations_uk.dart b/lib/generated/l10n/zulip_localizations_uk.dart index 50b1029dd7..a756bdba6a 100644 --- a/lib/generated/l10n/zulip_localizations_uk.dart +++ b/lib/generated/l10n/zulip_localizations_uk.dart @@ -128,6 +128,9 @@ class ZulipLocalizationsUk extends ZulipLocalizations { String get actionSheetOptionUnstarMessage => 'Зняти позначку зірочки з повідомлення'; + @override + String get actionSheetOptionEditMessage => 'Edit message'; + @override String get actionSheetOptionMarkTopicAsRead => 'Позначити тему як прочитану'; @@ -197,6 +200,9 @@ class ZulipLocalizationsUk extends ZulipLocalizations { @override String get errorMessageNotSent => 'Повідомлення не надіслано'; + @override + String get errorMessageEditNotSaved => 'Message not saved'; + @override String errorLoginCouldNotConnect(String url) { return 'Не вдалося підключитися до сервера:\n$url'; @@ -270,6 +276,9 @@ class ZulipLocalizationsUk extends ZulipLocalizations { String get errorUnstarMessageFailedTitle => 'Не вдалося зняти позначку зірочки з повідомлення'; + @override + String get errorCouldNotEditMessageTitle => 'Could not edit message'; + @override String get successLinkCopied => 'Посилання скопійовано'; @@ -288,6 +297,39 @@ class ZulipLocalizationsUk extends ZulipLocalizations { String get errorBannerCannotPostInChannelLabel => 'Ви не маєте дозволу на публікацію в цьому каналі.'; + @override + String get composeBoxBannerLabelEditMessage => 'Edit message'; + + @override + String get composeBoxBannerButtonCancel => 'Cancel'; + + @override + String get composeBoxBannerButtonSave => 'Save'; + + @override + String get editAlreadyInProgressTitle => 'Cannot edit message'; + + @override + String get editAlreadyInProgressMessage => + 'An edit is already in progress. Please wait for it to complete.'; + + @override + String get savingMessageEditLabel => 'SAVING EDIT…'; + + @override + String get savingMessageEditFailedLabel => 'EDIT NOT SAVED'; + + @override + String get discardDraftConfirmationDialogTitle => + 'Discard the message you’re writing?'; + + @override + String get discardDraftConfirmationDialogMessage => + 'When you edit a message, the content that was previously in the compose box is discarded.'; + + @override + String get discardDraftConfirmationDialogConfirmButton => 'Discard'; + @override String get composeBoxAttachFilesTooltip => 'Прикріпити файли'; @@ -316,6 +358,9 @@ class ZulipLocalizationsUk extends ZulipLocalizations { return 'Надіслати повідомлення $destination'; } + @override + String get preparingEditMessageContentInput => 'Preparing…'; + @override String get composeBoxSendTooltip => 'Надіслати'; diff --git a/lib/widgets/action_sheet.dart b/lib/widgets/action_sheet.dart index 4beea3db66..43db39dc18 100644 --- a/lib/widgets/action_sheet.dart +++ b/lib/widgets/action_sheet.dart @@ -12,6 +12,7 @@ import '../api/model/model.dart'; import '../api/route/channels.dart'; import '../api/route/messages.dart'; import '../generated/l10n/zulip_localizations.dart'; +import '../model/binding.dart'; import '../model/emoji.dart'; import '../model/internal_link.dart'; import '../model/narrow.dart'; @@ -576,6 +577,8 @@ void showMessageActionSheet({required BuildContext context, required Message mes CopyMessageTextButton(message: message, pageContext: pageContext), CopyMessageLinkButton(message: message, pageContext: pageContext), ShareButton(message: message, pageContext: pageContext), + if (_getShouldShowEditButton(pageContext, message)) + EditButton(message: message, pageContext: pageContext), ]; _showActionSheet(pageContext, optionButtons: optionButtons); @@ -591,6 +594,36 @@ abstract class MessageActionSheetMenuItemButton extends ActionSheetMenuItemButto final Message message; } +bool _getShouldShowEditButton(BuildContext pageContext, Message message) { + final store = PerAccountStoreWidget.of(pageContext); + + final messageListPage = MessageListPage.ancestorOf(pageContext); + final composeBoxState = messageListPage.composeBoxState; + final isComposeBoxOffered = composeBoxState != null; + final composeBoxController = composeBoxState?.controller; + + final editMessageErrorStatus = store.getEditMessageErrorStatus(message.id); + final editMessageInProgress = + // The compose box is in edit-message mode, with Cancel/Save instead of Send. + composeBoxController is EditMessageComposeBoxController + // An edit request is in progress or the error state. + || editMessageErrorStatus != null; + + final now = ZulipBinding.instance.utcNow().millisecondsSinceEpoch ~/ 1000; + final editLimit = store.realmMessageContentEditLimitSeconds; + final outsideEditLimit = + editLimit != null + && editLimit != 0 // TODO(server-6) remove (pre-FL 138, 0 represents no limit) + && now - message.timestamp > editLimit; + + return message.senderId == store.selfUserId + && isComposeBoxOffered + && store.realmAllowMessageEditing + && !outsideEditLimit + && !editMessageInProgress + && message.poll == null; // messages with polls cannot be edited +} + class ReactionButtons extends StatelessWidget { const ReactionButtons({ super.key, @@ -955,3 +988,22 @@ class ShareButton extends MessageActionSheetMenuItemButton { } } } + +class EditButton extends MessageActionSheetMenuItemButton { + EditButton({super.key, required super.message, required super.pageContext}); + + @override + IconData get icon => ZulipIcons.edit; + + @override + String label(ZulipLocalizations zulipLocalizations) => + zulipLocalizations.actionSheetOptionEditMessage; + + @override void onPressed() async { + final composeBoxState = findMessageListPage().composeBoxState; + if (composeBoxState == null) { + throw StateError('Compose box unexpectedly absent when edit-message button pressed'); + } + composeBoxState.startEditInteraction(message.id); + } +} diff --git a/lib/widgets/compose_box.dart b/lib/widgets/compose_box.dart index dfc55bf193..d629e69029 100644 --- a/lib/widgets/compose_box.dart +++ b/lib/widgets/compose_box.dart @@ -3,6 +3,7 @@ import 'dart:math'; import 'package:app_settings/app_settings.dart'; import 'package:flutter/material.dart'; +import 'package:flutter/scheduler.dart'; import 'package:flutter/services.dart'; import 'package:mime/mime.dart'; @@ -14,7 +15,9 @@ import '../model/binding.dart'; import '../model/compose.dart'; import '../model/narrow.dart'; import '../model/store.dart'; +import 'actions.dart'; import 'autocomplete.dart'; +import 'button.dart'; import 'color.dart'; import 'dialog.dart'; import 'icons.dart'; @@ -228,10 +231,13 @@ enum ContentValidationError { } class ComposeContentController extends ComposeController { - ComposeContentController({super.text}) { + ComposeContentController({super.text, this.requireNotEmpty = true}) { _update(); } + /// Whether to produce [ContentValidationError.empty]. + final bool requireNotEmpty; + // TODO(#1237) use `max_message_length` instead of hardcoded limit @override final maxLengthUnicodeCodePoints = kMaxMessageLengthCodePoints; @@ -378,7 +384,7 @@ class ComposeContentController extends ComposeController @override List _computeValidationErrors() { return [ - if (textNormalized.isEmpty) + if (requireNotEmpty && textNormalized.isEmpty) ContentValidationError.empty, if ( @@ -492,13 +498,13 @@ class _ContentInput extends StatelessWidget { const _ContentInput({ required this.narrow, required this.controller, - required this.hintText, - this.enabled = true, // ignore: unused_element_parameter + this.hintText, + this.enabled = true, }); final Narrow narrow; final ComposeBoxController controller; - final String hintText; + final String? hintText; final bool enabled; static double maxHeight(BuildContext context) { @@ -873,6 +879,31 @@ class _FixedDestinationContentInput extends StatelessWidget { } } +class _EditMessageContentInput extends StatelessWidget { + const _EditMessageContentInput({ + required this.narrow, + required this.controller, + }); + + final Narrow narrow; + final EditMessageComposeBoxController controller; + + @override + Widget build(BuildContext context) { + final zulipLocalizations = ZulipLocalizations.of(context); + final awaitingRawContent = ComposeBoxInheritedWidget.of(context) + .awaitingRawMessageContentForEdit; + return _ContentInput( + narrow: narrow, + controller: controller, + enabled: !awaitingRawContent, + hintText: awaitingRawContent + ? zulipLocalizations.preparingEditMessageContentInput + : null, + ); + } +} + /// Data on a file to be uploaded, from any source. /// /// A convenience class to represent data from the generic file picker, @@ -1375,7 +1406,7 @@ abstract class _ComposeBoxBody extends StatelessWidget { Widget? buildTopicInput(); Widget buildContentInput(); bool getComposeButtonsEnabled(BuildContext context); - Widget buildSendButton(); + Widget? buildSendButton(); @override Widget build(BuildContext context) { @@ -1409,6 +1440,7 @@ abstract class _ComposeBoxBody extends StatelessWidget { ]; final topicInput = buildTopicInput(); + final sendButton = buildSendButton(); return Column(children: [ Padding( padding: const EdgeInsets.symmetric(horizontal: 8), @@ -1426,7 +1458,7 @@ abstract class _ComposeBoxBody extends StatelessWidget { mainAxisAlignment: MainAxisAlignment.spaceBetween, children: [ Row(children: composeButtons), - buildSendButton(), + if (sendButton != null) sendButton, ]))), ]); } @@ -1488,6 +1520,28 @@ class _FixedDestinationComposeBoxBody extends _ComposeBoxBody { ); } +/// A compose box for editing an already-sent message. +class _EditMessageComposeBoxBody extends _ComposeBoxBody { + _EditMessageComposeBoxBody({required this.narrow, required this.controller}); + + @override + final Narrow narrow; + + @override + final EditMessageComposeBoxController controller; + + @override Widget? buildTopicInput() => null; + + @override Widget buildContentInput() => _EditMessageContentInput( + narrow: narrow, + controller: controller); + + @override bool getComposeButtonsEnabled(BuildContext context) => + !ComposeBoxInheritedWidget.of(context).awaitingRawMessageContentForEdit; + + @override Widget? buildSendButton() => null; +} + sealed class ComposeBoxController { final content = ComposeContentController(); final contentFocusNode = FocusNode(); @@ -1566,6 +1620,28 @@ class StreamComposeBoxController extends ComposeBoxController { class FixedDestinationComposeBoxController extends ComposeBoxController {} +class EditMessageComposeBoxController extends ComposeBoxController { + EditMessageComposeBoxController({ + required this.messageId, + required this.originalRawContent, + required String? initialText, + }) : _content = ComposeContentController( + text: initialText, + // Editing to delete the content is a supported form of + // deletion: https://zulip.com/help/delete-a-message#delete-message-content + requireNotEmpty: false); + + factory EditMessageComposeBoxController.empty(int messageId) => + EditMessageComposeBoxController(messageId: messageId, + originalRawContent: null, initialText: null); + + @override ComposeContentController get content => _content; + final ComposeContentController _content; + + final int messageId; + String? originalRawContent; +} + abstract class _Banner extends StatelessWidget { const _Banner(); @@ -1656,6 +1732,68 @@ class _ErrorBanner extends _Banner { } } +class _EditMessageBanner extends _Banner { + const _EditMessageBanner({required this.composeBoxState}); + + final ComposeBoxState composeBoxState; + + @override + String getLabel(ZulipLocalizations zulipLocalizations) => + zulipLocalizations.composeBoxBannerLabelEditMessage; + + @override + Color getLabelColor(DesignVariables designVariables) => + designVariables.bannerTextIntInfo; + + @override + Color getBackgroundColor(DesignVariables designVariables) => + designVariables.bannerBgIntInfo; + + void _handleTapSave (BuildContext context) { + final store = PerAccountStoreWidget.of(context); + final controller = composeBoxState.controller; + if (controller is! EditMessageComposeBoxController) return; // TODO(log) + final zulipLocalizations = ZulipLocalizations.of(context); + + if (controller.content.hasValidationErrors.value) { + final validationErrorMessages = + controller.content.validationErrors.map((error) => + error.message(zulipLocalizations)); + showErrorDialog(context: context, + title: zulipLocalizations.errorMessageEditNotSaved, + message: validationErrorMessages.join('\n\n')); + return; + } + + final originalRawContent = controller.originalRawContent; + if (originalRawContent == null) { + // Fetch-raw-content request hasn't finished; try again later. + // TODO show error dialog? + return; + } + + store.editMessage( + messageId: controller.messageId, + originalRawContent: originalRawContent, + newContent: controller.content.textNormalized); + composeBoxState.endEditInteraction(); + } + + @override + Widget buildTrailing(context) { + final zulipLocalizations = ZulipLocalizations.of(context); + return Row(mainAxisSize: MainAxisSize.min, spacing: 8, children: [ + ZulipWebUiKitButton(label: zulipLocalizations.composeBoxBannerButtonCancel, + onPressed: composeBoxState.endEditInteraction), + // TODO(#1481) disabled appearance when there are validation errors + // or the original raw content hasn't loaded yet + ZulipWebUiKitButton(label: zulipLocalizations.composeBoxBannerButtonSave, + attention: ZulipWebUiKitButtonAttention.high, + onPressed: () => _handleTapSave(context)), + ]); + } +} + /// The compose box. /// /// Takes the full screen width, covering the horizontal insets with its surface. @@ -1687,12 +1825,146 @@ class ComposeBox extends StatefulWidget { /// The interface for the state of a [ComposeBox]. abstract class ComposeBoxState extends State { ComposeBoxController get controller; + + /// Switch the compose box to editing mode. + /// + /// If there is already text in the compose box, gives a confirmation dialog + /// to confirm that it is OK to discard that text. + /// + /// If called from the message action sheet, fetches the raw message content + /// to fill in the edit-message compose box. + /// + /// If called by tapping a message in the message list with 'EDIT NOT SAVED', + /// fills the edit-message compose box with the content the user wanted + /// in the edit request that failed. + void startEditInteraction(int messageId); + + /// Switch the compose box back to regular non-edit mode, with no content. + void endEditInteraction(); } class _ComposeBoxState extends State with PerAccountStoreAwareStateMixin implements ComposeBoxState { @override ComposeBoxController get controller => _controller!; ComposeBoxController? _controller; + @override + void startEditInteraction(int messageId) async { + if (await _abortBecauseContentInputNotEmpty()) return; + if (!mounted) return; + + final store = PerAccountStoreWidget.of(context); + final zulipLocalizations = ZulipLocalizations.of(context); + + switch (store.getEditMessageErrorStatus(messageId)) { + case null: + _editFromRawContentFetch(messageId); + case true: + _editByRestoringFailedEdit(messageId); + case false: + // This can happen if you start an edit interaction on one + // MessageListPage and then do an edit on a different MessageListPage, + // and the second edit is still saving when you return to the first. + // + // Abort rather than sending a request with a prevContentSha256 + // that the server might not accept, and don't clear the compose + // box, so the user can try again after the request settles. + // TODO could write a test for this + showErrorDialog(context: context, + title: zulipLocalizations.editAlreadyInProgressTitle, + message: zulipLocalizations.editAlreadyInProgressMessage); + return; + } + } + + /// If there's text in the compose box, give a confirmation dialog + /// asking if it can be discarded and await the result. + Future _abortBecauseContentInputNotEmpty() async { + final zulipLocalizations = ZulipLocalizations.of(context); + if (controller.content.textNormalized.isNotEmpty) { + final dialog = showSuggestedActionDialog(context: context, + title: zulipLocalizations.discardDraftConfirmationDialogTitle, + message: zulipLocalizations.discardDraftConfirmationDialogMessage, + // TODO(#1032) "destructive" style for action button + actionButtonText: zulipLocalizations.discardDraftConfirmationDialogConfirmButton); + if (await dialog.result != true) return true; + } + return false; + } + + void _editByRestoringFailedEdit(int messageId) { + final store = PerAccountStoreWidget.of(context); + // Fill the content input with the content the user wanted in the failed + // edit attempt, not the original content. + // Side effect: Clears the "EDIT NOT SAVED" text in the message list. + final failedEdit = store.takeFailedMessageEdit(messageId); + setState(() { + controller.dispose(); + _controller = EditMessageComposeBoxController( + messageId: messageId, + originalRawContent: failedEdit.originalRawContent, + initialText: failedEdit.newContent, + ) + ..contentFocusNode.requestFocus(); + }); + } + + void _editFromRawContentFetch(int messageId) async { + final zulipLocalizations = ZulipLocalizations.of(context); + final emptyEditController = EditMessageComposeBoxController.empty(messageId); + setState(() { + controller.dispose(); + _controller = emptyEditController; + }); + final fetchedRawContent = await ZulipAction.fetchRawContentWithFeedback( + context: context, + messageId: messageId, + errorDialogTitle: zulipLocalizations.errorCouldNotEditMessageTitle, + ); + // TODO timeout this request? + if (!mounted) return; + if (!identical(controller, emptyEditController)) { + // user tapped Cancel during the fetch-raw-content request + // TODO in this case we don't want the error dialog caused by + // ZulipAction.fetchRawContentWithFeedback; suppress that + return; + } + if (fetchedRawContent == null) { + // Fetch-raw-content failed; abort the edit session. + // An error dialog was already shown, by fetchRawContentWithFeedback. + setState(() { + controller.dispose(); + _setNewController(PerAccountStoreWidget.of(context)); + }); + return; + } + // TODO scroll message list to ensure the message is still in view; + // highlight it? + assert(controller is EditMessageComposeBoxController); + final editMessageController = controller as EditMessageComposeBoxController; + setState(() { + // setState to refresh the input, upload buttons, etc. + // out of the disabled "Preparing…" state. + editMessageController.originalRawContent = fetchedRawContent; + }); + editMessageController.content.value = TextEditingValue(text: fetchedRawContent); + SchedulerBinding.instance.addPostFrameCallback((_) { + // post-frame callback so this happens after the input is enabled + editMessageController.contentFocusNode.requestFocus(); + }); + } + + @override + void endEditInteraction() { + assert(controller is EditMessageComposeBoxController); + if (controller is! EditMessageComposeBoxController) return; // TODO(log) + + final store = PerAccountStoreWidget.of(context); + setState(() { + controller.dispose(); + _setNewController(store); + }); + } + @override void onNewStore() { final newStore = PerAccountStoreWidget.of(context); @@ -1707,6 +1979,7 @@ class _ComposeBoxState extends State with PerAccountStoreAwareStateM case StreamComposeBoxController(): controller.topic.store = newStore; case FixedDestinationComposeBoxController(): + case EditMessageComposeBoxController(): // no reference to the store that needs updating } } @@ -1762,14 +2035,15 @@ class _ComposeBoxState extends State with PerAccountStoreAwareStateM @override Widget build(BuildContext context) { - final Widget? body; - final errorBanner = _errorBannerComposingNotAllowed(context); if (errorBanner != null) { return ComposeBoxInheritedWidget.fromComposeBoxState(this, child: _ComposeBoxContainer(body: null, banner: errorBanner)); } + final Widget? body; + Widget? banner; + final controller = this.controller; final narrow = widget.narrow; switch (controller) { @@ -1781,6 +2055,10 @@ class _ComposeBoxState extends State with PerAccountStoreAwareStateM narrow as SendableNarrow; body = _FixedDestinationComposeBoxBody(controller: controller, narrow: narrow); } + case EditMessageComposeBoxController(): { + body = _EditMessageComposeBoxBody(controller: controller, narrow: narrow); + banner = _EditMessageBanner(composeBoxState: this); + } } // TODO(#720) dismissable message-send error, maybe something like: @@ -1789,7 +2067,7 @@ class _ComposeBoxState extends State with PerAccountStoreAwareStateM // ZulipLocalizations.of(context).errorSendMessageTimeout); // } return ComposeBoxInheritedWidget.fromComposeBoxState(this, - child: _ComposeBoxContainer(body: body, banner: null)); + child: _ComposeBoxContainer(body: body, banner: banner)); } } @@ -1800,23 +2078,25 @@ class ComposeBoxInheritedWidget extends InheritedWidget { ComposeBoxState state, { required Widget child, }) { + final controller = state.controller; return ComposeBoxInheritedWidget._( - // TODO add fields + awaitingRawMessageContentForEdit: + controller is EditMessageComposeBoxController + && controller.originalRawContent == null, child: child, ); } const ComposeBoxInheritedWidget._({ - // TODO add fields + required this.awaitingRawMessageContentForEdit, required super.child, }); - // TODO add fields + final bool awaitingRawMessageContentForEdit; @override bool updateShouldNotify(covariant ComposeBoxInheritedWidget oldWidget) => - // TODO compare fields - false; + awaitingRawMessageContentForEdit != oldWidget.awaitingRawMessageContentForEdit; static ComposeBoxInheritedWidget of(BuildContext context) { final widget = context.dependOnInheritedWidgetOfExactType(); diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index 8c280eac1c..72f646cc65 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -1449,6 +1449,7 @@ class MessageWithPossibleSender extends StatelessWidget { @override Widget build(BuildContext context) { + final store = PerAccountStoreWidget.of(context); final designVariables = DesignVariables.of(context); final message = item.message; @@ -1473,6 +1474,25 @@ class MessageWithPossibleSender extends StatelessWidget { child: Icon(ZulipIcons.star_filled, size: 16, color: designVariables.star)); } + Widget content = MessageContent(message: message, content: item.content); + + final editMessageErrorStatus = store.getEditMessageErrorStatus(message.id); + if (editMessageErrorStatus != null) { + // The Figma also fades the sender row: + // https://github.com/zulip/zulip-flutter/pull/1498#discussion_r2076574000 + // We've decided to just fade the message content because that's the only + // thing that's changing. + content = Opacity(opacity: 0.6, child: content); + if (!editMessageErrorStatus) { + // IgnorePointer neutralizes interactable message content like links; + // this seemed appropriate along with the faded appearance. + content = IgnorePointer(child: content); + } else { + content = _RestoreEditMessageGestureDetector(messageId: message.id, + child: content); + } + } + return GestureDetector( behavior: HitTestBehavior.translucent, onLongPress: () => showMessageActionSheet(context: context, message: message), @@ -1489,10 +1509,12 @@ class MessageWithPossibleSender extends StatelessWidget { Expanded(child: Column( crossAxisAlignment: CrossAxisAlignment.stretch, children: [ - MessageContent(message: message, content: item.content), + content, if ((message.reactions?.total ?? 0) > 0) ReactionChipsList(messageId: message.id, reactions: message.reactions!), - if (editStateText != null) + if (editMessageErrorStatus != null) + _EditMessageStatusRow(messageId: message.id, status: editMessageErrorStatus) + else if (editStateText != null) Text(editStateText, textAlign: TextAlign.end, style: TextStyle( @@ -1508,3 +1530,75 @@ class MessageWithPossibleSender extends StatelessWidget { ]))); } } + +class _EditMessageStatusRow extends StatelessWidget { + const _EditMessageStatusRow({ + required this.messageId, + required this.status, + }); + + final int messageId; + final bool status; + + @override + Widget build(BuildContext context) { + final designVariables = DesignVariables.of(context); + final zulipLocalizations = ZulipLocalizations.of(context); + + final baseTextStyle = TextStyle( + fontSize: 12, + height: 12 / 12, + letterSpacing: proportionalLetterSpacing(context, + 0.05, baseFontSize: 12)); + + return switch (status) { + // TODO parse markdown and show new content as local echo? + false => Column( + crossAxisAlignment: CrossAxisAlignment.stretch, + spacing: 1.5, + children: [ + Text( + style: baseTextStyle + .copyWith(color: designVariables.btnLabelAttLowIntInfo), + textAlign: TextAlign.end, + zulipLocalizations.savingMessageEditLabel), + // TODO instead place within bottom outer padding: + // https://github.com/zulip/zulip-flutter/pull/1498#discussion_r2087576108 + LinearProgressIndicator( + minHeight: 2, + color: designVariables.foreground.withValues(alpha: 0.5), + backgroundColor: designVariables.foreground.withValues(alpha: 0.2), + ), + ]), + true => _RestoreEditMessageGestureDetector( + messageId: messageId, + child: Text( + style: baseTextStyle + .copyWith(color: designVariables.btnLabelAttLowIntDanger), + textAlign: TextAlign.end, + zulipLocalizations.savingMessageEditFailedLabel)), + }; + } +} + +class _RestoreEditMessageGestureDetector extends StatelessWidget { + const _RestoreEditMessageGestureDetector({ + required this.messageId, + required this.child, + }); + + final int messageId; + final Widget child; + + @override + Widget build(BuildContext context) { + return GestureDetector( + behavior: HitTestBehavior.opaque, + onTap: () { + final composeBoxState = MessageListPage.ancestorOf(context).composeBoxState; + if (composeBoxState == null) return; + composeBoxState.startEditInteraction(messageId); + }, + child: child); + } +} diff --git a/lib/widgets/theme.dart b/lib/widgets/theme.dart index a533311869..276e308b2b 100644 --- a/lib/widgets/theme.dart +++ b/lib/widgets/theme.dart @@ -130,6 +130,8 @@ class DesignVariables extends ThemeExtension { static final light = DesignVariables._( background: const Color(0xffffffff), bannerBgIntDanger: const Color(0xfff2e4e4), + bannerBgIntInfo: const Color(0xffddecf6), + bannerTextIntInfo: const Color(0xff06037c), bgBotBar: const Color(0xfff6f6f6), bgContextMenu: const Color(0xfff2f2f2), bgCounterUnread: const Color(0xff666699).withValues(alpha: 0.15), @@ -144,6 +146,7 @@ class DesignVariables extends ThemeExtension { btnBgAttMediumIntInfoNormal: const Color(0xff3c6bff).withValues(alpha: 0.12), btnLabelAttHigh: const Color(0xffffffff), btnLabelAttLowIntDanger: const Color(0xffc0070a), + btnLabelAttLowIntInfo: const Color(0xff2347c6), btnLabelAttMediumIntDanger: const Color(0xffac0508), btnLabelAttMediumIntInfo: const Color(0xff1027a6), btnShadowAttMed: const Color(0xff000000).withValues(alpha: 0.20), @@ -187,6 +190,8 @@ class DesignVariables extends ThemeExtension { static final dark = DesignVariables._( background: const Color(0xff000000), bannerBgIntDanger: const Color(0xff461616), + bannerBgIntInfo: const Color(0xff00253d), + bannerTextIntInfo: const Color(0xffcbdbfd), bgBotBar: const Color(0xff222222), bgContextMenu: const Color(0xff262626), bgCounterUnread: const Color(0xff666699).withValues(alpha: 0.37), @@ -201,6 +206,7 @@ class DesignVariables extends ThemeExtension { btnBgAttMediumIntInfoNormal: const Color(0xff97b6fe).withValues(alpha: 0.12), btnLabelAttHigh: const Color(0xffffffff).withValues(alpha: 0.85), btnLabelAttLowIntDanger: const Color(0xffff8b7c), + btnLabelAttLowIntInfo: const Color(0xff84a8fd), btnLabelAttMediumIntDanger: const Color(0xffff8b7c), btnLabelAttMediumIntInfo: const Color(0xff97b6fe), btnShadowAttMed: const Color(0xffffffff).withValues(alpha: 0.21), @@ -252,6 +258,8 @@ class DesignVariables extends ThemeExtension { DesignVariables._({ required this.background, required this.bannerBgIntDanger, + required this.bannerBgIntInfo, + required this.bannerTextIntInfo, required this.bgBotBar, required this.bgContextMenu, required this.bgCounterUnread, @@ -266,6 +274,7 @@ class DesignVariables extends ThemeExtension { required this.btnBgAttMediumIntInfoNormal, required this.btnLabelAttHigh, required this.btnLabelAttLowIntDanger, + required this.btnLabelAttLowIntInfo, required this.btnLabelAttMediumIntDanger, required this.btnLabelAttMediumIntInfo, required this.btnShadowAttMed, @@ -318,6 +327,8 @@ class DesignVariables extends ThemeExtension { final Color background; final Color bannerBgIntDanger; + final Color bannerBgIntInfo; + final Color bannerTextIntInfo; final Color bgBotBar; final Color bgContextMenu; final Color bgCounterUnread; @@ -332,6 +343,7 @@ class DesignVariables extends ThemeExtension { final Color btnBgAttMediumIntInfoNormal; final Color btnLabelAttHigh; final Color btnLabelAttLowIntDanger; + final Color btnLabelAttLowIntInfo; final Color btnLabelAttMediumIntDanger; final Color btnLabelAttMediumIntInfo; final Color btnShadowAttMed; @@ -379,6 +391,8 @@ class DesignVariables extends ThemeExtension { DesignVariables copyWith({ Color? background, Color? bannerBgIntDanger, + Color? bannerBgIntInfo, + Color? bannerTextIntInfo, Color? bgBotBar, Color? bgContextMenu, Color? bgCounterUnread, @@ -393,6 +407,7 @@ class DesignVariables extends ThemeExtension { Color? btnBgAttMediumIntInfoNormal, Color? btnLabelAttHigh, Color? btnLabelAttLowIntDanger, + Color? btnLabelAttLowIntInfo, Color? btnLabelAttMediumIntDanger, Color? btnLabelAttMediumIntInfo, Color? btnShadowAttMed, @@ -435,6 +450,8 @@ class DesignVariables extends ThemeExtension { return DesignVariables._( background: background ?? this.background, bannerBgIntDanger: bannerBgIntDanger ?? this.bannerBgIntDanger, + bannerBgIntInfo: bannerBgIntInfo ?? this.bannerBgIntInfo, + bannerTextIntInfo: bannerTextIntInfo ?? this.bannerTextIntInfo, bgBotBar: bgBotBar ?? this.bgBotBar, bgContextMenu: bgContextMenu ?? this.bgContextMenu, bgCounterUnread: bgCounterUnread ?? this.bgCounterUnread, @@ -449,6 +466,7 @@ class DesignVariables extends ThemeExtension { btnBgAttMediumIntInfoNormal: btnBgAttMediumIntInfoNormal ?? this.btnBgAttMediumIntInfoNormal, btnLabelAttHigh: btnLabelAttHigh ?? this.btnLabelAttHigh, btnLabelAttLowIntDanger: btnLabelAttLowIntDanger ?? this.btnLabelAttLowIntDanger, + btnLabelAttLowIntInfo: btnLabelAttLowIntInfo ?? this.btnLabelAttLowIntInfo, btnLabelAttMediumIntDanger: btnLabelAttMediumIntDanger ?? this.btnLabelAttMediumIntDanger, btnLabelAttMediumIntInfo: btnLabelAttMediumIntInfo ?? this.btnLabelAttMediumIntInfo, btnShadowAttMed: btnShadowAttMed ?? this.btnShadowAttMed, @@ -498,6 +516,8 @@ class DesignVariables extends ThemeExtension { return DesignVariables._( background: Color.lerp(background, other.background, t)!, bannerBgIntDanger: Color.lerp(bannerBgIntDanger, other.bannerBgIntDanger, t)!, + bannerBgIntInfo: Color.lerp(bannerBgIntInfo, other.bannerBgIntInfo, t)!, + bannerTextIntInfo: Color.lerp(bannerTextIntInfo, other.bannerTextIntInfo, t)!, bgBotBar: Color.lerp(bgBotBar, other.bgBotBar, t)!, bgContextMenu: Color.lerp(bgContextMenu, other.bgContextMenu, t)!, bgCounterUnread: Color.lerp(bgCounterUnread, other.bgCounterUnread, t)!, @@ -512,6 +532,7 @@ class DesignVariables extends ThemeExtension { btnBgAttMediumIntInfoNormal: Color.lerp(btnBgAttMediumIntInfoNormal, other.btnBgAttMediumIntInfoNormal, t)!, btnLabelAttHigh: Color.lerp(btnLabelAttHigh, other.btnLabelAttHigh, t)!, btnLabelAttLowIntDanger: Color.lerp(btnLabelAttLowIntDanger, other.btnLabelAttLowIntDanger, t)!, + btnLabelAttLowIntInfo: Color.lerp(btnLabelAttLowIntInfo, other.btnLabelAttLowIntInfo, t)!, btnLabelAttMediumIntDanger: Color.lerp(btnLabelAttMediumIntDanger, other.btnLabelAttMediumIntDanger, t)!, btnLabelAttMediumIntInfo: Color.lerp(btnLabelAttMediumIntInfo, other.btnLabelAttMediumIntInfo, t)!, btnShadowAttMed: Color.lerp(btnShadowAttMed, other.btnShadowAttMed, t)!, diff --git a/test/flutter_checks.dart b/test/flutter_checks.dart index 295dcde7b9..0bfbd2d33a 100644 --- a/test/flutter_checks.dart +++ b/test/flutter_checks.dart @@ -143,6 +143,10 @@ extension TextEditingControllerChecks on Subject { Subject get text => has((t) => t.text, 'text'); } +extension FocusNodeChecks on Subject { + Subject get hasFocus => has((t) => t.hasFocus, 'hasFocus'); +} + extension ScrollMetricsChecks on Subject { Subject get minScrollExtent => has((x) => x.minScrollExtent, 'minScrollExtent'); Subject get maxScrollExtent => has((x) => x.maxScrollExtent, 'maxScrollExtent'); diff --git a/test/widgets/action_sheet_test.dart b/test/widgets/action_sheet_test.dart index 7ab166bc44..ef7b8e64b3 100644 --- a/test/widgets/action_sheet_test.dart +++ b/test/widgets/action_sheet_test.dart @@ -23,6 +23,7 @@ import 'package:zulip/model/store.dart'; import 'package:zulip/model/typing_status.dart'; import 'package:zulip/widgets/action_sheet.dart'; import 'package:zulip/widgets/app_bar.dart'; +import 'package:zulip/widgets/button.dart'; import 'package:zulip/widgets/compose_box.dart'; import 'package:zulip/widgets/content.dart'; import 'package:zulip/widgets/emoji.dart'; @@ -52,11 +53,18 @@ late FakeApiConnection connection; Future setupToMessageActionSheet(WidgetTester tester, { required Message message, required Narrow narrow, + bool? realmAllowMessageEditing, + int? realmMessageContentEditLimitSeconds, }) async { addTearDown(testBinding.reset); assert(narrow.containsMessage(message)); - await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); + await testBinding.globalStore.add( + eg.selfAccount, + eg.initialSnapshot( + realmAllowMessageEditing: realmAllowMessageEditing, + realmMessageContentEditLimitSeconds: realmMessageContentEditLimitSeconds, + )); store = await testBinding.globalStore.perAccount(eg.selfAccount.id); await store.addUsers([ eg.selfUser, @@ -1432,6 +1440,169 @@ void main() { }); }); + group('EditButton', () { + Future tapEdit(WidgetTester tester) async { + await tester.ensureVisible(find.byIcon(ZulipIcons.edit, skipOffstage: false)); + await tester.tap(find.byIcon(ZulipIcons.edit)); + await tester.pump(); // [MenuItemButton.onPressed] called in a post-frame callback: flutter/flutter@e4a39fa2e + } + + group('present/absent appropriately', () { + /// Test whether the edit-message button is visible, given params. + /// + /// The message timestamp is 60s before the current time + /// ([TestZulipBinding.utcNow]) as of the start of the test run. + /// + /// The message has streamId: 1 and topic: 'topic'. + /// The message list is for that [TopicNarrow] unless [narrow] is passed. + void testVisibility(bool expected, { + bool self = true, + Narrow? narrow, + bool allowed = true, + int? limit, + bool boxInEditMode = false, + bool? errorStatus, + bool poll = false, + }) { + // It's inconvenient here to set up a state where the compose box + // is in edit mode and the action sheet is opened for a message + // with an edit request that's in progress or in the error state. + // In the setup, we'd need to either use two messages or (via an edge + // case) two MessageListPages. It should suffice to test the + // boxInEditMode and errorStatus states separately. + assert(!boxInEditMode || errorStatus == null); + + final description = [ + 'from self: $self', + 'narrow: $narrow', + 'realm allows: $allowed', + 'edit limit: $limit', + 'compose box is in editing mode: $boxInEditMode', + 'edit-message error status: $errorStatus', + 'has poll: $poll', + ].join(', '); + + void checkButtonIsPresent(bool expected) { + if (expected) { + check(find.byIcon(ZulipIcons.edit, skipOffstage: false)).findsOne(); + } else { + check(find.byIcon(ZulipIcons.edit, skipOffstage: false)).findsNothing(); + } + } + + testWidgets(description, (tester) async { + TypingNotifier.debugEnable = false; + addTearDown(TypingNotifier.debugReset); + + final message = eg.streamMessage( + stream: eg.stream(streamId: 1), + topic: 'topic', + sender: self ? eg.selfUser : eg.otherUser, + timestamp: eg.utcTimestamp(testBinding.utcNow()) - 60, + submessages: poll + ? [eg.submessage(content: eg.pollWidgetData(question: 'poll', options: ['A']))] + : null, + ); + + await setupToMessageActionSheet(tester, + message: message, + narrow: narrow ?? TopicNarrow.ofMessage(message), + realmAllowMessageEditing: allowed, + realmMessageContentEditLimitSeconds: limit, + ); + + if (!boxInEditMode && errorStatus == null) { + // The state we're testing is present on the original action sheet. + checkButtonIsPresent(expected); + return; + } + // The state we're testing requires a previous "edit message" action + // in order to set up. Use the first action sheet for that setup step. + + connection.prepare(json: GetMessageResult( + message: eg.streamMessage(content: 'foo')).toJson()); + await tapEdit(tester); + await tester.pump(Duration.zero); + await tester.enterText(find.byWidgetPredicate( + (widget) => widget is TextField && widget.controller?.text == 'foo'), + 'bar'); + + if (errorStatus == true) { + // We're testing the request-failed state. Prepare a failure + // and tap Save. + connection.prepare(apiException: eg.apiBadRequest()); + await tester.tap(find.widgetWithText(ZulipWebUiKitButton, 'Save')); + await tester.pump(Duration.zero); + } else if (errorStatus == false) { + // We're testing the request-in-progress state. Prepare a delay, + // tap Save, and wait through only part of the delay. + connection.prepare( + json: UpdateMessageResult().toJson(), delay: Duration(seconds: 1)); + await tester.tap(find.widgetWithText(ZulipWebUiKitButton, 'Save')); + await tester.pump(Duration(milliseconds: 500)); + } else { + // We're testing the state where the compose box is in + // edit-message mode. Keep it that way by not tapping Save. + } + + // See comment in setupToMessageActionSheet about warnIfMissed: false + await tester.longPress(find.byType(MessageContent), warnIfMissed: false); + // sheet appears onscreen; default duration of bottom-sheet enter animation + await tester.pump(const Duration(milliseconds: 250)); + check(find.byType(BottomSheet)).findsOne(); + checkButtonIsPresent(expected); + + await tester.pump(Duration(milliseconds: 500)); // flush timers + }); + } + + testVisibility(true); + // TODO(server-6) limit 0 not expected on 6.0+ + testVisibility(true, limit: 0); + testVisibility(true, limit: 600); + testVisibility(true, narrow: ChannelNarrow(1)); + + testVisibility(false, self: false); + testVisibility(false, narrow: CombinedFeedNarrow()); + testVisibility(false, allowed: false); + testVisibility(false, limit: 10); + testVisibility(false, boxInEditMode: true); + testVisibility(false, errorStatus: false); + testVisibility(false, errorStatus: true); + testVisibility(false, poll: true); + }); + + group('tap button', () { + ComposeBoxController? findComposeBoxController(WidgetTester tester) { + return tester.stateList(find.byType(ComposeBox)) + .singleOrNull?.controller; + } + + testWidgets('smoke', (tester) async { + final message = eg.streamMessage(sender: eg.selfUser); + await setupToMessageActionSheet(tester, + message: message, + narrow: TopicNarrow.ofMessage(message), + realmAllowMessageEditing: true, + realmMessageContentEditLimitSeconds: null, + ); + + check(findComposeBoxController(tester)) + .isA(); + + connection.prepare(json: GetMessageResult( + message: eg.streamMessage(content: 'foo')).toJson()); + await tapEdit(tester); + await tester.pump(Duration.zero); + + check(findComposeBoxController(tester)) + .isA() + ..messageId.equals(message.id) + ..originalRawContent.equals('foo'); + }); + }); + }); + group('MessageActionSheetCancelButton', () { final zulipLocalizations = GlobalLocalizations.zulipLocalizations; diff --git a/test/widgets/compose_box_checks.dart b/test/widgets/compose_box_checks.dart index 8008b510d3..b93ff7f1bf 100644 --- a/test/widgets/compose_box_checks.dart +++ b/test/widgets/compose_box_checks.dart @@ -1,6 +1,21 @@ import 'package:checks/checks.dart'; +import 'package:flutter/cupertino.dart'; import 'package:zulip/widgets/compose_box.dart'; +extension ComposeBoxStateChecks on Subject { + Subject get controller => has((c) => c.controller, 'controller'); +} + +extension ComposeBoxControllerChecks on Subject { + Subject get content => has((c) => c.content, 'content'); + Subject get contentFocusNode => has((c) => c.contentFocusNode, 'contentFocusNode'); +} + +extension EditMessageComposeBoxControllerChecks on Subject { + Subject get messageId => has((c) => c.messageId, 'messageId'); + Subject get originalRawContent => has((c) => c.originalRawContent, 'originalRawContent'); +} + extension ComposeContentControllerChecks on Subject { Subject> get validationErrors => has((c) => c.validationErrors, 'validationErrors'); } diff --git a/test/widgets/compose_box_test.dart b/test/widgets/compose_box_test.dart index 24c4d403b3..679f4de190 100644 --- a/test/widgets/compose_box_test.dart +++ b/test/widgets/compose_box_test.dart @@ -3,11 +3,12 @@ import 'dart:convert'; import 'dart:io'; import 'package:checks/checks.dart'; +import 'package:crypto/crypto.dart'; import 'package:file_picker/file_picker.dart'; import 'package:flutter_checks/flutter_checks.dart'; +import 'package:flutter_test/flutter_test.dart'; import 'package:http/http.dart' as http; import 'package:flutter/material.dart'; -import 'package:flutter_test/flutter_test.dart'; import 'package:image_picker/image_picker.dart'; import 'package:zulip/api/model/events.dart'; import 'package:zulip/api/model/model.dart'; @@ -18,6 +19,7 @@ import 'package:zulip/model/narrow.dart'; import 'package:zulip/model/store.dart'; import 'package:zulip/model/typing_status.dart'; import 'package:zulip/widgets/app.dart'; +import 'package:zulip/widgets/button.dart'; import 'package:zulip/widgets/color.dart'; import 'package:zulip/widgets/compose_box.dart'; import 'package:zulip/widgets/message_list.dart'; @@ -33,6 +35,7 @@ import '../model/store_checks.dart'; import '../model/test_store.dart'; import '../model/typing_status_test.dart'; import '../stdlib_checks.dart'; +import 'compose_box_checks.dart'; import 'dialog_checks.dart'; import 'test_app.dart'; @@ -41,6 +44,10 @@ void main() { late PerAccountStore store; late FakeApiConnection connection; + late ComposeBoxState state; + + // Caution: when testing edit-message UI, this will often be stale; + // read state.controller instead. late ComposeBoxController? controller; Future prepareComposeBox(WidgetTester tester, { @@ -64,6 +71,8 @@ void main() { streams: streams, zulipFeatureLevel: zulipFeatureLevel, realmMandatoryTopics: mandatoryTopics, + realmAllowMessageEditing: true, + realmMessageContentEditLimitSeconds: null, )); store = await testBinding.globalStore.perAccount(selfAccount.id); @@ -77,7 +86,8 @@ void main() { await tester.pumpAndSettle(); connection.takeRequests(); - controller = tester.state(find.byType(ComposeBox)).controller; + state = tester.state(find.byType(ComposeBox)); + controller = state.controller; } /// A [Finder] for the topic input. @@ -232,6 +242,33 @@ void main() { '\n\n^\n\n', 'a\n', '\n\na\n\n^\n'); }); }); + + group('ContentValidationError.empty', () { + late ComposeContentController controller; + + void checkCountsAsEmpty(String text, bool expected) { + controller.value = TextEditingValue(text: text); + expected + ? check(controller).validationErrors.contains(ContentValidationError.empty) + : check(controller).validationErrors.not((it) => it.contains(ContentValidationError.empty)); + } + + testWidgets('requireNotEmpty: true (default)', (tester) async { + controller = ComposeContentController(); + addTearDown(controller.dispose); + checkCountsAsEmpty('', true); + checkCountsAsEmpty(' ', true); + checkCountsAsEmpty('a', false); + }); + + testWidgets('requireNotEmpty: false', (tester) async { + controller = ComposeContentController(requireNotEmpty: false); + addTearDown(controller.dispose); + checkCountsAsEmpty('', false); + checkCountsAsEmpty(' ', false); + checkCountsAsEmpty('a', false); + }); + }); }); group('length validation', () { @@ -1363,4 +1400,468 @@ void main() { checkContentInputValue(tester, 'some content'); }); }); + + group('edit message', () { + final channel = eg.stream(); + final topic = 'topic'; + final message = eg.streamMessage(sender: eg.selfUser, stream: channel, topic: topic); + final dmMessage = eg.dmMessage(from: eg.selfUser, to: [eg.otherUser]); + + final channelNarrow = ChannelNarrow(channel.streamId); + final topicNarrow = eg.topicNarrow(channel.streamId, topic); + final dmNarrow = DmNarrow.ofMessage(dmMessage, selfUserId: eg.selfUser.userId); + + Message msgInNarrow(Narrow narrow) { + final List messages = [message, dmMessage]; + return messages.where((m) => narrow.containsMessage(m)).single; + } + + int msgIdInNarrow(Narrow narrow) => msgInNarrow(narrow).id; + + Future prepareEditMessage(WidgetTester tester, {required Narrow narrow}) async { + await prepareComposeBox(tester, + narrow: narrow, + streams: [channel]); + await store.addMessages([message, dmMessage]); + await tester.pump(); // message list updates + } + + /// Check that the compose box is in the "Preparing…" state, + /// awaiting the fetch-raw-content request. + Future checkAwaitingRawMessageContent(WidgetTester tester) async { + check(state.controller) + .isA() + ..originalRawContent.isNull() + ..content.value.text.equals(''); + check(tester.widget(contentInputFinder)) + .isA() + .decoration.isNotNull().hintText.equals('Preparing…'); + checkContentInputValue(tester, ''); + + // Controls are disabled + await tester.tap(find.byIcon(ZulipIcons.attach_file), warnIfMissed: false); + await tester.pump(); + check(testBinding.takePickFilesCalls()).isEmpty(); + + // Save button is disabled + final lastRequest = connection.lastRequest; + await tester.tap( + find.widgetWithText(ZulipWebUiKitButton, 'Save'), warnIfMissed: false); + await tester.pump(Duration.zero); + check(connection.lastRequest).equals(lastRequest); + } + + /// Starts an interaction from the action sheet's 'Edit message' button. + /// + /// The fetch-raw-content request is prepared with [delay] (default 1s). + Future startInteractionFromActionSheet( + WidgetTester tester, { + required int messageId, + String originalRawContent = 'foo', + Duration delay = const Duration(seconds: 1), + bool fetchShouldSucceed = true, + }) async { + await tester.longPress(find.byWidgetPredicate((widget) => + widget is MessageWithPossibleSender && widget.item.message.id == messageId)); + // sheet appears onscreen; default duration of bottom-sheet enter animation + await tester.pump(const Duration(milliseconds: 250)); + final findEditButton = find.descendant( + of: find.byType(BottomSheet), + matching: find.byIcon(ZulipIcons.edit, skipOffstage: false)); + await tester.ensureVisible(findEditButton); + if (fetchShouldSucceed) { + connection.prepare(delay: delay, + json: GetMessageResult(message: eg.streamMessage(content: originalRawContent)).toJson()); + } else { + connection.prepare(apiException: eg.apiBadRequest(), delay: delay); + } + await tester.tap(findEditButton); + await tester.pump(); + await tester.pump(); + connection.takeRequests(); + } + + /// Starts an interaction by tapping a failed edit in the message list. + Future startInteractionFromRestoreFailedEdit( + WidgetTester tester, { + required int messageId, + String originalRawContent = 'foo', + String newContent = 'bar', + }) async { + await startInteractionFromActionSheet(tester, + messageId: messageId, originalRawContent: originalRawContent); + await tester.pump(Duration(seconds: 1)); // raw-content request + await enterContent(tester, newContent); + + connection.prepare(apiException: eg.apiBadRequest()); + await tester.tap(find.widgetWithText(ZulipWebUiKitButton, 'Save')); + await tester.pump(Duration.zero); + await tester.tap(find.text('EDIT NOT SAVED')); + await tester.pump(); + connection.takeRequests(); + } + + void checkRequest(int messageId, { + required String prevContent, + required String content, + }) { + final prevContentSha256 = sha256.convert(utf8.encode(prevContent)).toString(); + check(connection.takeRequests()).single.isA() + ..method.equals('PATCH') + ..url.path.equals('/api/v1/messages/$messageId') + ..bodyFields.deepEquals({ + 'prev_content_sha256': prevContentSha256, + 'content': content, + }); + } + + /// Check that the compose box is not in editing mode. + void checkNotInEditingMode(WidgetTester tester, { + required Narrow narrow, + String expectedContentText = '', + }) { + switch (narrow) { + case ChannelNarrow(): + check(state.controller) + .isA() + .content.value.text.equals(expectedContentText); + case TopicNarrow(): + case DmNarrow(): + check(state.controller) + .isA() + .content.value.text.equals(expectedContentText); + default: + throw StateError('unexpected narrow type'); + } + checkContentInputValue(tester, expectedContentText); + } + + void testSmoke({required Narrow narrow, required _EditInteractionStart start}) { + testWidgets('smoke: $narrow, ${start.message()}', (tester) async { + await prepareEditMessage(tester, narrow: narrow); + checkNotInEditingMode(tester, narrow: narrow); + + final messageId = msgIdInNarrow(narrow); + switch (start) { + case _EditInteractionStart.actionSheet: + await startInteractionFromActionSheet(tester, + messageId: messageId, + originalRawContent: 'foo'); + await checkAwaitingRawMessageContent(tester); + await tester.pump(Duration(seconds: 1)); // fetch-raw-content request + checkContentInputValue(tester, 'foo'); + case _EditInteractionStart.restoreFailedEdit: + await startInteractionFromRestoreFailedEdit(tester, + messageId: messageId, + originalRawContent: 'foo', + newContent: 'bar'); + checkContentInputValue(tester, 'bar'); + } + + // Now that we have the raw content, check the input is interactive + // but no typing notifications are sent… + check(TypingNotifier.debugEnable).isTrue(); + check(state).controller.contentFocusNode.hasFocus.isTrue(); + await enterContent(tester, 'some new content'); + check(connection.takeRequests()).isEmpty(); + + // …and the upload buttons work. + testBinding.pickFilesResult = FilePickerResult([ + PlatformFile(name: 'file.jpg', size: 1000, readStream: Stream.fromIterable(['asdf'.codeUnits]))]); + connection.prepare(json: + UploadFileResult(uri: '/path/file.jpg').toJson()); + await tester.tap(find.byIcon(ZulipIcons.attach_file), warnIfMissed: false); + await tester.pump(Duration.zero); + checkNoErrorDialog(tester); + check(testBinding.takePickFilesCalls()).length.equals(1); + connection.takeRequests(); // upload request + + // TODO could also check that quote-and-reply and autocomplete work + // (but as their own test cases, for a single narrow and start) + + // Save; check that the request is made and the compose box resets. + connection.prepare(json: UpdateMessageResult().toJson()); + await tester.tap(find.widgetWithText(ZulipWebUiKitButton, 'Save')); + checkRequest(messageId, + prevContent: 'foo', content: 'some new content[file.jpg](/path/file.jpg)'); + await tester.pump(Duration.zero); + checkNotInEditingMode(tester, narrow: narrow); + }); + } + testSmoke(narrow: channelNarrow, start: _EditInteractionStart.actionSheet); + testSmoke(narrow: topicNarrow, start: _EditInteractionStart.actionSheet); + testSmoke(narrow: dmNarrow, start: _EditInteractionStart.actionSheet); + testSmoke(narrow: channelNarrow, start: _EditInteractionStart.restoreFailedEdit); + testSmoke(narrow: topicNarrow, start: _EditInteractionStart.restoreFailedEdit); + testSmoke(narrow: dmNarrow, start: _EditInteractionStart.restoreFailedEdit); + + Future expectAndHandleDiscardConfirmation( + WidgetTester tester, { + required bool shouldContinue, + }) async { + final (actionButton, cancelButton) = checkSuggestedActionDialog(tester, + expectedTitle: 'Discard the message you’re writing?', + expectedMessage: 'When you edit a message, the content that was previously in the compose box is discarded.', + expectedActionButtonText: 'Discard'); + if (shouldContinue) { + await tester.tap(find.byWidget(actionButton)); + } else { + await tester.tap(find.byWidget(cancelButton)); + } + } + + // Test the "Discard…?" confirmation dialog when you tap "Edit message" in + // the action sheet but there's text in the compose box for a new message. + void testInterruptComposingFromActionSheet({required Narrow narrow}) { + testWidgets('interrupting new-message compose: $narrow', (tester) async { + TypingNotifier.debugEnable = false; + addTearDown(TypingNotifier.debugReset); + + final messageId = msgIdInNarrow(narrow); + await prepareEditMessage(tester, narrow: narrow); + checkNotInEditingMode(tester, narrow: narrow); + + await enterContent(tester, 'composing new message'); + + // Expect confirmation dialog; tap Cancel + await startInteractionFromActionSheet(tester, messageId: messageId); + await expectAndHandleDiscardConfirmation(tester, shouldContinue: false); + check(connection.takeRequests()).isEmpty(); + // fetch-raw-content request wasn't actually sent; + // take back its prepared response + connection.clearPreparedResponses(); + + // Twiddle the input to make sure it still works + checkNotInEditingMode(tester, + narrow: narrow, expectedContentText: 'composing new message'); + await enterContent(tester, 'composing new message…'); + checkContentInputValue(tester, 'composing new message…'); + + // Try again, but this time tap Discard and expect to enter an edit session + await startInteractionFromActionSheet(tester, + messageId: messageId, originalRawContent: 'foo'); + await expectAndHandleDiscardConfirmation(tester, shouldContinue: true); + await tester.pump(); + await checkAwaitingRawMessageContent(tester); + await tester.pump(Duration(seconds: 1)); // fetch-raw-content request + check(connection.takeRequests()).length.equals(1); + checkContentInputValue(tester, 'foo'); + await enterContent(tester, 'bar'); + + // Save; check that the request is made and the compose box resets. + connection.prepare(json: UpdateMessageResult().toJson()); + await tester.tap(find.widgetWithText(ZulipWebUiKitButton, 'Save')); + checkRequest(messageId, prevContent: 'foo', content: 'bar'); + await tester.pump(Duration.zero); + checkNotInEditingMode(tester, narrow: narrow); + }); + } + // Cover multiple narrows, checking that the Discard button resets the state + // correctly for each one. + testInterruptComposingFromActionSheet(narrow: channelNarrow); + testInterruptComposingFromActionSheet(narrow: topicNarrow); + testInterruptComposingFromActionSheet(narrow: dmNarrow); + + // Test the "Discard…?" confirmation dialog when you want to restore + // a failed edit but there's text in the compose box for a new message. + void testInterruptComposingFromFailedEdit({required Narrow narrow}) { + testWidgets('interrupting new-message compose by tapping failed edit to restore: $narrow', (tester) async { + TypingNotifier.debugEnable = false; + addTearDown(TypingNotifier.debugReset); + + final messageId = msgIdInNarrow(narrow); + await prepareEditMessage(tester, narrow: narrow); + + await startInteractionFromActionSheet(tester, + messageId: messageId, originalRawContent: 'foo'); + await tester.pump(Duration(seconds: 1)); // raw-content request + await enterContent(tester, 'bar'); + + connection.prepare(apiException: eg.apiBadRequest()); + await tester.tap(find.widgetWithText(ZulipWebUiKitButton, 'Save')); + connection.takeRequests(); + await tester.pump(Duration.zero); + checkNotInEditingMode(tester, narrow: narrow); + check(find.text('EDIT NOT SAVED')).findsOne(); + + await enterContent(tester, 'composing new message'); + + // Expect confirmation dialog; tap Cancel + await tester.tap(find.text('EDIT NOT SAVED')); + await tester.pump(); + await expectAndHandleDiscardConfirmation(tester, shouldContinue: false); + checkNotInEditingMode(tester, + narrow: narrow, expectedContentText: 'composing new message'); + + // Twiddle the input to make sure it still works + await enterContent(tester, 'composing new message…'); + + // Try again, but this time tap Discard and expect to enter edit session + await tester.tap(find.text('EDIT NOT SAVED')); + await tester.pump(); + await expectAndHandleDiscardConfirmation(tester, shouldContinue: true); + await tester.pump(); + checkContentInputValue(tester, 'bar'); + await enterContent(tester, 'baz'); + + // Save; check that the request is made and the compose box resets. + connection.prepare(json: UpdateMessageResult().toJson()); + await tester.tap(find.widgetWithText(ZulipWebUiKitButton, 'Save')); + checkRequest(messageId, prevContent: 'foo', content: 'baz'); + await tester.pump(Duration.zero); + checkNotInEditingMode(tester, narrow: narrow); + }); + } + // (So tests run faster, skip some narrows that are already covered above.) + testInterruptComposingFromFailedEdit(narrow: channelNarrow); + // testInterruptComposingFromFailedEdit(narrow: topicNarrow); + // testInterruptComposingFromFailedEdit(narrow: dmNarrow); + + // TODO also test: + // - Restore a failed edit, but when there's compose input for an edit- + // message session. (The failed edit would be for a different message, + // or else started from a different MessageListPage.) + + void testFetchRawContentFails({required Narrow narrow}) { + final description = 'fetch-raw-content fails: $narrow'; + testWidgets(description, (tester) async { + await prepareEditMessage(tester, narrow: narrow); + checkNotInEditingMode(tester, narrow: narrow); + + final messageId = msgIdInNarrow(narrow); + await startInteractionFromActionSheet(tester, + messageId: messageId, + originalRawContent: 'foo', + fetchShouldSucceed: false); + await checkAwaitingRawMessageContent(tester); + await tester.pump(Duration(seconds: 1)); // fetch-raw-content request + checkErrorDialog(tester, expectedTitle: 'Could not edit message'); + checkNotInEditingMode(tester, narrow: narrow); + }); + } + // Skip some narrows so the tests run faster; + // the codepaths to be tested are basically the same. + // testFetchRawContentFails(narrow: channelNarrow); + testFetchRawContentFails(narrow: topicNarrow); + // testFetchRawContentFails(narrow: dmNarrow); + + /// Test that an edit session is really cleared by the Cancel button. + /// + /// If `start: _EditInteractionStart.actionSheet` (the default), + /// pass duringFetchRawContentRequest to control whether the Cancel button + /// is tapped during (true) or after (false) the fetch-raw-content request. + /// + /// If `start: _EditInteractionStart.restoreFailedEdit`, + /// don't pass duringFetchRawContentRequest. + void testCancel({ + required Narrow narrow, + _EditInteractionStart start = _EditInteractionStart.actionSheet, + bool? duringFetchRawContentRequest, + }) { + final description = StringBuffer()..write('tap Cancel '); + switch (start) { + case _EditInteractionStart.actionSheet: + assert(duringFetchRawContentRequest != null); + description + ..write(duringFetchRawContentRequest! ? 'during ' : 'after ') + ..write('fetch-raw-content request: '); + case _EditInteractionStart.restoreFailedEdit: + assert(duringFetchRawContentRequest == null); + description.write('when editing from a restored failed edit: '); + } + description.write('$narrow'); + testWidgets(description.toString(), (tester) async { + await prepareEditMessage(tester, narrow: narrow); + checkNotInEditingMode(tester, narrow: narrow); + + final messageId = msgIdInNarrow(narrow); + switch (start) { + case _EditInteractionStart.actionSheet: + await startInteractionFromActionSheet(tester, + messageId: messageId, delay: Duration(seconds: 5)); + await checkAwaitingRawMessageContent(tester); + await tester.pump(duringFetchRawContentRequest! + ? Duration(milliseconds: 500) + : Duration(seconds: 5)); + case _EditInteractionStart.restoreFailedEdit: + await startInteractionFromRestoreFailedEdit(tester, + messageId: messageId, + newContent: 'bar'); + checkContentInputValue(tester, 'bar'); + } + + await tester.tap(find.widgetWithText(ZulipWebUiKitButton, 'Cancel')); + await tester.pump(); + checkNotInEditingMode(tester, narrow: narrow); + + // We've canceled the previous edit session, so we should be able to + // do a new edit-message session… + await startInteractionFromActionSheet(tester, + messageId: messageId, originalRawContent: 'foo'); + await checkAwaitingRawMessageContent(tester); + await tester.pump(Duration(seconds: 1)); // fetch-raw-content request + checkContentInputValue(tester, 'foo'); + await enterContent(tester, 'qwerty'); + connection.prepare(json: UpdateMessageResult().toJson()); + await tester.tap(find.widgetWithText(ZulipWebUiKitButton, 'Save')); + checkRequest(messageId, prevContent: 'foo', content: 'qwerty'); + await tester.pump(Duration.zero); + checkNotInEditingMode(tester, narrow: narrow); + + // …or send a new message. + connection.prepare(json: {}); // for typing-start request + connection.prepare(json: {}); // for typing-stop request + await enterContent(tester, 'new message to send'); + state.controller.contentFocusNode.unfocus(); + await tester.pump(); + check(connection.takeRequests()).deepEquals(>[ + (it) => it.isA() + ..method.equals('POST')..url.path.equals('/api/v1/typing'), + (it) => it.isA() + ..method.equals('POST')..url.path.equals('/api/v1/typing')]); + if (narrow is ChannelNarrow) { + await enterTopic(tester, narrow: narrow, topic: topic); + } + await tester.pump(); + await tapSendButton(tester); + check(connection.takeRequests()).single.isA() + ..method.equals('POST') + ..url.path.equals('/api/v1/messages'); + checkContentInputValue(tester, ''); + + if (start == _EditInteractionStart.actionSheet && duringFetchRawContentRequest!) { + // Await the fetch-raw-content request from the canceled edit session; + // its completion shouldn't affect anything. + await tester.pump(Duration(seconds: 5)); + } + checkNotInEditingMode(tester, narrow: narrow); + check(connection.takeRequests()).isEmpty(); + }); + } + // Skip some narrows so the tests run faster; + // the codepaths to be tested are basically the same. + testCancel(narrow: channelNarrow, duringFetchRawContentRequest: false); + // testCancel(narrow: topicNarrow, duringFetchRawContentRequest: false); + testCancel(narrow: dmNarrow, duringFetchRawContentRequest: false); + // testCancel(narrow: channelNarrow, duringFetchRawContentRequest: true); + testCancel(narrow: topicNarrow, duringFetchRawContentRequest: true); + // testCancel(narrow: dmNarrow, duringFetchRawContentRequest: true); + testCancel(narrow: channelNarrow, start: _EditInteractionStart.restoreFailedEdit); + // testCancel(narrow: topicNarrow, start: _EditInteractionStart.restoreFailedEdit); + // testCancel(narrow: dmNarrow, start: _EditInteractionStart.restoreFailedEdit); + }); +} + +/// How the edit interaction is started: +/// from the action sheet, or by restoring a failed edit. +enum _EditInteractionStart { + actionSheet, + restoreFailedEdit; + + String message() { + return switch (this) { + _EditInteractionStart.actionSheet => 'from action sheet', + _EditInteractionStart.restoreFailedEdit => 'from restoring a failed edit', + }; + } } diff --git a/test/widgets/message_list_test.dart b/test/widgets/message_list_test.dart index f4de7b54ae..69a7204827 100644 --- a/test/widgets/message_list_test.dart +++ b/test/widgets/message_list_test.dart @@ -20,6 +20,7 @@ import 'package:zulip/model/store.dart'; import 'package:zulip/model/typing_status.dart'; import 'package:zulip/widgets/autocomplete.dart'; import 'package:zulip/widgets/color.dart'; +import 'package:zulip/widgets/compose_box.dart'; import 'package:zulip/widgets/content.dart'; import 'package:zulip/widgets/icons.dart'; import 'package:zulip/widgets/message_list.dart'; @@ -36,6 +37,7 @@ import '../flutter_checks.dart'; import '../stdlib_checks.dart'; import '../test_images.dart'; import '../test_navigation.dart'; +import 'compose_box_checks.dart'; import 'content_checks.dart'; import 'dialog_checks.dart'; import 'message_list_checks.dart'; @@ -1424,7 +1426,7 @@ void main() { }); }); - group('edit state label', () { + group('EDITED/MOVED label and edit-message error status', () { void checkMarkersCount({required int edited, required int moved}) { check(find.text('EDITED').evaluate()).length.equals(edited); check(find.text('MOVED').evaluate()).length.equals(moved); @@ -1455,6 +1457,81 @@ void main() { await tester.pump(); checkMarkersCount(edited: 2, moved: 0); }); + + void checkEditInProgress(WidgetTester tester) { + check(find.text('SAVING EDIT…')).findsOne(); + check(find.byType(LinearProgressIndicator)).findsOne(); + final opacityWidget = tester.widget(find.ancestor( + of: find.byType(MessageContent), + matching: find.byType(Opacity))); + check(opacityWidget.opacity).equals(0.6); + checkMarkersCount(edited: 0, moved: 0); + } + + void checkEditNotInProgress(WidgetTester tester) { + check(find.text('SAVING EDIT…')).findsNothing(); + check(find.byType(LinearProgressIndicator)).findsNothing(); + check(find.ancestor( + of: find.byType(MessageContent), + matching: find.byType(Opacity))).findsNothing(); + } + + void checkEditFailed(WidgetTester tester) { + check(find.text('EDIT NOT SAVED')).findsOne(); + final opacityWidget = tester.widget(find.ancestor( + of: find.byType(MessageContent), + matching: find.byType(Opacity))); + check(opacityWidget.opacity).equals(0.6); + checkMarkersCount(edited: 0, moved: 0); + } + + testWidgets('successful edit', (tester) async { + final message = eg.streamMessage(); + await setupMessageListPage(tester, + narrow: TopicNarrow.ofMessage(message), + messages: [message]); + + connection.prepare(json: UpdateMessageResult().toJson()); + store.editMessage(messageId: message.id, + originalRawContent: 'foo', + newContent: 'bar'); + await tester.pump(Duration.zero); + checkEditInProgress(tester); + await store.handleEvent(eg.updateMessageEditEvent(message)); + await tester.pump(); + checkEditNotInProgress(tester); + }); + + testWidgets('failed edit', (tester) async { + final message = eg.streamMessage(); + await setupMessageListPage(tester, + narrow: TopicNarrow.ofMessage(message), + messages: [message]); + + connection.prepare(apiException: eg.apiBadRequest(), delay: Duration(seconds: 1)); + store.editMessage(messageId: message.id, + originalRawContent: 'foo', + newContent: 'bar'); + await tester.pump(Duration.zero); + checkEditInProgress(tester); + await tester.pump(Duration(seconds: 1)); + checkEditFailed(tester); + + connection.prepare(json: GetMessageResult( + message: eg.streamMessage(content: 'foo')).toJson(), delay: Duration(milliseconds: 500)); + await tester.tap(find.byType(MessageContent)); + // We don't clear out the failed attempt, with the intended new content… + checkEditFailed(tester); + await tester.pump(Duration(milliseconds: 500)); + // …until we have the current content, from a successful message fetch, + // for prevContentSha256. + checkEditNotInProgress(tester); + + final state = MessageListPage.ancestorOf(tester.element(find.byType(MessageContent))); + check(state.composeBoxState).isNotNull().controller + .isA() + .content.value.text.equals('bar'); + }); }); group('_UnreadMarker animations', () {