Skip to content

Commit 55f3a1a

Browse files
committed
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 element-hq#6519, which reset the mode unconditionally. Reply is still cleared (the attachment becomes the reply). Fixes element-hq#7083.
1 parent 6124d99 commit 55f3a1a

3 files changed

Lines changed: 92 additions & 7 deletions

File tree

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerPresenter.kt

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -288,8 +288,7 @@ class MessageComposerPresenter(
288288
inReplyToEventId = inReplyToEventId,
289289
)
290290

291-
// Reset composer since the attachment has been sent
292-
messageComposerContext.composerMode = MessageComposerMode.Normal
291+
resetComposerModeAfterAttaching()
293292
}
294293
is MessageComposerEvent.SetMode -> {
295294
localCoroutineScope.setMode(event.composerMode, markdownTextEditorState, richTextEditorState)
@@ -638,8 +637,7 @@ class MessageComposerPresenter(
638637
val inReplyToEventId = (messageComposerContext.composerMode as? MessageComposerMode.Reply)?.eventId
639638
navigator.navigateToPreviewAttachments(persistentListOf(mediaAttachment), inReplyToEventId)
640639

641-
// Reset composer since the attachment will be sent in a separate flow
642-
messageComposerContext.composerMode = MessageComposerMode.Normal
640+
resetComposerModeAfterAttaching()
643641
}
644642

645643
private fun handlePickedMediaList(
@@ -663,7 +661,15 @@ class MessageComposerPresenter(
663661
val inReplyToEventId = (messageComposerContext.composerMode as? MessageComposerMode.Reply)?.eventId
664662
navigator.navigateToPreviewAttachments(attachments, inReplyToEventId)
665663

666-
messageComposerContext.composerMode = MessageComposerMode.Normal
664+
resetComposerModeAfterAttaching()
665+
}
666+
667+
private fun resetComposerModeAfterAttaching() {
668+
// An attachment is sent as its own message, so a Reply/Normal mode is consumed and reset.
669+
// An in-progress edit must survive: the typed text still edits the original on the next send.
670+
if (!messageComposerContext.composerMode.isEditing) {
671+
messageComposerContext.composerMode = MessageComposerMode.Normal
672+
}
667673
}
668674

669675
private suspend fun sendMedia(

features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/messagecomposer/MessageComposerPresenterTest.kt

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import io.element.android.features.messages.impl.utils.TextPillificationHelper
3434
import io.element.android.libraries.architecture.AsyncAction
3535
import io.element.android.libraries.core.mimetype.MimeTypes
3636
import io.element.android.libraries.designsystem.utils.snackbar.SnackbarDispatcher
37+
import io.element.android.libraries.featureflag.api.FeatureFlags
3738
import io.element.android.libraries.featureflag.test.FakeFeatureFlagService
3839
import io.element.android.libraries.matrix.api.core.EventId
3940
import io.element.android.libraries.matrix.api.core.RoomId
@@ -758,6 +759,79 @@ class MessageComposerPresenterTest : RobolectricTest() {
758759
}
759760
}
760761

762+
@Test
763+
fun `present - Pick media from gallery while editing keeps the edit pending`() = runTest {
764+
val onPreviewAttachmentLambda = lambdaRecorder { _: ImmutableList<Attachment>, _: EventId? -> }
765+
val navigator = FakeMessagesNavigator(
766+
onPreviewAttachmentLambda = onPreviewAttachmentLambda
767+
)
768+
val presenter = createPresenter(navigator = navigator)
769+
pickerProvider.givenMimeType(MimeTypes.Images)
770+
presenter.test {
771+
var state = awaitFirstItem()
772+
val editMode = anEditMode()
773+
state.eventSink(MessageComposerEvent.SetMode(editMode))
774+
state = awaitItem()
775+
assertThat(state.mode).isEqualTo(editMode)
776+
// The media is sent as a new message while the edit stays active. If the mode reset,
777+
// a Normal-mode state would be emitted and expectNoEvents() below would fail.
778+
state.eventSink(MessageComposerEvent.PickAttachmentSource.FromGallery)
779+
onPreviewAttachmentLambda.assertions().isCalledOnce()
780+
expectNoEvents()
781+
assertThat(state.mode).isEqualTo(editMode)
782+
}
783+
}
784+
785+
@Test
786+
fun `present - Pick media from gallery while replying clears the reply mode`() = runTest {
787+
val onPreviewAttachmentLambda = lambdaRecorder { _: ImmutableList<Attachment>, _: EventId? -> }
788+
val navigator = FakeMessagesNavigator(
789+
onPreviewAttachmentLambda = onPreviewAttachmentLambda
790+
)
791+
val presenter = createPresenter(navigator = navigator)
792+
pickerProvider.givenMimeType(MimeTypes.Images)
793+
presenter.test {
794+
var state = awaitFirstItem()
795+
state.eventSink(MessageComposerEvent.SetMode(aReplyMode()))
796+
state = awaitItem()
797+
assertThat(state.mode).isInstanceOf(MessageComposerMode.Reply::class.java)
798+
// The media becomes the reply, so the reply intent is consumed and the composer resets.
799+
state.eventSink(MessageComposerEvent.PickAttachmentSource.FromGallery)
800+
onPreviewAttachmentLambda.assertions().isCalledOnce()
801+
state = awaitItem()
802+
assertThat(state.mode).isEqualTo(MessageComposerMode.Normal)
803+
}
804+
}
805+
806+
@Test
807+
fun `present - Pick multiple media from gallery while editing keeps the edit pending`() = runTest {
808+
val onPreviewAttachmentLambda = lambdaRecorder { _: ImmutableList<Attachment>, _: EventId? -> }
809+
val navigator = FakeMessagesNavigator(
810+
onPreviewAttachmentLambda = onPreviewAttachmentLambda
811+
)
812+
val presenter = createPresenter(
813+
navigator = navigator,
814+
featureFlagService = FakeFeatureFlagService(
815+
initialState = mapOf(FeatureFlags.SendGalleryMessages.key to true)
816+
),
817+
)
818+
pickerProvider.givenMimeType(MimeTypes.Images)
819+
// Two Uris take the multi-item branch (handlePickedMediaList) instead of delegating to the
820+
// single-item path, so the edit is preserved for gallery messages too.
821+
pickerProvider.givenMultipleResults(listOf(mockk(), mockk()))
822+
presenter.test {
823+
var state = awaitFirstItem()
824+
val editMode = anEditMode()
825+
state.eventSink(MessageComposerEvent.SetMode(editMode))
826+
state = awaitItem()
827+
assertThat(state.mode).isEqualTo(editMode)
828+
state.eventSink(MessageComposerEvent.PickAttachmentSource.FromGallery)
829+
onPreviewAttachmentLambda.assertions().isCalledOnce()
830+
expectNoEvents()
831+
assertThat(state.mode).isEqualTo(editMode)
832+
}
833+
}
834+
761835
@Test
762836
fun `present - Pick media from gallery & cancel does nothing`() = runTest {
763837
val presenter = createPresenter()

libraries/mediapickers/test/src/main/kotlin/io/element/android/libraries/mediapickers/test/FakePickerProvider.kt

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import io.element.android.libraries.mediapickers.api.PickerProvider
1919
class FakePickerProvider : PickerProvider {
2020
private var mimeType = MimeTypes.Any
2121
private var result: Uri? = null
22+
private var multipleResults: List<Uri>? = null
2223

2324
@Composable
2425
override fun registerGalleryPicker(onResult: (uri: Uri?, mimeType: String?) -> Unit): PickerLauncher<PickVisualMediaRequest, Uri?> {
@@ -32,7 +33,7 @@ class FakePickerProvider : PickerProvider {
3233

3334
@Composable
3435
override fun registerGalleryMultiPicker(onResult: (uris: List<Uri>) -> Unit): PickerLauncher<PickVisualMediaRequest, List<Uri>> {
35-
return NoOpPickerLauncher { onResult(result?.let { listOf(it) } ?: emptyList()) }
36+
return NoOpPickerLauncher { onResult(multipleResults ?: result?.let { listOf(it) } ?: emptyList()) }
3637
}
3738

3839
@Composable
@@ -42,7 +43,7 @@ class FakePickerProvider : PickerProvider {
4243

4344
@Composable
4445
override fun registerFileMultiPicker(mimeType: String, onResult: (uris: List<Uri>) -> Unit): PickerLauncher<Array<String>, List<Uri>> {
45-
return NoOpPickerLauncher { onResult(result?.let { listOf(it) } ?: emptyList()) }
46+
return NoOpPickerLauncher { onResult(multipleResults ?: result?.let { listOf(it) } ?: emptyList()) }
4647
}
4748

4849
@Composable
@@ -59,6 +60,10 @@ class FakePickerProvider : PickerProvider {
5960
this.result = value
6061
}
6162

63+
fun givenMultipleResults(uris: List<Uri>) {
64+
this.multipleResults = uris
65+
}
66+
6267
fun givenMimeType(mimeType: String) {
6368
this.mimeType = mimeType
6469
}

0 commit comments

Comments
 (0)