diff --git a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/ChatModel.kt b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/ChatModel.kt index 340ba2c1ad..ef8a392b6f 100644 --- a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/ChatModel.kt +++ b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/ChatModel.kt @@ -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(null) diff --git a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/WelcomeView.kt b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/WelcomeView.kt index 324057f2b7..396ac2b05c 100644 --- a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/WelcomeView.kt +++ b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/WelcomeView.kt @@ -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 } } } diff --git a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ComposeContextProfilePickerView.kt b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ComposeContextProfilePickerView.kt index a8dfa6e1e6..a11c07cda7 100644 --- a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ComposeContextProfilePickerView.kt +++ b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ComposeContextProfilePickerView.kt @@ -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 diff --git a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/helpers/ModalView.kt b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/helpers/ModalView.kt index 3937c85124..6c08cd27de 100644 --- a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/helpers/ModalView.kt +++ b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/helpers/ModalView.kt @@ -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 = - 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 -> diff --git a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/newchat/NewChatView.kt b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/newchat/NewChatView.kt index 1e8bdd3268..ca09fc9531 100644 --- a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/newchat/NewChatView.kt +++ b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/newchat/NewChatView.kt @@ -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("") } }