From 147955f52db4fcafff665095891de6534100217d Mon Sep 17 00:00:00 2001 From: Narasimha-sc <166327228+Narasimha-sc@users.noreply.github.com> Date: Thu, 6 Aug 2026 15:32:17 +0000 Subject: [PATCH] ios: fix the fallback ordering and close the input window on both pickers Third review pass. Still not compiled - no Swift toolchain here. The compose picker's fallback had moved the chatId clear out of the do block, so it fired when the switch had FAILED - closing a prepared chat that was still perfectly valid and still owned by the active profile. It is now conditional on the switch having happened. Dismissing the form before the switch did not help either: the dismissal animates for about a third of a second, getTopViewController() keeps returning the sheet until it ends, and the local database work finishes sooner - so the alert was presented on a controller being dismissed and dropped, on the common path. The explicit dismissal is gone (the switch removes this view and its sheet anyway) and the alert is deferred past the transition. The one-time link picker's rows had no guard at all: allowsHitTesting keyed on switchingProfileByTimeout, which latches half a second late, so a second row was tappable for that whole window right after the form closed - starting a second connection change on a pccConnId the first was recreating. Both pickers now block input from the moment the work starts. In the compose picker the guard is on the branches that start work, not on the whole Button, so expanding and collapsing the list - which is local - still works, and a tap in a chat where the profile cannot be changed still explains itself. Also: that picker's other-profiles list sorted activeOrder ascending while every other list on both platforms sorts descending, which the core change made visible by giving a never-activated profile order 0; the one-time link picker's fallback now dismisses and resyncs its selection as the compose picker does; and the incognito handler's new else no longer writes incognitoEnabled, which is bound to the app-wide default and marks the invitation as used. --- .../ContextProfilePickerView.swift | 36 ++++++++------ .../Shared/Views/NewChat/NewChatView.swift | 49 ++++++++++++------- 2 files changed, 52 insertions(+), 33 deletions(-) diff --git a/apps/ios/Shared/Views/Chat/ComposeMessage/ContextProfilePickerView.swift b/apps/ios/Shared/Views/Chat/ComposeMessage/ContextProfilePickerView.swift index 9ac3b03141..5da33af018 100644 --- a/apps/ios/Shared/Views/Chat/ComposeMessage/ContextProfilePickerView.swift +++ b/apps/ios/Shared/Views/Chat/ComposeMessage/ContextProfilePickerView.swift @@ -104,7 +104,10 @@ struct ContextProfilePickerView: View { let otherUsers = users .filter { u in u.userId != selectedUser.userId } - .sorted(using: KeyPathComparator(\.activeOrder)) + // Descending, as every other profile list sorts: a profile + // that was never activated has active_order 0 and belongs + // at the end, not the front. + .sorted(using: KeyPathComparator(\.activeOrder, order: .reverse)) ForEach(otherUsers) { p in profilerPickerUserOption(p) .contentShape(Rectangle()) @@ -171,7 +174,6 @@ struct ContextProfilePickerView: View { private func profilerPickerUserOption(_ user: User) -> some View { Button { - if busy { return } if !chat.chatInfo.profileChangeProhibited { if selectedUser == user { if !incognitoDefault { @@ -181,6 +183,10 @@ struct ContextProfilePickerView: View { listExpanded = false } } else if selectedUser != user { + // Only the branch that starts work is guarded - expanding and + // collapsing the list is local and stays available. + if busy { return } + changingProfile = true changeProfile(user) } } else { @@ -275,23 +281,26 @@ struct ContextProfilePickerView: View { // resync to what the host actually did and report it. The failure is the // switch, not the creation, so it is not rethrown into the form's "error // creating profile" handler. - // - // Dismiss the form first. The switch below replaces the chat list with the new - // user's, and the prepared chat stayed with the previous one - so this view, - // which is the sheet's presenter, is removed from the hierarchy by the switch - // itself. Nothing shown from inside it would survive, and an alert raised - // while it is being torn down is discarded. - await MainActor.run { showAddProfile = false } + var switched = false do { try await changeActiveUserAsync_(newUser.userId, viewPwd: nil) + switched = true } catch { logger.error("changeActiveUserAsync_ error: \(responseError(error))") } await MainActor.run { - // The prepared chat is not in the new user's list, so a pushed chat view - // would render blank. Only clear it if it is still the chat this picker - // belongs to - a notification tap may have navigated elsewhere by now. - if chatModel.chatId == chat.id { chatModel.chatId = nil } + // Only when the switch actually happened: the prepared chat then belongs + // to a profile that is no longer active, so it is absent from the reloaded + // list and a pushed chat view renders blank. If the switch failed nothing + // moved and the chat is still fine - closing it would be the regression. + if switched && chatModel.chatId == chat.id { chatModel.chatId = nil } + } + // The switch replaces the chat list, which removes this view - and the sheet + // it presents - from the hierarchy. Both that teardown and an explicit + // dismissal animate, and getTopViewController() keeps returning the sheet + // until the transition ends, so an alert raised now is presented on a + // controller being dismissed and dropped. Let it settle first. + DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) { showAlert(NSLocalizedString("Error changing chat profile", comment: "alert title")) } return @@ -361,7 +370,6 @@ struct ContextProfilePickerView: View { private func incognitoOption() -> some View { Button { - if busy { return } if !chat.chatInfo.profileChangeProhibited { if incognitoDefault { listExpanded.toggle() diff --git a/apps/ios/Shared/Views/NewChat/NewChatView.swift b/apps/ios/Shared/Views/NewChat/NewChatView.swift index a31bd3ec14..91730085d1 100644 --- a/apps/ios/Shared/Views/NewChat/NewChatView.swift +++ b/apps/ios/Shared/Views/NewChat/NewChatView.swift @@ -428,13 +428,13 @@ private struct ActiveProfilePicker: View { dismiss() } } else { - // Same latch as in the selectedProfile handler below: without - // this, a nil result leaves profileSwitchStatus stuck and the - // picker dimmed behind a spinner with hit testing off. - await MainActor.run { - profileSwitchStatus = .idle - incognitoEnabled = !incognito - } + // Only reachable with no connection - apiSetConnectionIncognito + // throws rather than returning nil. Nothing was changed, so just + // release the status, which otherwise stays .switchingIncognito + // and latches the picker behind a spinner with hit testing off. + // incognitoEnabled is left alone: it is bound to the app-wide + // default, and writing it marks the invitation as used. + await MainActor.run { profileSwitchStatus = .idle } } } catch { logger.error("apiSetConnectionIncognito error: \(responseError(error))") @@ -529,9 +529,15 @@ private struct ActiveProfilePicker: View { } + // switchingProfileByTimeout only latches half a second after a switch starts, to keep + // the spinner from flickering on fast switches - but input has to be blocked from the + // moment the work begins, or a second tap inside that window starts a competing + // connection change and user switch. + private var busy: Bool { creatingProfile || switchingProfileByTimeout || profileSwitchStatus != .idle } + @ViewBuilder private func viewBody() -> some View { profilePicker() - .allowsHitTesting(!switchingProfileByTimeout) + .allowsHitTesting(!busy) .modifier(ThemedBackground(grouped: true)) .overlay { if switchingProfileByTimeout { @@ -598,7 +604,7 @@ private struct ActiveProfilePicker: View { // profileSwitchStatus, not just switchingProfileByTimeout: that only latches half // a second later, and creatingProfile is cleared as soon as the switch is handed // to the selectedProfile handler - leaving the row live in between. - .disabled(creatingProfile || switchingProfileByTimeout || profileSwitchStatus != .idle) + .disabled(busy) } // Creates a profile for this invitation without activating it, then routes through @@ -628,19 +634,24 @@ private struct ActiveProfilePicker: View { // creating profile" handler. do { try await changeActiveUserAsync_(newUser.userId, viewPwd: nil) - } catch {} - // Not the view's own `alert`: it is bound to a view that is presenting this - // form as a sheet, and UIKit refuses an alert on a controller that already - // has one presented - it would never be shown. showAlert goes to the top view - // controller, so it appears over the form, which stays open as on any other - // failure. + } catch { + logger.error("changeActiveUserAsync_ error: \(responseError(error))") + } + // Dismiss the form, as the compose picker does on the same failure: the app is + // now showing a different profile and the connection stayed with the previous + // one, so there is nothing left to do here and a second attempt would fail the + // same way. await MainActor.run { - // The app has switched to the new profile, so make the picker agree with - // it rather than leaving the checkmark on a profile that is no longer - // active. The connection stayed with the previous profile and cannot be - // moved from here; the alert says so. + showAddProfile = false + // Make the picker agree with the profile that is now active rather than + // leaving the checkmark on one that is not. profileSwitchStatus = .idle selectedProfile = newUser + } + // getTopViewController() keeps returning the sheet until its dismissal + // transition ends, and an alert presented on a controller being dismissed is + // dropped - so let it settle first. + DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) { showAlert(NSLocalizedString("Error changing chat profile", comment: "alert title")) } return