Lock the Link New Device flow to portrait and improve Cancel/Try Again handling. (#5094)

* Lock the LinkNewDeviceScreen to portrait. * Add cancel buttons to the QR Code errors and handle cancel/startOver correctly. * Use the shared QRCodeErrorView in the LinkNewDeviceScreen. * Clarify the different QR Error/ErrorStates a bit more. * Update snapshots.
2026-11-03 06:55:59 +00:00 · 2026-02-12 11:21:24 +00:00
parent 6b93bbf558
commit 92a5fb395c
75 changed files with 303 additions and 193 deletions
@@ -16,10 +16,12 @@ enum LinkNewDeviceScreenCoordinatorAction {

 struct LinkNewDeviceScreenCoordinatorParameters {
    let clientProxy: ClientProxyProtocol
+    let orientationManager: OrientationManagerProtocol
 }

 final class LinkNewDeviceScreenCoordinator: CoordinatorProtocol {
    private let viewModel: LinkNewDeviceScreenViewModelProtocol
+    private let orientationManager: OrientationManagerProtocol
    
    private var cancellables = Set<AnyCancellable>()
 
@@ -30,6 +32,7 @@ final class LinkNewDeviceScreenCoordinator: CoordinatorProtocol {
    
    init(parameters: LinkNewDeviceScreenCoordinatorParameters) {
        viewModel = LinkNewDeviceScreenViewModel(clientProxy: parameters.clientProxy)
+        orientationManager = parameters.orientationManager
    }
    
    func start() {
@@ -47,6 +50,13 @@ final class LinkNewDeviceScreenCoordinator: CoordinatorProtocol {
            }
        }
        .store(in: &cancellables)
+        
+        orientationManager.setOrientation(.portrait)
+        orientationManager.lockOrientation(.portrait)
+    }
+    
+    func stop() {
+        orientationManager.lockOrientation(.all)
    }
        
    func toPresentable() -> AnyView {
@@ -14,7 +14,12 @@ enum LinkNewDeviceScreenViewModelAction {
 }

 struct LinkNewDeviceScreenViewState: BindableState {
-    enum Mode: Equatable { case loading, readyToLink(isGeneratingCode: Bool), notSupported }
+    enum Mode: Equatable {
+        case loading
+        case readyToLink(isGeneratingCode: Bool)
+        case error(QRCodeLoginState.ErrorState)
+    }
+    
    var mode: Mode = .loading
    
    let showLinkDesktopComputerButton: Bool
@@ -23,5 +28,6 @@ struct LinkNewDeviceScreenViewState: BindableState {
 enum LinkNewDeviceScreenViewAction {
    case linkMobileDevice
    case linkDesktopComputer
+    case errorAction(QRCodeErrorView.Action)
    case dismiss
 }
@@ -39,6 +39,8 @@ class LinkNewDeviceScreenViewModel: LinkNewDeviceScreenViewModelType, LinkNewDev
            Task { await linkMobileDevice() }
        case .linkDesktopComputer:
            actionsSubject.send(.linkDesktopComputer)
+        case .errorAction(let action):
+            handleErrorAction(action)
        case .dismiss:
            actionsSubject.send(.dismiss)
        }
@@ -50,7 +52,7 @@ class LinkNewDeviceScreenViewModel: LinkNewDeviceScreenViewModelType, LinkNewDev
        if await clientProxy.isLoginWithQRCodeSupported {
            state.mode = .readyToLink(isGeneratingCode: false)
        } else {
-            state.mode = .notSupported
+            state.mode = .error(.notSupported)
        }
    }
    
@@ -73,8 +75,22 @@ class LinkNewDeviceScreenViewModel: LinkNewDeviceScreenViewModelType, LinkNewDev
            actionsSubject.send(.linkMobileDevice(progressPublisher))
            state.mode = .readyToLink(isGeneratingCode: false)
        } catch {
-            #warning("Needs some form of re-usable error handling, will handle with the next screen.")
-            state.mode = .notSupported
+            // This is hard to share a mapping from the QRCodeLoginError with the
+            // QRCodeLoginScreen given that some of those are scan errors…
+            state.mode = .error(.unknown)
+        }
+    }
+    
+    private func handleErrorAction(_ action: QRCodeErrorView.Action) {
+        switch action {
+        case .startOver:
+            // Reset to ready state to allow trying again.
+            state.mode = .readyToLink(isGeneratingCode: false)
+        case .openSettings, .signInManually:
+            MXLog.error("Unexpected error action: \(action)")
+            actionsSubject.send(.dismiss)
+        case .cancel:
+            actionsSubject.send(.dismiss)
        }
    }
 }
@@ -5,6 +5,7 @@
 // Please see LICENSE files in the repository root for full details.
 //

+import Combine
 import Compound
 import SwiftUI

@@ -12,16 +13,24 @@ struct LinkNewDeviceScreen: View {
    @Bindable var context: LinkNewDeviceScreenViewModel.Context
    
    var body: some View {
-        FullscreenDialog(topPadding: 24, horizontalPadding: 24) {
-            mainContent
-        } bottomContent: {
-            buttons
+        switch context.viewState.mode {
+        case .error(let errorState):
+            QRCodeErrorView(errorState: errorState, canSignInManually: false) { action in
+                context.send(viewAction: .errorAction(action))
+            }
+            .backgroundStyle(.compound.bgCanvasDefault)
+        default:
+            FullscreenDialog(topPadding: 24, horizontalPadding: 24) {
+                mainContent
+            } bottomContent: {
+                buttons
+            }
+            .background()
+            .backgroundStyle(.compound.bgSubtleSecondary)
+            .navigationTitle(L10n.commonLinkNewDevice)
+            .navigationBarTitleDisplayMode(.inline)
+            .toolbar { toolbar }
        }
-        .background()
-        .backgroundStyle(.compound.bgSubtleSecondary)
-        .navigationTitle(L10n.commonLinkNewDevice)
-        .navigationBarTitleDisplayMode(.inline)
-        .toolbar { toolbar }
    }
    
    @ViewBuilder
@@ -31,11 +40,8 @@ struct LinkNewDeviceScreen: View {
            TitleAndIcon(title: L10n.screenLinkNewDeviceRootTitle,
                         icon: \.computer,
                         iconStyle: .default)
-        case .notSupported:
-            TitleAndIcon(title: L10n.screenLinkNewDeviceErrorNotSupportedTitle,
-                         subtitle: L10n.screenLinkNewDeviceErrorNotSupportedSubtitle,
-                         icon: \.errorSolid,
-                         iconStyle: .alertSolid)
+        case .error:
+            EmptyView() // Not reachable.
        }
    }
    
@@ -79,11 +85,8 @@ struct LinkNewDeviceScreen: View {
                }
            }
            .disabled(isGeneratingCode)
-        case .notSupported:
-            Button(L10n.actionDismiss) {
-                context.send(viewAction: .dismiss)
-            }
-            .buttonStyle(.compound(.primary))
+        case .error:
+            EmptyView() // Not reachable.
        }
    }
    
