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 ef8a392b6f..def4e4798d 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 @@ -123,8 +123,8 @@ object ChatModel { // 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 + * form until the invitation has been moved onto it. Lives on the model rather than in + * the picker that starts 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.) */ 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 396ac2b05c..ddd333eaa3 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 @@ -60,7 +60,7 @@ fun bioFitsLimit(bio: String): Boolean { } @Composable -fun CreateProfile(onSubmit: (displayName: String, shortDescr: String, image: String?) -> Unit) { +fun CreateProfile(submitting: Boolean = false, onSubmit: (displayName: String, shortDescr: String, image: String?) -> Unit) { val scope = rememberCoroutineScope() val scrollState = rememberScrollState() val keyboardState by getKeyboardState() @@ -159,7 +159,7 @@ fun CreateProfile(onSubmit: (displayName: String, shortDescr: String, image: Str SettingsActionItem( painterResource(MR.images.ic_check), stringResource(MR.strings.create_another_profile_button), - disabled = !canCreateProfile(displayName.value) || !bioFitsLimit(shortDescr.value), + disabled = submitting || !canCreateProfile(displayName.value) || !bioFitsLimit(shortDescr.value), textColor = MaterialTheme.colors.primary, iconColor = MaterialTheme.colors.primary, click = { onSubmit(displayName.value, shortDescr.value, profileImage.value) }, @@ -362,13 +362,17 @@ fun createProfileForInvitation(rhId: Long?, onCreated: suspend (User) -> Unit) { // 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. + // Not while one is still being created: the in-flight job identifies its own form only + // by this id, so a second form opened underneath it would be the one closed and the + // wrong profile handed over. if (chatModel.creatingProfileForInvitation.value) return + // Two taps before the modal renders would otherwise stack two modals sharing one id, + // after which close() could dismiss the wrong one. NotClosing, or re-opening the form + // right after backing out of it is a silent no-op until the animation ends. val modalManager = ModalManager.fullscreen - if (modalManager.hasModalOpen(ModalViewId.CONTEXT_USER_PICKER_NEW_PROFILE)) return + if (modalManager.hasModalOpenNotClosing(ModalViewId.CONTEXT_USER_PICKER_NEW_PROFILE)) return modalManager.showModalCloseable(id = ModalViewId.CONTEXT_USER_PICKER_NEW_PROFILE) { close -> - CreateProfile { displayName, shortDescr, image -> + CreateProfile(submitting = chatModel.creatingProfileForInvitation.value) { displayName, shortDescr, image -> if (chatModel.creatingProfileForInvitation.value) return@CreateProfile chatModel.creatingProfileForInvitation.value = true withBGApi { @@ -383,8 +387,14 @@ fun createProfileForInvitation(rhId: Long?, onCreated: suspend (User) -> Unit) { // remote host ignoring the unknown field. Reassigning would now fail, so resync // to what the host actually did and report it. Not switching_profile_error_message: // that says the invitation was moved, and on this path it was not. - controller.changeActiveUser(newUser.remoteHostId, newUser.userId, null) - AlertManager.shared.showAlertMsg(generalGetString(MR.strings.error_changing_user)) + // The form is dismissed first: the app is about to be showing a different + // profile, and leaving the form on top of it invites a second attempt that + // would fail the same way. + withContext(Dispatchers.Main) { + if (modalManager.isLastModalOpenNotClosing(ModalViewId.CONTEXT_USER_PICKER_NEW_PROFILE)) close() + controller.changeActiveUser(newUser.remoteHostId, newUser.userId, null) + AlertManager.shared.showAlertMsg(generalGetString(MR.strings.error_changing_user)) + } return@withBGApi } // Keep chatModel.users current even if onCreated's reassignment fails - it only @@ -414,7 +424,10 @@ fun createProfileForInvitation(rhId: Long?, onCreated: suspend (User) -> Unit) { // 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 + if (!modalManager.isLastModalOpenNotClosing(ModalViewId.CONTEXT_USER_PICKER_NEW_PROFILE)) { + Log.i(TAG, "createProfileForInvitation: form closed before the invitation was moved, profile ${newUser.userId} left created") + return@withContext + } close() onCreated(newUser) } 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 a11c07cda7..1d533492e8 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 @@ -11,6 +11,7 @@ import androidx.compose.material.* import androidx.compose.runtime.* import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.ui.draw.alpha import androidx.compose.ui.draw.clip import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.text.font.FontWeight @@ -116,17 +117,15 @@ fun ComposeContextProfilePickerView( viewPwd = null, keepingChatId = chat.id ) + // No chatId assignment here. changeActiveUser_ passes keepingChatId, and + // updateChats only clears chatId when the chat is missing from the reloaded + // list - so on success this would be a no-op, and in the one case the guard + // does fire it would re-point the chat view at a chat that is not there. if (chatModel.currentUser.value?.userId != newUser.userId) { AlertManager.shared.showAlertMsg( generalGetString(MR.strings.switching_profile_error_title), String.format(generalGetString(MR.strings.switching_profile_error_message), newUser.chatViewName) ) - } else { - // Reopen the chat under the new profile - only once the switch is known to - // have happened, or this would point the chat view at a chat that now belongs - // to a different profile, defeating the guard in updateChats that clears it. - // The id is unchanged by the reassignment - it is the contact/group id. - chatModel.chatId.value = chat.id } } } finally { @@ -257,7 +256,15 @@ fun ComposeContextProfilePickerView( Modifier .fillMaxWidth() .sizeIn(minHeight = DEFAULT_MIN_SECTION_ITEM_HEIGHT + 8.dp) - .clickable(enabled = !busy, onClick = { createProfileForInvitation(rhId) { changeProfileTo(it) } }) + .clickable(enabled = !busy, onClick = { + // Same guard as every other row: the flag is live state the receiver loop + // flips, so it can turn true between this row being laid out and the tap. + if (!chat.chatInfo.profileChangeProhibited) { + createProfileForInvitation(rhId) { changeProfileTo(it) } + } else { + showCantChangeProfileAlert() + } + }) .padding(horizontal = DEFAULT_PADDING_HALF, vertical = 4.dp), horizontalArrangement = Arrangement.SpaceBetween, verticalAlignment = Alignment.CenterVertically @@ -286,7 +293,9 @@ fun ComposeContextProfilePickerView( LazyColumnWithScrollBarNoAppBar( Modifier .heightIn(max = MAX_USER_PICKER_HEIGHT) - .background(MaterialTheme.colors.surface), + .background(MaterialTheme.colors.surface) + // The rows are already unclickable while busy; this is the only thing that says so. + .alpha(if (busy) 0.6f else 1f), reverseLayout = true, maxHeight = remember { mutableStateOf(MAX_USER_PICKER_HEIGHT) }, containerAlignment = Alignment.BottomEnd @@ -340,7 +349,8 @@ fun ComposeContextProfilePickerView( fun CurrentSelection() { Column( Modifier - .background(MaterialTheme.colors.surface), + .background(MaterialTheme.colors.surface) + .alpha(if (busy) 0.6f else 1f), ) { Text( generalGetString(MR.strings.context_user_picker_your_profile), diff --git a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chatlist/ShareListView.kt b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chatlist/ShareListView.kt index 382d126db7..4aa3c9678b 100644 --- a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chatlist/ShareListView.kt +++ b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chatlist/ShareListView.kt @@ -107,7 +107,7 @@ private fun ShareListToolbar(chatModel: ChatModel, stopped: Boolean, onSearchVal .filter { u -> !u.user.activeUser && !u.user.hidden } .all { u -> u.unreadCount == 0 } UserProfileButton(chatModel.currentUser.value?.profile?.image, allRead) { - ModalManager.start.showCustomModal(keyboardCoversBar = false) { close -> + ModalManager.start.showCustomModal(keyboardCoversBar = false, id = ModalViewId.ACTIVE_PROFILE_PICKER) { close -> val search = rememberSaveable { mutableStateOf("") } ModalView( { close() }, 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 6c08cd27de..68267e1f54 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 @@ -92,7 +92,8 @@ class ModalData(val keyboardCoversBar: Boolean = true) { enum class ModalViewId { SECONDARY_CHAT, CONTEXT_USER_PICKER_INCOGNITO, - CONTEXT_USER_PICKER_NEW_PROFILE + CONTEXT_USER_PICKER_NEW_PROFILE, + ACTIVE_PROFILE_PICKER } class ModalManager(private val placement: ModalPlacement? = null) { @@ -125,14 +126,23 @@ class ModalManager(private val placement: ModalPlacement? = null) { * * 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. */ + * relied on. */ 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 } + /** [hasModalOpen] with the same exclusion as [isLastModalOpenNotClosing]: a modal + * dismissed but still animating out does not count. Without it, re-opening something + * straight after closing it is a silent no-op for the length of the animation. */ + fun hasModalOpenNotClosing(id: ModalViewId): Boolean { + for (i in modalViews.indices) { + if (i !in toRemove && modalViews.getOrNull(i)?.id == id) return true + } + return false + } + 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 -> ModalView(close, showClose = showClose, cardScreen = cardScreen, endButtons = endButtons, content = { content() }) 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 ca09fc9531..163031c948 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 @@ -302,7 +302,7 @@ fun ActiveProfilePicker( // 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) { + val filteredProfiles = remember(searchTextOrPassword.value, chatModel.users.size, chatModel.currentUser.value?.userId) { filteredProfiles(chatModel.users.map { it.user }.sortedBy { !it.activeUser }, searchTextOrPassword.value) } @@ -367,7 +367,12 @@ fun ActiveProfilePicker( } } - close() + // Only if this picker is still the top of its stack. The reassignment above can + // wait indefinitely on the retry alert, and back is not blocked while it does, so + // an unconditional close() here would pop whatever the user moved on to. + if (ModalManager.start.isLastModalOpenNotClosing(ModalViewId.ACTIVE_PROFILE_PICKER)) { + close() + } } finally { switchingProfile.value = false } @@ -580,7 +585,7 @@ private fun InviteView(rhId: Long?, connLinkInvitation: CreatedConnLink, contact end = 16.dp ), click = { - ModalManager.start.showCustomModal(keyboardCoversBar = false) { close -> + ModalManager.start.showCustomModal(keyboardCoversBar = false, id = ModalViewId.ACTIVE_PROFILE_PICKER) { close -> val search = rememberSaveable { mutableStateOf("") } ModalView( { close() }, diff --git a/tests/ChatTests/Profiles.hs b/tests/ChatTests/Profiles.hs index ee2ff5dc93..1729fffa80 100644 --- a/tests/ChatTests/Profiles.hs +++ b/tests/ChatTests/Profiles.hs @@ -4005,9 +4005,10 @@ testCreateUserKeepingActiveUser = testChat2 aliceProfile bobProfile test -- ... and the active user is unchanged, with no switch back needed bob ##> "/u" showActiveUser bob "bob (Bob)" - -- the new user's record is not active either, which is the field both clients - -- branch on to detect a core that ignored the flag; /u only covers the TVar. - -- (/users sorts by display name, so the order here says nothing about active_order.) + -- the new user's row is not active either - /u only covers the currentUser TVar. + -- Note this reads the stored row, not the CRActiveUser payload the clients branch + -- on, and /users sorts by display name, so it says nothing about active_order. + -- Neither of those is covered by any test here. bob ##> "/users" bob <## "bob (Bob) (active)" bob <## "robert"