android, desktop: show the create-profile form over every pane

Follow-up to the previous commit, which fixed the centre-pane placement by moving
the form to the picker's own pane. That was wrong in two different ways, and the
shared modal helpers it changed had a blast radius that was not accounted for.

Placement. ModalManager.end, used for the compose picker, puts the form in a pane
*beside* the chat: the picker stays fully live next to it, with no scrim, so a profile
can be selected while the form is open - and apiChangeConnectionUser recreates the
connection, invalidating the id the form is about to use. It also calls
desktopExpandWindowToWidth, which is grow-only, so the window is permanently widened.
ModalManager.start, used for the one-time link picker, does cover it, but disposes it,
losing whatever was typed in its search box. ModalManager.fullscreen is an opaque
Surface over every pane: nothing can be operated while the form is up, nothing is torn
down, chatId is untouched and the window does not move. The manager is no longer a
parameter - there is only one right answer.

Modal helpers. hasModalOpen/isLastModalOpen were taught to ignore modals waiting out
their close animation, but they have five other callers - ChatListView, ChatView twice,
ChatItemView and SimpleXAPI - all deciding when to tear down secondary chats, and all
silently changed by it. isLastModalOpen also went from an indexed read to a full
iteration on a path called off the main thread. Both are reverted; the new behaviour
lives in isLastModalOpenNotClosing, which only this flow calls and which walks by
index.

Main thread. Making onCreated suspending moved the reassignment onto withBGApi's
single-thread dispatcher, so changeProfileTo mutated chatsContext, chatId and (via
changeActiveUser_) chatModel.users off Main, where the receiver loop writes the same
structures - the very hazard the same commit called out for its own users refresh.
onCreated and close() are now handed back to Dispatchers.Main, which is where both
pickers ran this before it became a callback.

