From 55f3a1acfbfcffee27f7db303ea8e3a3720ad4e4 Mon Sep 17 00:00:00 2001 From: manfrommedan Date: Wed, 8 Jul 2026 15:32:01 +0300 Subject: [PATCH] Keep the in-progress edit when attaching media Picking an attachment reset the composer to Normal, which dropped an in-progress edit: the attachment went out as a new message and the leftover edit text then created another one instead of editing the original. Route every attach path through a single resetComposerModeAfterAttaching() helper that clears Reply/Normal but keeps an edit pending, so the typed text still edits the original on the next send. This also covers the multi-item gallery picker added in #6519, which reset the mode unconditionally. Reply is still cleared (the attachment becomes the reply). Fixes #7083. --- .../MessageComposerPresenter.kt | 16 ++-- .../MessageComposerPresenterTest.kt | 74 +++++++++++++++++++ .../mediapickers/test/FakePickerProvider.kt | 9 ++- 3 files changed, 92 insertions(+), 7 deletions(-) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerPresenter.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerPresenter.kt index 858e872365e..eaa00f427e3 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerPresenter.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerPresenter.kt @@ -288,8 +288,7 @@ class MessageComposerPresenter( inReplyToEventId = inReplyToEventId, ) - // Reset composer since the attachment has been sent - messageComposerContext.composerMode = MessageComposerMode.Normal + resetComposerModeAfterAttaching() } is MessageComposerEvent.SetMode -> { localCoroutineScope.setMode(event.composerMode, markdownTextEditorState, richTextEditorState) @@ -638,8 +637,7 @@ class MessageComposerPresenter( val inReplyToEventId = (messageComposerContext.composerMode as? MessageComposerMode.Reply)?.eventId navigator.navigateToPreviewAttachments(persistentListOf(mediaAttachment), inReplyToEventId) - // Reset composer since the attachment will be sent in a separate flow - messageComposerContext.composerMode = MessageComposerMode.Normal + resetComposerModeAfterAttaching() } private fun handlePickedMediaList( @@ -663,7 +661,15 @@ class MessageComposerPresenter( val inReplyToEventId = (messageComposerContext.composerMode as? MessageComposerMode.Reply)?.eventId navigator.navigateToPreviewAttachments(attachments, inReplyToEventId) - messageComposerContext.composerMode = MessageComposerMode.Normal + resetComposerModeAfterAttaching() + } + + private fun resetComposerModeAfterAttaching() { + // An attachment is sent as its own message, so a Reply/Normal mode is consumed and reset. + // An in-progress edit must survive: the typed text still edits the original on the next send. + if (!messageComposerContext.composerMode.isEditing) { + messageComposerContext.composerMode = MessageComposerMode.Normal + } } private suspend fun sendMedia( diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerPresenterTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerPresenterTest.kt index 7c6f47058e8..499e4bf91a3 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerPresenterTest.kt +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerPresenterTest.kt @@ -34,6 +34,7 @@ import io.element.android.features.messages.impl.utils.TextPillificationHelper import io.element.android.libraries.architecture.AsyncAction import io.element.android.libraries.core.mimetype.MimeTypes import io.element.android.libraries.designsystem.utils.snackbar.SnackbarDispatcher +import io.element.android.libraries.featureflag.api.FeatureFlags import io.element.android.libraries.featureflag.test.FakeFeatureFlagService import io.element.android.libraries.matrix.api.core.EventId import io.element.android.libraries.matrix.api.core.RoomId @@ -758,6 +759,79 @@ class MessageComposerPresenterTest : RobolectricTest() { } } + @Test + fun `present - Pick media from gallery while editing keeps the edit pending`() = runTest { + val onPreviewAttachmentLambda = lambdaRecorder { _: ImmutableList, _: EventId? -> } + val navigator = FakeMessagesNavigator( + onPreviewAttachmentLambda = onPreviewAttachmentLambda + ) + val presenter = createPresenter(navigator = navigator) + pickerProvider.givenMimeType(MimeTypes.Images) + presenter.test { + var state = awaitFirstItem() + val editMode = anEditMode() + state.eventSink(MessageComposerEvent.SetMode(editMode)) + state = awaitItem() + assertThat(state.mode).isEqualTo(editMode) + // The media is sent as a new message while the edit stays active. If the mode reset, + // a Normal-mode state would be emitted and expectNoEvents() below would fail. + state.eventSink(MessageComposerEvent.PickAttachmentSource.FromGallery) + onPreviewAttachmentLambda.assertions().isCalledOnce() + expectNoEvents() + assertThat(state.mode).isEqualTo(editMode) + } + } + + @Test + fun `present - Pick media from gallery while replying clears the reply mode`() = runTest { + val onPreviewAttachmentLambda = lambdaRecorder { _: ImmutableList, _: EventId? -> } + val navigator = FakeMessagesNavigator( + onPreviewAttachmentLambda = onPreviewAttachmentLambda + ) + val presenter = createPresenter(navigator = navigator) + pickerProvider.givenMimeType(MimeTypes.Images) + presenter.test { + var state = awaitFirstItem() + state.eventSink(MessageComposerEvent.SetMode(aReplyMode())) + state = awaitItem() + assertThat(state.mode).isInstanceOf(MessageComposerMode.Reply::class.java) + // The media becomes the reply, so the reply intent is consumed and the composer resets. + state.eventSink(MessageComposerEvent.PickAttachmentSource.FromGallery) + onPreviewAttachmentLambda.assertions().isCalledOnce() + state = awaitItem() + assertThat(state.mode).isEqualTo(MessageComposerMode.Normal) + } + } + + @Test + fun `present - Pick multiple media from gallery while editing keeps the edit pending`() = runTest { + val onPreviewAttachmentLambda = lambdaRecorder { _: ImmutableList, _: EventId? -> } + val navigator = FakeMessagesNavigator( + onPreviewAttachmentLambda = onPreviewAttachmentLambda + ) + val presenter = createPresenter( + navigator = navigator, + featureFlagService = FakeFeatureFlagService( + initialState = mapOf(FeatureFlags.SendGalleryMessages.key to true) + ), + ) + pickerProvider.givenMimeType(MimeTypes.Images) + // Two Uris take the multi-item branch (handlePickedMediaList) instead of delegating to the + // single-item path, so the edit is preserved for gallery messages too. + pickerProvider.givenMultipleResults(listOf(mockk(), mockk())) + presenter.test { + var state = awaitFirstItem() + val editMode = anEditMode() + state.eventSink(MessageComposerEvent.SetMode(editMode)) + state = awaitItem() + assertThat(state.mode).isEqualTo(editMode) + state.eventSink(MessageComposerEvent.PickAttachmentSource.FromGallery) + onPreviewAttachmentLambda.assertions().isCalledOnce() + expectNoEvents() + assertThat(state.mode).isEqualTo(editMode) + } + } + @Test fun `present - Pick media from gallery & cancel does nothing`() = runTest { val presenter = createPresenter() diff --git a/libraries/mediapickers/test/src/main/kotlin/io/element/android/libraries/mediapickers/test/FakePickerProvider.kt b/libraries/mediapickers/test/src/main/kotlin/io/element/android/libraries/mediapickers/test/FakePickerProvider.kt index 5d3be8acd17..022ad05da2c 100644 --- a/libraries/mediapickers/test/src/main/kotlin/io/element/android/libraries/mediapickers/test/FakePickerProvider.kt +++ b/libraries/mediapickers/test/src/main/kotlin/io/element/android/libraries/mediapickers/test/FakePickerProvider.kt @@ -19,6 +19,7 @@ import io.element.android.libraries.mediapickers.api.PickerProvider class FakePickerProvider : PickerProvider { private var mimeType = MimeTypes.Any private var result: Uri? = null + private var multipleResults: List? = null @Composable override fun registerGalleryPicker(onResult: (uri: Uri?, mimeType: String?) -> Unit): PickerLauncher { @@ -32,7 +33,7 @@ class FakePickerProvider : PickerProvider { @Composable override fun registerGalleryMultiPicker(onResult: (uris: List) -> Unit): PickerLauncher> { - return NoOpPickerLauncher { onResult(result?.let { listOf(it) } ?: emptyList()) } + return NoOpPickerLauncher { onResult(multipleResults ?: result?.let { listOf(it) } ?: emptyList()) } } @Composable @@ -42,7 +43,7 @@ class FakePickerProvider : PickerProvider { @Composable override fun registerFileMultiPicker(mimeType: String, onResult: (uris: List) -> Unit): PickerLauncher, List> { - return NoOpPickerLauncher { onResult(result?.let { listOf(it) } ?: emptyList()) } + return NoOpPickerLauncher { onResult(multipleResults ?: result?.let { listOf(it) } ?: emptyList()) } } @Composable @@ -59,6 +60,10 @@ class FakePickerProvider : PickerProvider { this.result = value } + fun givenMultipleResults(uris: List) { + this.multipleResults = uris + } + fun givenMimeType(mimeType: String) { this.mimeType = mimeType }