Skip to content

Fix issue where text is cleared when cancelling a reply #1617

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Oct 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/1617.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix issue where text is cleared when cancelling a reply
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ fun aMessagesState() = MessagesState(
composerState = aMessageComposerState().copy(
richTextEditorState = RichTextEditorState("Hello", initialFocus = true),
isFullScreen = false,
mode = MessageComposerMode.Normal("Hello"),
mode = MessageComposerMode.Normal,
),
voiceMessageComposerState = aVoiceMessageComposerState(),
timelineState = aTimelineState().copy(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,6 @@ import javax.inject.Inject
@SingleIn(RoomScope::class)
@ContributesBinding(RoomScope::class)
class MessageComposerContextImpl @Inject constructor() : MessageComposerContext {
override var composerMode: MessageComposerMode by mutableStateOf(MessageComposerMode.Normal(""))
override var composerMode: MessageComposerMode by mutableStateOf(MessageComposerMode.Normal)
internal set
}
Original file line number Diff line number Diff line change
Expand Up @@ -155,10 +155,12 @@ class MessageComposerPresenter @Inject constructor(
when (event) {
MessageComposerEvents.ToggleFullScreenState -> isFullScreen.value = !isFullScreen.value
MessageComposerEvents.CloseSpecialMode -> {
localCoroutineScope.launch {
richTextEditorState.setHtml("")
if (messageComposerContext.composerMode is MessageComposerMode.Edit) {
localCoroutineScope.launch {
richTextEditorState.setHtml("")
}
}
Comment on lines +158 to 162
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here is the fix

messageComposerContext.composerMode = MessageComposerMode.Normal("")
messageComposerContext.composerMode = MessageComposerMode.Normal
}
is MessageComposerEvents.SendMessage -> appCoroutineScope.sendMessage(
message = event.message,
Expand Down Expand Up @@ -253,7 +255,7 @@ class MessageComposerPresenter @Inject constructor(
val capturedMode = messageComposerContext.composerMode
// Reset composer right away
richTextEditorState.setHtml("")
updateComposerMode(MessageComposerMode.Normal(""))
updateComposerMode(MessageComposerMode.Normal)
when (capturedMode) {
is MessageComposerMode.Normal -> room.sendMessage(body = message.markdown, htmlBody = message.html)
is MessageComposerMode.Edit -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ open class MessageComposerStateProvider : PreviewParameterProvider<MessageCompos
fun aMessageComposerState(
composerState: RichTextEditorState = RichTextEditorState(""),
isFullScreen: Boolean = false,
mode: MessageComposerMode = MessageComposerMode.Normal(content = ""),
mode: MessageComposerMode = MessageComposerMode.Normal,
showTextFormatting: Boolean = false,
showAttachmentSourcePicker: Boolean = false,
canShareLocation: Boolean = true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ class MessageComposerPresenterTest {
val initialState = awaitItem()
assertThat(initialState.isFullScreen).isFalse()
assertThat(initialState.richTextEditorState.messageHtml).isEqualTo("")
assertThat(initialState.mode).isEqualTo(MessageComposerMode.Normal(""))
assertThat(initialState.mode).isEqualTo(MessageComposerMode.Normal)
assertThat(initialState.showAttachmentSourcePicker).isFalse()
assertThat(initialState.canShareLocation).isTrue()
assertThat(initialState.attachmentsState).isEqualTo(AttachmentsState.None)
Expand Down Expand Up @@ -153,7 +153,10 @@ class MessageComposerPresenterTest {
assertThat(state.mode).isEqualTo(mode)
state = awaitItem()
assertThat(state.richTextEditorState.messageHtml).isEqualTo(A_MESSAGE)
backToNormalMode(state, skipCount = 1)
state = backToNormalMode(state, skipCount = 1)

// The message that was being edited is cleared
assertThat(state.richTextEditorState.messageHtml).isEqualTo("")
}
}

Expand All @@ -174,6 +177,26 @@ class MessageComposerPresenterTest {
}
}

@Test
fun `present - cancel reply`() = runTest {
val presenter = createPresenter(this)
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
skipItems(1)
var state = awaitItem()
val mode = aReplyMode()
state.eventSink.invoke(MessageComposerEvents.SetMode(mode))
state = awaitItem()
assertThat(state.mode).isEqualTo(mode)
state.richTextEditorState.setHtml(A_REPLY)
state = backToNormalMode(state)

// The message typed while replying is not cleared
assertThat(state.richTextEditorState.messageHtml).isEqualTo(A_REPLY)
}
}

@Test
fun `present - change mode to quote`() = runTest {
val presenter = createPresenter(this)
Expand Down Expand Up @@ -683,12 +706,12 @@ class MessageComposerPresenterTest {
}
}

private suspend fun ReceiveTurbine<MessageComposerState>.backToNormalMode(state: MessageComposerState, skipCount: Int = 0) {
private suspend fun ReceiveTurbine<MessageComposerState>.backToNormalMode(state: MessageComposerState, skipCount: Int = 0): MessageComposerState {
state.eventSink.invoke(MessageComposerEvents.CloseSpecialMode)
skipItems(skipCount)
val normalState = awaitItem()
assertThat(normalState.mode).isEqualTo(MessageComposerMode.Normal(""))
assertThat(normalState.richTextEditorState.messageHtml).isEqualTo("")
assertThat(normalState.mode).isEqualTo(MessageComposerMode.Normal)
return normalState
}

private fun createPresenter(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,5 @@ import io.element.android.features.messages.api.MessageComposerContext
import io.element.android.libraries.textcomposer.model.MessageComposerMode

class MessageComposerContextFake(
override var composerMode: MessageComposerMode = MessageComposerMode.Normal(null)
override var composerMode: MessageComposerMode = MessageComposerMode.Normal
) : MessageComposerContext
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,7 @@ internal fun TextComposerSimplePreview() = ElementPreview {
RichTextEditorState("", initialFocus = true),
voiceMessageState = VoiceMessageState.Idle,
onSendMessage = {},
composerMode = MessageComposerMode.Normal(""),
composerMode = MessageComposerMode.Normal,
onResetComposerMode = {},
enableTextFormatting = true,
enableVoiceMessages = true,
Expand All @@ -493,7 +493,7 @@ internal fun TextComposerSimplePreview() = ElementPreview {
RichTextEditorState("A message", initialFocus = true),
voiceMessageState = VoiceMessageState.Idle,
onSendMessage = {},
composerMode = MessageComposerMode.Normal(""),
composerMode = MessageComposerMode.Normal,
onResetComposerMode = {},
enableTextFormatting = true,
enableVoiceMessages = true,
Expand All @@ -506,7 +506,7 @@ internal fun TextComposerSimplePreview() = ElementPreview {
),
voiceMessageState = VoiceMessageState.Idle,
onSendMessage = {},
composerMode = MessageComposerMode.Normal(""),
composerMode = MessageComposerMode.Normal,
onResetComposerMode = {},
enableTextFormatting = true,
enableVoiceMessages = true,
Expand All @@ -516,7 +516,7 @@ internal fun TextComposerSimplePreview() = ElementPreview {
RichTextEditorState("A message without focus", initialFocus = false),
voiceMessageState = VoiceMessageState.Idle,
onSendMessage = {},
composerMode = MessageComposerMode.Normal(""),
composerMode = MessageComposerMode.Normal,
onResetComposerMode = {},
enableTextFormatting = true,
enableVoiceMessages = true,
Expand All @@ -533,7 +533,7 @@ internal fun TextComposerFormattingPreview() = ElementPreview {
RichTextEditorState("", initialFocus = false),
voiceMessageState = VoiceMessageState.Idle,
showTextFormatting = true,
composerMode = MessageComposerMode.Normal(""),
composerMode = MessageComposerMode.Normal,
enableTextFormatting = true,
enableVoiceMessages = true,
)
Expand All @@ -542,7 +542,7 @@ internal fun TextComposerFormattingPreview() = ElementPreview {
RichTextEditorState("A message", initialFocus = false),
voiceMessageState = VoiceMessageState.Idle,
showTextFormatting = true,
composerMode = MessageComposerMode.Normal(""),
composerMode = MessageComposerMode.Normal,
enableTextFormatting = true,
enableVoiceMessages = true,
)
Expand All @@ -551,7 +551,7 @@ internal fun TextComposerFormattingPreview() = ElementPreview {
RichTextEditorState("A message\nWith several lines\nTo preview larger textfields and long lines with overflow", initialFocus = false),
voiceMessageState = VoiceMessageState.Idle,
showTextFormatting = true,
composerMode = MessageComposerMode.Normal(""),
composerMode = MessageComposerMode.Normal,
enableTextFormatting = true,
enableVoiceMessages = true,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ internal fun SendButton(
@PreviewsDayNight
@Composable
internal fun SendButtonPreview() = ElementPreview {
val normalMode = MessageComposerMode.Normal("")
val normalMode = MessageComposerMode.Normal
val editMode = MessageComposerMode.Edit(null, "", null)
Row {
SendButton(canSendMessage = true, onClick = {}, composerMode = normalMode)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import kotlinx.parcelize.Parcelize

sealed interface MessageComposerMode : Parcelable {
@Parcelize
data class Normal(val content: CharSequence?) : MessageComposerMode
data object Normal: MessageComposerMode

sealed class Special(open val eventId: EventId?, open val defaultContent: String) :
MessageComposerMode
Expand Down