@@ -103,7 +106,8 @@ struct LinkNewDeviceScreen_Previews: PreviewProvider, TestablePreview {
    static let viewModel = makeViewModel(mode: .readyToLink(isGeneratingCode: false))
    static let generatingViewModel = makeViewModel(mode: .readyToLink(isGeneratingCode: true))
    static let loadingViewModel = makeViewModel(mode: .loading)
-    static let unsupportedViewModel = makeViewModel(mode: .notSupported)
+    static let unsupportedViewModel = makeViewModel(mode: .error(.notSupported))
+    static let unknownErrorViewModel = makeViewModel(mode: .error(.unknown))
    
    static var previews: some View {
        NavigationStack {
@@ -127,7 +131,13 @@ struct LinkNewDeviceScreen_Previews: PreviewProvider, TestablePreview {
            LinkNewDeviceScreen(context: unsupportedViewModel.context)
        }
        .previewDisplayName("Unsupported")
-        .snapshotPreferences(expect: unsupportedViewModel.context.observe(\.viewState.mode).map { $0 == .notSupported })
+        .snapshotPreferences(expect: unsupportedViewModel.context.observe(\.viewState.mode).map { $0 == .error(.notSupported) })
+        
+        NavigationStack {
+            LinkNewDeviceScreen(context: unknownErrorViewModel.context)
+        }
+        .previewDisplayName("Unknown error")
+        .snapshotPreferences(expect: unknownErrorViewModel.context.observe(\.viewState.mode).map { $0 == .error(.unknown) })
    }
    
    static func makeViewModel(mode: LinkNewDeviceScreenViewState.Mode) -> LinkNewDeviceScreenViewModel {
@@ -137,19 +147,25 @@ struct LinkNewDeviceScreen_Previews: PreviewProvider, TestablePreview {
            case .loading:
                try? await Task.sleep(for: .seconds(20))
                return false
-            case .readyToLink:
-                return true
-            case .notSupported:
+            case .error(.notSupported):
                return false
+            case .readyToLink, .error:
+                return true
            }
        }
        
+        let linkMobileProgressSubject = CurrentValueSubject<LinkNewDeviceService.LinkMobileProgress, QRCodeLoginError>(.starting)
+        clientProxy.linkNewDeviceServiceReturnValue = LinkNewDeviceServiceMock(.init(linkMobileProgressPublisher: linkMobileProgressSubject.asCurrentValuePublisher()))
+        
        let viewModel = LinkNewDeviceScreenViewModel(clientProxy: clientProxy)
        
        Task {
            try? await Task.sleep(for: .milliseconds(100))
            if case .readyToLink(isGeneratingCode: true) = mode {
                viewModel.context.send(viewAction: .linkMobileDevice)
+            } else if case .error = mode {
+                viewModel.context.send(viewAction: .linkMobileDevice)
+                linkMobileProgressSubject.send(completion: .failure(.unknown))
            }
        }
        
@@ -17,19 +17,26 @@ struct QRCodeLoginScreenCoordinatorParameters {
 }

 enum QRCodeLoginScreenCoordinatorAction: CustomStringConvertible {
-    case dismiss
+    /// Restart the flow.
+    ///
+    /// This action should only be sent when linking a new device. When logging in it
+    /// is handled internally within the screen.
+    case startOver
    case signInManually
    case signedIn(userSession: UserSessionProtocol)
    case requestOIDCAuthorisation(URL, OIDCAccountSettingsPresenter.Continuation)
    case linkedDevice
+    /// Cancel the flow (dismiss the modal).
+    case cancel
    
    var description: String {
        switch self {
-        case .dismiss: "dismiss"
+        case .startOver: "startOver"
        case .signInManually: "signInManually"
        case .signedIn: "signedIn"
        case .requestOIDCAuthorisation: "requestOIDCAuthorisation"
        case .linkedDevice: "linkedDevice"
+        case .cancel: "cancel"
        }
    }
 }
@@ -60,14 +67,16 @@ final class QRCodeLoginScreenCoordinator: CoordinatorProtocol {
            switch action {
            case .signInManually:
                actionsSubject.send(.signInManually)
-            case .dismiss:
-                actionsSubject.send(.dismiss)
+            case .startOver:
+                actionsSubject.send(.startOver)
            case .signedIn(let userSession):
                actionsSubject.send(.signedIn(userSession: userSession))
            case .requestOIDCAuthorisation(let url, let continuation):
                actionsSubject.send(.requestOIDCAuthorisation(url, continuation))
            case .linkedDevice:
                actionsSubject.send(.linkedDevice)
+            case .cancel:
+                actionsSubject.send(.cancel)
            }
        }
        .store(in: &cancellables)
@@ -9,19 +9,26 @@
 import SwiftUI

 enum QRCodeLoginScreenViewModelAction: CustomStringConvertible {
-    case dismiss
+    /// Restart the flow.
+    ///
+    /// This action should only be sent when linking a new device. When logging in it
+    /// is handled internally within the screen.
+    case startOver
    case signInManually
    case signedIn(userSession: UserSessionProtocol)
    case requestOIDCAuthorisation(URL, OIDCAccountSettingsPresenter.Continuation)
    case linkedDevice
+    /// Cancel the flow (dismiss the modal).
+    case cancel
    
    var description: String {
        switch self {
-        case .dismiss: "dismiss"
+        case .startOver: "startOver"
        case .signInManually: "signInManually"
        case .signedIn: "signedIn"
        case .requestOIDCAuthorisation: "requestOIDCAuthorisation"
        case .linkedDevice: "linkedDevice"
+        case .cancel: "cancel"
        }
    }
 }
@@ -49,13 +56,13 @@ struct QRCodeLoginScreenViewState: BindableState {
        switch mode {
        case .login:
            switch state {
-            case .loginInstructions, .scan, .error(.noCameraPermission): true
+            case .loginInstructions, .scan: true
            case .error: false
            case .linkDesktopInstructions, .displayCode, .displayQR, .confirmCode: false // Unreachable states.
            }
        case .linkDesktop, .linkMobile:
            switch state {
-            case .displayCode, .confirmCode, .scan, .error(.noCameraPermission): true
+            case .displayCode, .confirmCode, .scan: true
            case .linkDesktopInstructions, .displayQR, .error: false
            case .loginInstructions: false // Unreachable state.
            }
@@ -81,7 +88,8 @@ struct QRCodeLoginScreenViewStateBindings {
 }

 enum QRCodeLoginScreenViewAction {
-    case dismiss
+    /// Cancel the entire flow (dismiss the modal).
+    case cancel
    case startScan
    case sendCheckCode
    case errorAction(QRCodeErrorView.Action)
@@ -107,13 +115,17 @@ enum QRCodeLoginState: Equatable {
    case error(ErrorState)
    
    enum ErrorState: Equatable, CaseIterable {
+        /// The account provider doesn't support the use of QR codes.
+        case notSupported
        case noCameraPermission
        case connectionNotSecure
        case cancelled
        case declined
        case expired
+        /// The other device does not support linking Element X by QR code.
        case linkingNotSupported
-        case deviceNotSupported
+        /// Login cannot be continued due to a lack of Sliding Sync.
+        case slidingSyncNotAvailable
        /// Expected a QR code for a new device, however the processed code belongs to a device that is already signed in.
        case deviceAlreadySignedIn
        case unknown
@@ -49,8 +49,8 @@ class QRCodeLoginScreenViewModel: QRCodeLoginScreenViewModelType, QRCodeLoginScr
    
    override func process(viewAction: QRCodeLoginScreenViewAction) {
        switch viewAction {
-        case .dismiss, .errorAction(.dismiss):
-            actionsSubject.send(.dismiss)
+        case .cancel, .errorAction(.cancel):
+            actionsSubject.send(.cancel)
        case .startScan:
            Task { await startScanIfPossible() }
        case .sendCheckCode:
@@ -58,9 +58,12 @@ class QRCodeLoginScreenViewModel: QRCodeLoginScreenViewModelType, QRCodeLoginScr
        case .errorAction(.startOver):
            switch state.mode {
            case .login:
-                Task { await startScanIfPossible() }
+                // Login restarts the flow on this screen.
+                state.bindings.qrResult = nil
+                state.state = .loginInstructions
            case .linkDesktop, .linkMobile:
-                actionsSubject.send(.dismiss)
+                // Linking a new device starts on the previous screen.
+                actionsSubject.send(.startOver)
            }
        case .errorAction(.openSettings):
            appMediator.openAppSettings()
@@ -267,8 +270,8 @@ class QRCodeLoginScreenViewModel: QRCodeLoginScreenViewModelType, QRCodeLoginScr
                case .success:
                    break // The state will be updated by the status publisher.
                case .failure(.userCancellation):
-                    MXLog.info("User cancelled the WAS, dismissing.")
-                    actionsSubject.send(.dismiss)
+                    MXLog.info("User cancelled the WAS, the flow must be restarted.")
+                    actionsSubject.send(.startOver)
                case .failure:
                    handleError(.unknown)
                }
@@ -296,8 +299,8 @@ class QRCodeLoginScreenViewModel: QRCodeLoginScreenViewModelType, QRCodeLoginScr
            state.state = .error(.linkingNotSupported)
        case .expired:
            state.state = .error(.expired)
-        case .deviceNotSupported:
-            state.state = .error(.deviceNotSupported)
+        case .slidingSyncNotAvailable:
+            state.state = .error(.slidingSyncNotAvailable)
        case .deviceAlreadySignedIn:
            state.state = .error(.deviceAlreadySignedIn)
        case .unknown:
@@ -12,11 +12,13 @@ struct QRCodeErrorView: View {
    let errorState: QRCodeLoginState.ErrorState
    let canSignInManually: Bool
    
-    enum Action { case openSettings, startOver, signInManually, dismiss }
+    enum Action { case openSettings, startOver, signInManually, cancel }
    let action: (Action) -> Void
    
    var title: String {
        switch errorState {
+        case .notSupported:
+            L10n.screenLinkNewDeviceErrorNotSupportedTitle
        case .noCameraPermission:
            L10n.screenQrCodeLoginNoCameraPermissionStateTitle
        case .connectionNotSecure:
@@ -29,7 +31,7 @@ struct QRCodeErrorView: View {
            L10n.screenQrCodeLoginErrorExpiredTitle
        case .linkingNotSupported:
            L10n.screenQrCodeLoginErrorLinkingNotSuportedTitle
-        case .deviceNotSupported:
+        case .slidingSyncNotAvailable:
            L10n.screenQrCodeLoginErrorSlidingSyncNotSupportedTitle(InfoPlistReader.main.bundleDisplayName)
        case .deviceAlreadySignedIn:
            L10n.screenQrCodeLoginErrorDeviceAlreadySignedInTitle
@@ -40,6 +42,8 @@ struct QRCodeErrorView: View {
    
    var subtitle: String {
        switch errorState {
+        case .notSupported:
+            L10n.screenLinkNewDeviceErrorNotSupportedSubtitle
        case .noCameraPermission:
            L10n.screenQrCodeLoginNoCameraPermissionStateDescription(InfoPlistReader.main.productionAppName)
        case .connectionNotSecure:
@@ -52,7 +56,7 @@ struct QRCodeErrorView: View {
            L10n.screenQrCodeLoginErrorExpiredSubtitle
        case .linkingNotSupported:
            L10n.screenQrCodeLoginErrorLinkingNotSuportedSubtitle(InfoPlistReader.main.bundleDisplayName)
-        case .deviceNotSupported:
+        case .slidingSyncNotAvailable:
            L10n.screenQrCodeLoginErrorSlidingSyncNotSupportedSubtitle(InfoPlistReader.main.bundleDisplayName)
        case .deviceAlreadySignedIn:
            L10n.screenQrCodeLoginErrorDeviceAlreadySignedInSubtitle
@@ -117,26 +121,30 @@ struct QRCodeErrorView: View {
                     iconStyle: iconStyle)
    }
    
-    @ViewBuilder
    private var buttons: some View {
-        switch errorState {
-        case .noCameraPermission:
-            Button(L10n.screenQrCodeLoginNoCameraPermissionButton) {
-                action(.openSettings)
-            }
-            .buttonStyle(.compound(.primary))
-        case .connectionNotSecure, .unknown, .expired, .declined, .deviceNotSupported:
-            Button(L10n.screenQrCodeLoginStartOverButton) {
-                action(.startOver)
-            }
-            .buttonStyle(.compound(.primary))
-        case .cancelled:
-            Button(L10n.actionTryAgain) {
-                action(.startOver)
-            }
-            .buttonStyle(.compound(.primary))
-        case .linkingNotSupported:
-            VStack(spacing: 16) {
+        VStack(spacing: 16) {
+            switch errorState {
+            case .noCameraPermission:
+                Button(L10n.screenQrCodeLoginNoCameraPermissionButton) {
+                    action(.openSettings)
+                }
+                .buttonStyle(.compound(.primary))
+                
+                Button(L10n.actionCancel) {
+                    action(.cancel)
+                }
+                .buttonStyle(.compound(.secondary))
+            case .connectionNotSecure, .unknown, .expired, .declined, .slidingSyncNotAvailable, .cancelled:
+                Button(L10n.actionTryAgain) {
+                    action(.startOver)
+                }
+                .buttonStyle(.compound(.primary))
+                
+                Button(L10n.actionCancel) {
+                    action(.cancel)
+                }
+                .buttonStyle(.compound(.secondary))
+            case .notSupported, .linkingNotSupported:
                if canSignInManually {
                    Button(L10n.screenOnboardingSignInManually) {
                        action(.signInManually)
@@ -144,16 +152,16 @@ struct QRCodeErrorView: View {
                    .buttonStyle(.compound(.primary))
                }
                
-                Button(L10n.actionCancel) {
-                    action(.dismiss)
+                Button(L10n.actionDismiss) {
+                    action(.cancel)
                }
-                .buttonStyle(.compound(.tertiary))
+                .buttonStyle(.compound(canSignInManually ? .secondary : .primary))
+            case .deviceAlreadySignedIn:
+                Button(L10n.actionContinue) {
+                    action(.cancel)
+                }
+                .buttonStyle(.compound(.primary))
            }
-        case .deviceAlreadySignedIn:
-            Button(L10n.actionContinue) {
-                action(.dismiss)
-            }
-            .buttonStyle(.compound(.primary))
        }
    }
 }
@@ -181,13 +189,14 @@ struct QRCodeErrorView_Previews: PreviewProvider, TestablePreview {
 private extension QRCodeLoginState.ErrorState {
    var previewDisplayName: String {
        switch self {
+        case .notSupported: "Not supported"
        case .noCameraPermission: "No Camera Permission"
        case .connectionNotSecure: "Connection not secure"
        case .linkingNotSupported: "Linking unsupported"
        case .cancelled: "Cancelled"
        case .declined: "Declined"
        case .expired: "Expired"
-        case .deviceNotSupported: "Device not supported"
+        case .slidingSyncNotAvailable: "Sliding sync not available"
        case .deviceAlreadySignedIn: "Device already signed in"
        case .unknown: "Unknown error"
        }
@@ -116,7 +116,7 @@ struct QRCodeLoginScreen: View {
                }
            } bottomContent: {
                Button(L10n.actionCancel) {
-                    context.send(viewAction: .dismiss)
+                    context.send(viewAction: .cancel)
                }
                .buttonStyle(.compound(.secondary))
            }
@@ -281,7 +281,7 @@ struct QRCodeLoginScreen: View {
        ToolbarItem(placement: .cancellationAction) {
            if context.viewState.shouldDisplayCancelButton {
                Button(L10n.actionCancel) {
-                    context.send(viewAction: .dismiss)
+                    context.send(viewAction: .cancel)
                }
            }
        }