mirror of
https://github.com/simplex-chat/simplex-chat.git
synced 2026-07-24 12:51:32 +00:00
android, desktop: fix draft loss when switching to chat where user cannot send messages (#7239)
* android, desktop: save draft when switching to chat where user cannot send messages On desktop, chat and chatId change in the same recomposition when another chat is opened from the always-visible chat list. The effect clearing compose state of a non-sendable chat (observer, channel subscriber, review by admins) was composed before the draft-saving KeyChangeEffect and ran first, wiping the live compose state before it could be saved and then clearing the previously saved draft via clearPrevDraft. Effects launch in composition order, so the clearing effect is moved after KeyChangeEffect: the previous chat's draft is saved first, and clearCurrentDraft is a no-op for it because draftChatId no longer matches the opened chat. Clearing the opened chat's own draft when it cannot send is preserved, as is clearing when the open chat itself becomes non-sendable (only the sendMsgEnabled key changes). * plans: investigation and justification for draft message loss fix
This commit is contained in:
+8
-7
@@ -1308,13 +1308,6 @@ fun ComposeView(
|
||||
}
|
||||
}
|
||||
|
||||
LaunchedEffect(rememberUpdatedState(chat.chatInfo.sendMsgEnabled).value) {
|
||||
if (!chat.chatInfo.sendMsgEnabled) {
|
||||
clearCurrentDraft()
|
||||
clearState()
|
||||
}
|
||||
}
|
||||
|
||||
KeyChangeEffect(chatModel.chatId.value) { prevChatId ->
|
||||
val cs = composeState.value
|
||||
if (cs.liveMessage != null && (cs.message.text.isNotEmpty() || cs.liveMessage.sent)) {
|
||||
@@ -1345,6 +1338,14 @@ fun ComposeView(
|
||||
chatModel.removeLiveDummy()
|
||||
CIFile.cachedRemoteFileRequests.clear()
|
||||
}
|
||||
// Must be composed after KeyChangeEffect above (effects run in composition order),
|
||||
// so that on chat switch the previous chat's draft is saved before it is cleared here.
|
||||
LaunchedEffect(rememberUpdatedState(chat.chatInfo.sendMsgEnabled).value) {
|
||||
if (!chat.chatInfo.sendMsgEnabled) {
|
||||
clearCurrentDraft()
|
||||
clearState()
|
||||
}
|
||||
}
|
||||
// keep the attach size limit in sync with the chat: the user's active badge raises it, but not in incognito chats where no badge is presented
|
||||
LaunchedEffect(chat.chatInfo) {
|
||||
val incognito = if (chat.chatInfo.profileChangeProhibited) chat.chatInfo.incognito else chatModel.controller.appPrefs.incognito.get()
|
||||
|
||||
@@ -0,0 +1,67 @@
|
||||
# Fix draft loss when switching to a chat where user cannot send messages
|
||||
|
||||
**Branch:** `nd/fix-draft-message-loss-on-restricted` · **PR:** [#7239](https://github.com/simplex-chat/simplex-chat/pull/7239) (draft) · **Commit:** `9d4bb2188`
|
||||
|
||||
## Problem
|
||||
|
||||
A draft typed in the current chat is lost when another chat is opened where the user
|
||||
cannot send messages (observer, channel subscriber, review by admins). Reproduces on
|
||||
desktop only: the chat list is always visible, so `chat` and `chatId` change in the
|
||||
same recomposition (`CenterPartOfScreen` passes `ChatModel.chatId` itself as
|
||||
`currentChatId`, App.kt:389). Android is unaffected — `AndroidScreen` updates
|
||||
`currentChatId` via a `snapshotFlow` collector, so the draft-saving effect fires a
|
||||
frame before the chat swaps.
|
||||
|
||||
## Root cause
|
||||
|
||||
In `ComposeView.kt`, the effect clearing compose state of a non-sendable chat
|
||||
(`LaunchedEffect(sendMsgEnabled)`) was composed *before* the draft-saving
|
||||
`KeyChangeEffect(chatModel.chatId)`. `LaunchedEffect` bodies run in composition
|
||||
order, so on a same-recomposition switch the clear effect wiped the live compose
|
||||
state before it could be saved, and `KeyChangeEffect` then fell into its
|
||||
else-branch, destroying any previously saved draft via `clearPrevDraft` and deleting
|
||||
attachment files. Present since the effect was introduced (`7b362ff65`, #5922) —
|
||||
not a recent regression.
|
||||
|
||||
## Fix
|
||||
|
||||
Move the clearing effect after `KeyChangeEffect` (6 lines moved + 2-line comment).
|
||||
The draft is saved first; `clearCurrentDraft()` is then a no-op for it because
|
||||
`draftChatId` no longer matches the opened chat. Preserved behavior: the opened
|
||||
chat's own draft is still cleared when it cannot send, and the draft is still
|
||||
cleared when the open chat itself becomes non-sendable (only the `sendMsgEnabled`
|
||||
key changes). Incidental improvements: an active live message is now sent on switch
|
||||
instead of dropped, and the `inProgress` branch no longer resurrects cleared text.
|
||||
|
||||
## Verification
|
||||
|
||||
- Headless Compose-runtime harness (scratchpad `TmpDraftLossEffectOrderTest.kt` /
|
||||
`TmpDraftFixVerifyTest.kt`): reproduced the bug (clear-before-save order observed),
|
||||
verified the fix across 5 scenarios (switch to non-sendable, in-chat demotion,
|
||||
non-sendable chat's own saved draft cleared, Android staggered path, sendable
|
||||
control). All pass; `:common:desktopTest` and `:common:compileKotlinDesktop` green.
|
||||
- Manual desktop repro confirmed the bug and fix.
|
||||
- Works with `privacySaveLastDraft` off: text discarded per the setting's contract,
|
||||
never saved; disabling purges the slot, so no stale-state interactions.
|
||||
|
||||
## iOS
|
||||
|
||||
Not reproduced on device (desktop repro path is safe on iOS: back-navigation fires
|
||||
`ComposeView.onDisappear`, which saves the draft). A code-traced fix for in-place
|
||||
chat switches (member info → "message", notification tap, "forwarded from" — where
|
||||
`onDisappear` never runs) was removed from the PR and parked on
|
||||
`nd/ios-draft-save-on-chat-switch` (commit `7b9b8d987`) pending device verification.
|
||||
Decisive test: type a draft in a group → member info → "message" → back to group.
|
||||
If the draft survives without the change, delete the side branch.
|
||||
|
||||
## Pre-existing issues found (not addressed, both platforms)
|
||||
|
||||
- Secondary (member support/reports) chat views share the group's chat id and race
|
||||
the primary on the single draft slot (Kotlin: unguarded `KeyChangeEffect` in
|
||||
secondary instances can clear the primary's just-saved draft when a modal is open
|
||||
during a switch; iOS: secondary `onDisappear` saves under the swapped chat id).
|
||||
- Kotlin: drafts are not saved when a chat is closed to `chatId = null` on desktop
|
||||
(ChatView is disposed before `KeyChangeEffect` runs; only the forwarding case is
|
||||
saved via `DisposableEffect`).
|
||||
- Switching between two non-sendable chats restores the target's draft into the
|
||||
disabled composer (clear effect does not re-fire on `false→false`).
|
||||
Reference in New Issue
Block a user