From 66a313f3285d38dd9406d2dfa29a8f1e1cf93cb0 Mon Sep 17 00:00:00 2001 From: Narasimha-sc <166327228+Narasimha-sc@users.noreply.github.com> Date: Fri, 7 Aug 2026 15:57:52 +0000 Subject: [PATCH] stop the create form closing mid-creation, and register the profile earlier ModalView's Back handler and back arrow were live while the profile was being created, so Back/Esc closed the form between creation and the reassignment: profile created, invitation never moved, nothing said, and the next attempt at the same name fails as a duplicate. Gate them on the in-flight flag, as MigrateFromDevice and ChooseServerOperators do, which is the Kotlin equivalent of iOS's interactiveDismissDisabled. Move the chatModel.users registration above the stale-host branch, matching what iOS got in 54e2fac3d: changeActiveUser_ throws before its own refresh, so the block below the early return never ran. Also fix the incognito twin of a bug this branch already fixes on iOS: with no connection to change, profileSwitchStatus stayed .switchingIncognito and left the whole picker dead. Pre-existing, but immediate now that hit testing keys on the status rather than the delayed spinner flag. --- .../Shared/Views/NewChat/NewChatView.swift | 8 ++ .../chat/simplex/common/views/WelcomeView.kt | 126 +++++++++--------- .../2026-07-30-new-profile-for-invitation.md | 23 ++++ 3 files changed, 97 insertions(+), 60 deletions(-) diff --git a/apps/ios/Shared/Views/NewChat/NewChatView.swift b/apps/ios/Shared/Views/NewChat/NewChatView.swift index 21350452d2..2bc831b65d 100644 --- a/apps/ios/Shared/Views/NewChat/NewChatView.swift +++ b/apps/ios/Shared/Views/NewChat/NewChatView.swift @@ -427,6 +427,14 @@ private struct ActiveProfilePicker: View { profileSwitchStatus = .idle dismiss() } + } else { + // Nothing to change, so nothing happened. Without this the status + // stays .switchingIncognito and busy leaves the whole picker - + // including the new "Add profile" row - permanently dead. + await MainActor.run { + profileSwitchStatus = .idle + incognitoEnabled = !incognito + } } } catch { profileSwitchStatus = .idle 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 39919d354c..096523474f 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 @@ -370,69 +370,75 @@ fun createProfileForInvitation(rhId: Long?, onCreated: suspend (User) -> Unit) { // compose picker live in the pane beside the form; start disposes the picker that opened it. val modalManager = ModalManager.fullscreen if (modalManager.isLastModalOpenNotClosing(ModalViewId.CONTEXT_USER_PICKER_NEW_PROFILE)) return - modalManager.showModalCloseable(id = ModalViewId.CONTEXT_USER_PICKER_NEW_PROFILE) { close -> - CreateProfile(chatModel, close, submitting = chatModel.creatingProfileForInvitation.value) { displayName, shortDescr, image -> - if (chatModel.creatingProfileForInvitation.value) return@CreateProfile - chatModel.creatingProfileForInvitation.value = true - // On Main, like the pickers' own handlers: every call here suspends into IO, and the - // chat model is updated on Main by the receiver loop. - withApi { - try { - val ownerUserId = chatModel.currentUser.value?.userId - val profile = Profile(displayName.trim(), "", shortDescr.trim().ifEmpty { null }, image = image) - val newUser = controller.apiCreateActiveUser(rhId, profile, keepActiveUser = true) ?: return@withApi - if (newUser.activeUser) { - // An older remote host ignored the flag and activated it, so the reassignment - // would fail. Resync to what the host did and report it, with the form - // dismissed - the app is about to be showing a different profile. - if (modalManager.isLastModalOpenNotClosing(ModalViewId.CONTEXT_USER_PICKER_NEW_PROFILE)) close() - // changeActiveUser_, not changeActiveUser: the latter shows its own alert on - // failure, which would stack with the one below reporting the same thing. - runCatching { controller.changeActiveUser_(newUser.remoteHostId, newUser.userId, null) } - .onFailure { Log.e(TAG, "createProfileForInvitation: resync failed: ${it.stackTraceToString()}") } - AlertManager.shared.showAlertMsg(generalGetString(MR.strings.error_changing_user)) - return@withApi - } - // List the new profile even if the reassignment below fails. listUsers throws and - // withApi does not catch, so the refresh is guarded. - if (chatModel.remoteHostId() == rhId) { - val updatedUsers = runCatching { controller.listUsers(rhId) }.getOrNull() - if (updatedUsers != null) { - chatModel.users.clear() - chatModel.users.addAll(updatedUsers) - } else if (chatModel.users.none { it.user.userId == newUser.userId }) { - // listUsers failed, and the picker is keyed on chatModel.users.size - without - // this it shows no row for the profile just created, and creating it again - // fails on the duplicate name. + modalManager.showCustomModal(id = ModalViewId.CONTEXT_USER_PICKER_NEW_PROFILE) { close -> + // Back, Esc and the back arrow must not dismiss the form while the profile is being + // created: it would exist with the invitation never moved onto it and nothing said, + // and the next attempt at the same name fails as a duplicate. iOS closes the same + // window with interactiveDismissDisabled. + ModalView(close, enableClose = !chatModel.creatingProfileForInvitation.value) { + CreateProfile(chatModel, close, submitting = chatModel.creatingProfileForInvitation.value) { displayName, shortDescr, image -> + if (chatModel.creatingProfileForInvitation.value) return@CreateProfile + chatModel.creatingProfileForInvitation.value = true + // On Main, like the pickers' own handlers: every call here suspends into IO, and the + // chat model is updated on Main by the receiver loop. + withApi { + try { + val ownerUserId = chatModel.currentUser.value?.userId + val profile = Profile(displayName.trim(), "", shortDescr.trim().ifEmpty { null }, image = image) + val newUser = controller.apiCreateActiveUser(rhId, profile, keepActiveUser = true) ?: return@withApi + // Before any early return below, as on iOS: nothing else adds it, so a profile + // left out of chatModel.users exists in the database but in no list the app + // shows, and creating it again fails on the duplicate name. + if (chatModel.remoteHostId() == rhId && chatModel.users.none { it.user.userId == newUser.userId }) { chatModel.users.add(UserInfo(newUser, 0)) } + if (newUser.activeUser) { + // An older remote host ignored the flag and activated it, so the reassignment + // would fail. Resync to what the host did and report it, with the form + // dismissed - the app is about to be showing a different profile. + if (modalManager.isLastModalOpenNotClosing(ModalViewId.CONTEXT_USER_PICKER_NEW_PROFILE)) close() + // changeActiveUser_, not changeActiveUser: the latter shows its own alert on + // failure, which would stack with the one below reporting the same thing. + runCatching { controller.changeActiveUser_(newUser.remoteHostId, newUser.userId, null) } + .onFailure { Log.e(TAG, "createProfileForInvitation: resync failed: ${it.stackTraceToString()}") } + AlertManager.shared.showAlertMsg(generalGetString(MR.strings.error_changing_user)) + return@withApi + } + // Refresh so the new profile carries its real unread count and ordering rather + // than the placeholder added above. listUsers throws and withApi does not catch. + if (chatModel.remoteHostId() == rhId) { + runCatching { controller.listUsers(rhId) }.getOrNull()?.let { + chatModel.users.clear() + chatModel.users.addAll(it) + } + } + // onCreated resolves the invitation under whatever is active when it runs, and a + // notification tap or a host switch can have changed that while we were creating. + if (chatModel.currentUser.value?.userId != ownerUserId || chatModel.remoteHostId() != rhId) { + // Closed: the profile exists, so leaving the form up means the next Create + // fails on the duplicate name for as long as the name is unchanged. + if (modalManager.isLastModalOpenNotClosing(ModalViewId.CONTEXT_USER_PICKER_NEW_PROFILE)) close() + AlertManager.shared.showAlertMsg(generalGetString(MR.strings.error_changing_user)) + return@withApi + } + // Form gone - the user backed out of it. Don't move the invitation under a screen + // they left, and don't report an error for something they did on purpose. + if (!modalManager.isLastModalOpenNotClosing(ModalViewId.CONTEXT_USER_PICKER_NEW_PROFILE)) { + Log.i(TAG, "createProfileForInvitation: form closed before the invitation moved") + return@withApi + } + close() + try { + onCreated(newUser) + } catch (e: Exception) { + // changeActiveUser_ throws, and withApi does not catch - the global handler + // would close a modal or clear chatId with nothing said about the failure. + Log.e(TAG, "createProfileForInvitation: moving the invitation failed: ${e.stackTraceToString()}") + AlertManager.shared.showAlertMsg(generalGetString(MR.strings.error_changing_user)) + } + } finally { + chatModel.creatingProfileForInvitation.value = false } - // onCreated resolves the invitation under whatever is active when it runs, and a - // notification tap or a host switch can have changed that while we were creating. - if (chatModel.currentUser.value?.userId != ownerUserId || chatModel.remoteHostId() != rhId) { - // Closed: the profile exists, so leaving the form up means the next Create - // fails on the duplicate name for as long as the name is unchanged. - if (modalManager.isLastModalOpenNotClosing(ModalViewId.CONTEXT_USER_PICKER_NEW_PROFILE)) close() - AlertManager.shared.showAlertMsg(generalGetString(MR.strings.error_changing_user)) - return@withApi - } - // Form gone - the user backed out of it. Don't move the invitation under a screen - // they left, and don't report an error for something they did on purpose. - if (!modalManager.isLastModalOpenNotClosing(ModalViewId.CONTEXT_USER_PICKER_NEW_PROFILE)) { - Log.i(TAG, "createProfileForInvitation: form closed before the invitation moved") - return@withApi - } - close() - try { - onCreated(newUser) - } catch (e: Exception) { - // changeActiveUser_ throws, and withApi does not catch - the global handler - // would close a modal or clear chatId with nothing said about the failure. - Log.e(TAG, "createProfileForInvitation: moving the invitation failed: ${e.stackTraceToString()}") - AlertManager.shared.showAlertMsg(generalGetString(MR.strings.error_changing_user)) - } - } finally { - chatModel.creatingProfileForInvitation.value = false } } } diff --git a/plans/2026-07-30-new-profile-for-invitation.md b/plans/2026-07-30-new-profile-for-invitation.md index e50016fc66..167d7bf0e6 100644 --- a/plans/2026-07-30-new-profile-for-invitation.md +++ b/plans/2026-07-30-new-profile-for-invitation.md @@ -367,6 +367,15 @@ re-derive them. replaced, and the `UserInfo` fallback above makes the count change in the one case this feature introduced. +- **The form's own close affordances are gated, the "covered by another modal" case is + not.** `ModalView(close, enableClose = !creatingProfileForInvitation)` now disables Back, + Esc and the back arrow while the profile is being created — the Kotlin equivalent of + iOS's `interactiveDismissDisabled`, following `MigrateFromDevice`/`ChooseServerOperators`, + which use the same idiom. Note the Android consequence: with no enabled `BackHandler`, + Back falls through to whatever handles it beneath rather than doing nothing. That is the + same trade-off the migration screens already accept, and it beats the alternative of + orphaning a profile. The separate case below is still open. + - **A modal pushed on top of the create form is indistinguishable from backing out of it** (`WelcomeView.kt`). The guard is `!isLastModalOpenNotClosing(...)`, which is also false when another modal covers the form — on Android all four `ModalManager` placements share @@ -464,3 +473,17 @@ This is the exact mirror of a bug this branch *did* have to fix — `selectProfi called `incognito.set(false)` before the connection move, which with the picker now staying open on failure produced "preference off, Incognito still ticked". Same shape, opposite direction; see §4. + +Also fixed in the same round, pre-existing and adjacent (like §9): + +- **`onChange(of: incognitoEnabled)` could strand the whole picker** + (`NewChat/NewChatView.swift`). Its work sits inside + `if let contactConn = contactConnection, let conn = try await apiSetConnectionIncognito(…)`, + so when `contactConnection` is nil the body did nothing and `profileSwitchStatus` stayed + `.switchingIncognito` forever. That is reachable: the incognito row renders + unconditionally, while "Add profile" is gated on `contactConnection != nil`. Pre-existing, + and the picker ended up dead either way — but this branch made it immediate rather than + after the 0.5 s spinner latch, because `allowsHitTesting(!busy)` now keys on + `profileSwitchStatus` directly. It is the exact twin of the `apiChangeConnectionUser` + -returns-nil case this branch already fixes, so it got the same `else`: reset the status + and roll the toggle back.