Also: the in-flight flag moves to ChatModel so it is not a top-level val first created
inside a composition; the entry guard consults it, since with one manager per pane the
per-manager check alone let a second form open and silently drop its submit; the
one-time link picker's filteredProfiles is keyed on the profile count, so a profile
created from it appears without relying on the composition being rebuilt; and the
progress indicator skips its 500ms grace while creating, since that timer restarts
from zero in the rebuilt composition and would otherwise leave the rows looking idle.
This commit is contained in:
Narasimha-sc
2026-08-06 14:17:17 +00:00
parent 75941e4f79
commit 60cc62ee3c
5 changed files with 74 additions and 60 deletions
@@ -122,6 +122,13 @@ object ChatModel {
val incompleteInitializedDbRemoved = mutableStateOf(false)
// map of connections network statuses, key is agent connection id
val switchingUsersAndHosts = mutableStateOf(false)
/** True from the moment a profile is submitted in the "add profile for this invitation"
* form until the invitation has been moved onto it. Lives here rather than in the picker
* that started it: on Android every ModalManager placement is one stack rendering only
* its top entry, so the one-time link picker - itself a modal - is disposed while the
* form is above it and returns with every remembered flag reset. (The compose picker is
* not a modal and does survive, on both platforms.) */
val creatingProfileForInvitation = mutableStateOf(false)
// current chat
val chatId = mutableStateOf<String?>(null)
@@ -349,30 +349,28 @@ private fun CreateFirstProfileDesktop(chatModel: ChatModel, close: () -> Unit) {
}
}
/** True while a profile is being created for an invitation and handed over to the picker
* that asked for it. Deliberately not remembered in either picker: on Android the picker's
* composition is disposed while the create-profile modal is on top of it, and comes back
* with every remembered flag reset - a per-picker flag would be released the moment the
* form opens, leaving the rows live during the reassignment. */
val creatingProfileForInvitation = mutableStateOf(false)
// Creates a profile for an invitation and hands it to onCreated, which moves the
// invitation onto it. The profile is created *without* becoming active: the reassignment
// APIs resolve the prepared chat or connection under the active user, so the profile that
// owns the invitation has to stay active until onCreated has run - which is also why
// onCreated is suspending, so the in-flight flag covers the reassignment and not just the
// creation.
fun createProfileForInvitation(rhId: Long?, modalManager: ModalManager, onCreated: suspend (User) -> Unit) {
// Shown in the picker's own pane: ModalManager.center nulls chatId on desktop, which
// closes the chat the prepared invitation is in - and for the picker that is itself a
// start-pane modal, leaves the picker live beside the form.
fun createProfileForInvitation(rhId: Long?, onCreated: suspend (User) -> Unit) {
// ModalManager.fullscreen on purpose. center nulls chatId on desktop, closing the very
// chat a prepared invitation is in; end widens the window for good and leaves the
// compose picker live in the pane beside the form; start would work for the one-time
// link picker but disposes it, losing what was typed in its search box. fullscreen is
// an opaque Surface over every pane, so no picker can be operated while the form is up
// and none of them is torn down. On Android all four are the same manager anyway.
// Two taps before the modal renders would otherwise stack two modals sharing one id,
// after which close() could dismiss the wrong one.
if (chatModel.creatingProfileForInvitation.value) return
val modalManager = ModalManager.fullscreen
if (modalManager.hasModalOpen(ModalViewId.CONTEXT_USER_PICKER_NEW_PROFILE)) return
modalManager.showModalCloseable(id = ModalViewId.CONTEXT_USER_PICKER_NEW_PROFILE) { close ->
CreateProfile { displayName, shortDescr, image ->
if (creatingProfileForInvitation.value) return@CreateProfile
creatingProfileForInvitation.value = true
if (chatModel.creatingProfileForInvitation.value) return@CreateProfile
chatModel.creatingProfileForInvitation.value = true
withBGApi {
try {
// The reassignment in onCreated resolves the invitation under whatever is active
@@ -401,22 +399,27 @@ fun createProfileForInvitation(rhId: Long?, modalManager: ModalManager, onCreate
}
}
}
// A notification tap or a remote host switch can change the active user or tear
// the form down while the profile is being created. Reassigning after either
// would resolve the invitation under the wrong profile, or move it under a
// screen the user has already left, so stop with the profile created.
if (
chatModel.currentUser.value?.userId != ownerUserId ||
chatModel.remoteHostId() != rhId ||
!modalManager.isLastModalOpen(ModalViewId.CONTEXT_USER_PICKER_NEW_PROFILE)
) {
AlertManager.shared.showAlertMsg(generalGetString(MR.strings.error_changing_user))
return@withBGApi
// Hand over on the main thread: onCreated reassigns the chat or connection and
// switches the user, all of which update structures the receiver loop also
// writes to on Main - and Main is where both pickers ran this before it became
// a suspending callback. close() touches the modal stack, so it goes here too.
withContext(Dispatchers.Main) {
// A notification tap or a remote host switch can change the active user or the
// host while the profile is being created; reassigning then would resolve the
// invitation under the wrong profile. Stop, with the profile created.
if (chatModel.currentUser.value?.userId != ownerUserId || chatModel.remoteHostId() != rhId) {
AlertManager.shared.showAlertMsg(generalGetString(MR.strings.error_changing_user))
return@withContext
}
// The form is gone - most likely the user backed out of it while the profile
// was being created. Don't move the invitation under a screen they have left,
// and don't report an error for something they did deliberately.
if (!modalManager.isLastModalOpenNotClosing(ModalViewId.CONTEXT_USER_PICKER_NEW_PROFILE)) return@withContext
close()
onCreated(newUser)
}
close()
onCreated(newUser)
} finally {
creatingProfileForInvitation.value = false
chatModel.creatingProfileForInvitation.value = false
}
}
}
@@ -19,7 +19,6 @@ import chat.simplex.common.model.*
import chat.simplex.common.platform.*
import chat.simplex.common.ui.theme.*
import chat.simplex.common.views.createProfileForInvitation
import chat.simplex.common.views.creatingProfileForInvitation
import chat.simplex.common.views.helpers.*
import chat.simplex.common.views.newchat.IncognitoOptionImage
import chat.simplex.common.views.usersettings.IncognitoView
@@ -46,7 +45,7 @@ fun ComposeContextProfilePickerView(
val changingProfile = remember { mutableStateOf(false) }
// Creating a profile keeps the rows disabled too - the reassignment runs after the form
// closes, and until it has, picking anything else moves the invitation twice.
val busy = changingProfile.value || creatingProfileForInvitation.value
val busy = changingProfile.value || chatModel.creatingProfileForInvitation.value
val maxHeightInPx = with(LocalDensity.current) { windowHeight().toPx() }
val isVisible = remember { mutableStateOf(false) }
@@ -258,15 +257,7 @@ fun ComposeContextProfilePickerView(
Modifier
.fillMaxWidth()
.sizeIn(minHeight = DEFAULT_MIN_SECTION_ITEM_HEIGHT + 8.dp)
// ModalManager.end, like the incognito info modal above: the center manager nulls
// chatId on desktop, which closes the very chat this picker belongs to.
.clickable(enabled = !busy, onClick = {
if (!chat.chatInfo.profileChangeProhibited) {
createProfileForInvitation(rhId, ModalManager.end) { changeProfileTo(it) }
} else {
showCantChangeProfileAlert()
}
})
.clickable(enabled = !busy, onClick = { createProfileForInvitation(rhId) { changeProfileTo(it) } })
.padding(horizontal = DEFAULT_PADDING_HALF, vertical = 4.dp),
horizontalArrangement = Arrangement.SpaceBetween,
verticalAlignment = Alignment.CenterVertically
@@ -113,16 +113,25 @@ class ModalManager(private val placement: ModalPlacement? = null) {
private var passcodeView: MutableStateFlow<(@Composable (close: () -> Unit) -> Unit)?> = MutableStateFlow(null)
private var onTimePasscodeView: MutableStateFlow<(@Composable (close: () -> Unit) -> Unit)?> = MutableStateFlow(null)
// A modal closed while its animation is still running stays in modalViews until the
// animation ends, so both checks have to skip the ones already staged for removal -
// otherwise a caller that closes on "is my modal still the last one?" pops the modal
// underneath instead.
private fun openModalViews(): List<ModalViewHolder> =
if (toRemove.isEmpty()) modalViews else modalViews.filterIndexed { i, _ -> i !in toRemove }
fun hasModalOpen(id: ModalViewId): Boolean = modalViews.any { it.id == id }
fun hasModalOpen(id: ModalViewId): Boolean = openModalViews().any { it.id == id }
fun isLastModalOpen(id: ModalViewId): Boolean = modalViews.lastOrNull()?.id == id
fun isLastModalOpen(id: ModalViewId): Boolean = openModalViews().lastOrNull()?.id == id
/** Like [isLastModalOpen], except that a modal already dismissed and only waiting out
* its close animation does not count as open: [closeModal] leaves it in [modalViews]
* and stages its index in [toRemove]. A caller that decides whether to close on "is my
* modal still the last one?" needs this, or a back-tap during a long operation makes it
* pop the screen underneath as well.
*
* Separate from [isLastModalOpen] rather than folded into it, because that one is also
* used to decide when to tear down secondary chats, where the existing behaviour is
* relied on. Indexed access rather than iteration, so it stays safe if a caller ever
* reaches it off the main thread while these lists are being mutated. */
fun isLastModalOpenNotClosing(id: ModalViewId): Boolean {
var i = modalViews.size - 1
while (i >= 0 && i in toRemove) i--
return i >= 0 && modalViews.getOrNull(i)?.id == id
}
fun showModal(settings: Boolean = false, showClose: Boolean = true, id: ModalViewId? = null, forceAnimated: Boolean = false, cardScreen: Boolean = false, endButtons: @Composable RowScope.() -> Unit = {}, content: @Composable ModalData.() -> Unit) {
showCustomModal(id = id, forceAnimated = forceAnimated) { close ->
@@ -40,7 +40,6 @@ import chat.simplex.common.ui.theme.*
import chat.simplex.common.views.chat.item.CIFileViewScope
import chat.simplex.common.views.chat.topPaddingToContent
import chat.simplex.common.views.createProfileForInvitation
import chat.simplex.common.views.creatingProfileForInvitation
import chat.simplex.common.views.helpers.*
import chat.simplex.common.views.usersettings.*
import chat.simplex.common.BuildConfigCommon
@@ -300,8 +299,10 @@ fun ActiveProfilePicker(
}
val selectedProfile by remember { chatModel.currentUser }
val searchTextOrPassword = rememberSaveable { search }
// Intentionally don't use derivedStateOf in order to NOT change an order after user was selected
val filteredProfiles = remember(searchTextOrPassword.value) {
// Intentionally don't use derivedStateOf in order to NOT change an order after user was selected.
// Keyed on the profile count as well so a profile created from this picker appears in it
// without relying on the composition being torn down and rebuilt by the form on top.
val filteredProfiles = remember(searchTextOrPassword.value, chatModel.users.size) {
filteredProfiles(chatModel.users.map { it.user }.sortedBy { !it.activeUser }, searchTextOrPassword.value)
}
@@ -309,7 +310,7 @@ fun ActiveProfilePicker(
// Creating a profile for this invitation keeps the picker busy until the connection has
// been moved onto it, not just until the profile exists.
val busy = switchingProfile.value || creatingProfileForInvitation.value
val busy = switchingProfile.value || chatModel.creatingProfileForInvitation.value
LaunchedEffect(busy) {
progressByTimeout = if (busy) {
@@ -319,6 +320,10 @@ fun ActiveProfilePicker(
false
}
}
// Creating skips the 500ms grace: the picker is recomposed from scratch when the form
// closes, so progressByTimeout starts again from false and would leave the rows looking
// idle while the invitation is being moved.
val showProgress = progressByTimeout || chatModel.creatingProfileForInvitation.value
suspend fun selectProfileAsync(user: User) {
switchingProfile.value = true
@@ -327,10 +332,12 @@ fun ActiveProfilePicker(
if (contactConnection != null) {
updatedConn = controller.apiChangeConnectionUser(rhId, contactConnection.pccConnId, user.userId)
// The connection was not moved - apiChangeConnectionUser reports it. Leave the
// picker open instead of switching or dismissing: a profile just created for this
// invitation would otherwise be stranded with nothing pointing at it, and the
// connection needs the network here, so this is what happens when offline.
// The connection was not moved. Leave the picker open instead of switching or
// dismissing: a profile just created for this invitation would otherwise be
// stranded with nothing pointing at it, and this call provisions a new queue, so
// it is what fails offline. apiChangeConnectionUser reports the failure itself
// except when sendCmdWithRetry gives up, which is either the user cancelling the
// retry or the command being cancelled.
if (updatedConn == null) return
withContext(Dispatchers.Main) {
chatModel.chatsContext.updateContactConnection(rhId, updatedConn)
@@ -377,10 +384,7 @@ fun ActiveProfilePicker(
title = stringResource(MR.strings.users_add),
disabled = busy,
selected = false,
// ModalManager.start, the pane this picker is itself shown in, like the incognito
// info modal below: the center manager renders the form in another pane on desktop,
// leaving this picker live beside it, and nulls chatId, closing any open chat.
onSelected = { createProfileForInvitation(rhId, ModalManager.start) { selectProfileAsync(it) } },
onSelected = { createProfileForInvitation(rhId) { selectProfileAsync(it) } },
image = {
Box(Modifier.size(42.dp), contentAlignment = Alignment.Center) {
Icon(
@@ -445,7 +449,7 @@ fun ActiveProfilePicker(
Column(
Modifier
.fillMaxSize()
.alpha(if (progressByTimeout) 0.6f else 1f)
.alpha(if (showProgress) 0.6f else 1f)
) {
LazyColumnWithScrollBar(Modifier.padding(top = topPaddingToContent(false)), userScrollEnabled = !busy) {
item {
@@ -504,7 +508,7 @@ fun ActiveProfilePicker(
}
}
}
if (progressByTimeout) {
if (showProgress) {
DefaultProgressView("")
}
}