From 4776119e07728ebce68c2be6b8a7c8a3f5b772b1 Mon Sep 17 00:00:00 2001 From: Alan Wizemann Date: Mon, 27 Apr 2026 14:20:03 +0200 Subject: [PATCH] fix(ios-onboarding): hide Cancel on first-run onboarding (#55) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit App Store Connect feedback: "Cancel button not working" on the "Connect to Hermes" onboarding screen. Confirmed root cause in RootModel.cancelOnboarding: state = servers.isEmpty ? .onboarding(forNewServer: ServerID()) : .serverList When the user has zero configured servers (the first-run case), the conditional re-presented a fresh onboarding view. The button fired, the state mutated, but the visible result was "tap Cancel, get an identical screen" — indistinguishable from a dead button. The defensive intent ("don't strand the user on an empty server list") was reasonable, but the UX-as-shipped is worse than the strand it tried to prevent — first-run TestFlight users see a seemingly broken app. Fix at the right layer: don't show Cancel when there's nowhere to go. - New `canCancel: Bool` parameter on OnboardingRootView (default true). When false, the leading toolbar slot omits the Cancel button entirely. - RootView passes `canCancel: !model.servers.isEmpty`. - RootModel.cancelOnboarding simplified — drops the defensive `.isEmpty` re-loop branch, asserts the invariant in debug, and in release still routes to `.serverList` (which renders an empty-state with the "+ Add server" toolbar button) rather than re-presenting onboarding. Co-Authored-By: Claude Opus 4.7 (1M context) --- scarf/Scarf iOS/App/ScarfIOSApp.swift | 23 +++++++++++++++++-- .../Onboarding/OnboardingRootView.swift | 18 ++++++++++++++- 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/scarf/Scarf iOS/App/ScarfIOSApp.swift b/scarf/Scarf iOS/App/ScarfIOSApp.swift index bbb371e..e33c8d8 100644 --- a/scarf/Scarf iOS/App/ScarfIOSApp.swift +++ b/scarf/Scarf iOS/App/ScarfIOSApp.swift @@ -185,8 +185,20 @@ final class RootModel { /// Cancel an in-progress onboarding and return to the list. /// Called by the sheet's Cancel affordance. + /// + /// Issue #55: prior versions had a defensive `servers.isEmpty` + /// fallback that re-presented onboarding when there was nothing + /// to fall back to. That made Cancel look broken on first-run. + /// `OnboardingRootView` now hides the Cancel button when + /// `canCancel == false`, so this path is only ever reached when + /// at least one server already exists. In debug we assert that + /// invariant; in release we still route to `.serverList` (which + /// renders an empty-state with the "+ Add server" button) rather + /// than re-presenting onboarding, so the worst case is "user + /// sees the empty server list" rather than "Cancel does nothing." func cancelOnboarding() { - state = servers.isEmpty ? .onboarding(forNewServer: ServerID()) : .serverList + assert(!servers.isEmpty, "cancelOnboarding called with no servers — Cancel button should be hidden via OnboardingRootView.canCancel") + state = .serverList } /// Called from OnboardingView when the flow finishes. Reload the @@ -320,7 +332,14 @@ struct RootView: View { case .serverList: ServerListView(model: model) case .onboarding(let forNewServer): - OnboardingRootView(targetServerID: forNewServer) { + // canCancel is gated on whether there's a server list to + // return to (issue #55). On first-run the user MUST add + // their first server to use the app — the toolbar omits + // the Cancel button in that case. + OnboardingRootView( + targetServerID: forNewServer, + canCancel: !model.servers.isEmpty + ) { await model.onboardingFinished(serverID: forNewServer) } onCancel: { model.cancelOnboarding() diff --git a/scarf/Scarf iOS/Onboarding/OnboardingRootView.swift b/scarf/Scarf iOS/Onboarding/OnboardingRootView.swift index cba8921..922298b 100644 --- a/scarf/Scarf iOS/Onboarding/OnboardingRootView.swift +++ b/scarf/Scarf iOS/Onboarding/OnboardingRootView.swift @@ -18,15 +18,24 @@ struct OnboardingRootView: View { /// step 1 with nowhere to go. Optional for callers that don't /// need cancel (shouldn't be any, but keeps the API forgiving). let onCancel: @MainActor () -> Void + /// Whether the Cancel button should appear in the nav bar + /// (issue #55). False on the first-run onboarding where there + /// is no `.serverList` to fall back to — showing Cancel there + /// fired the action but the state machine routed straight back + /// into onboarding, so the button looked broken to TestFlight + /// users. + let canCancel: Bool @State private var vm: OnboardingViewModel init( targetServerID: ServerID, + canCancel: Bool = true, onFinished: @escaping @MainActor () async -> Void, onCancel: @escaping @MainActor () -> Void = {} ) { self.targetServerID = targetServerID + self.canCancel = canCancel self.onFinished = onFinished self.onCancel = onCancel let service = CitadelSSHService() @@ -63,9 +72,16 @@ struct OnboardingRootView: View { // to cancel. Hiding the button then also keeps // users from accidentally wiping a just-saved // server mid-race. + // + // Also hidden on first-run onboarding (issue #55): + // there is no server list to return to, so Cancel + // would either be inert (state machine looping + // back into onboarding) or confusing (an empty + // server list with no path forward). Better to + // not show the affordance at all. if case .connected = vm.step { EmptyView() - } else { + } else if canCancel { Button("Cancel") { onCancel() }