Repository navigation
iOS: first-run onboarding before pairing #5655
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
c914929
d6cfcf3
b5f7ccf
408ea6b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| public import Foundation | ||
|
|
||
| /// Tracks whether the user has seen the first-run onboarding, persisted in an | ||
| /// injected `UserDefaults`. | ||
| /// | ||
| /// The onboarding explains what cmux is and how the phone pairs to a Mac. It is | ||
| /// presented post-authentication, in front of the never-paired add-device state, | ||
| /// and must show once per install and never reappear. The seen flag is read | ||
| /// synchronously at construction time (mirroring `MobileClientIDRepository` / | ||
| /// `MobileDisplaySettings`): the root view reads it before deciding what to | ||
| /// mount, which avoids a flash of the add-device screen ahead of onboarding on | ||
| /// first launch. | ||
| /// | ||
| /// The backing `UserDefaults` is injected so the store is testable without | ||
| /// touching `.standard`; the app constructs it at the composition root with | ||
| /// `UserDefaults.standard`. | ||
| /// | ||
| /// `forceSeen` lets the caller treat onboarding as already seen regardless of | ||
| /// what is persisted. The mobile app passes the UI-test / dogfood bypass through | ||
| /// this so the XCUITest harness and the dev-launch auto-pair path are not wedged | ||
| /// behind a manual tap-through (the bypass decision itself lives in the UI layer, | ||
| /// which can read `UITestConfig`; this type stays dependency-light). | ||
| /// | ||
| /// ```swift | ||
| /// let store = MobileOnboardingStore(defaults: .standard, forceSeen: false) | ||
| /// if !store.hasSeenOnboarding { /* present onboarding */ } | ||
| /// store.markSeen() | ||
| /// ``` | ||
| public struct MobileOnboardingStore: Sendable { | ||
| /// The defaults key under which the seen flag is stored. | ||
| public static let defaultsKey = "dev.cmux.mobile.onboarding.seen.v1" | ||
|
|
||
| // UserDefaults is Apple-documented thread-safe; OK to hold nonisolated. | ||
| private nonisolated(unsafe) let defaults: UserDefaults | ||
| private let forceSeen: Bool | ||
|
|
||
| /// Create a store backed by the given defaults. | ||
| /// - Parameters: | ||
| /// - defaults: The persistence store for the seen flag. Inject a | ||
| /// suite-scoped `UserDefaults` in tests. | ||
| /// - forceSeen: When `true`, ``hasSeenOnboarding`` always returns `true` | ||
| /// and ``markSeen()`` is a no-op, so onboarding never presents. The app | ||
| /// passes the UI-test / dogfood bypass here. | ||
| public init(defaults: UserDefaults, forceSeen: Bool = false) { | ||
| self.defaults = defaults | ||
| self.forceSeen = forceSeen | ||
| } | ||
|
|
||
| /// Whether the first-run onboarding has already been shown on this install. | ||
| /// | ||
| /// Returns `true` when `forceSeen` is set (UI-test / dogfood bypass) or when | ||
| /// the seen flag is persisted. Read synchronously so the root view never | ||
| /// flashes a later screen before deciding to present onboarding. | ||
| public var hasSeenOnboarding: Bool { | ||
| if forceSeen { return true } | ||
| return defaults.bool(forKey: Self.defaultsKey) | ||
| } | ||
|
|
||
| /// Persist that the user has finished (or skipped) onboarding. | ||
| /// | ||
| /// A no-op when `forceSeen` is set, so the bypass never writes through to the | ||
| /// real install's defaults. | ||
| public func markSeen() { | ||
| guard !forceSeen else { return } | ||
| defaults.set(true, forKey: Self.defaultsKey) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| import Foundation | ||
| import Testing | ||
|
|
||
| @testable import CmuxMobileShellModel | ||
|
|
||
| /// Behavior tests for ``MobileOnboardingStore`` using a suite-scoped | ||
| /// `UserDefaults` so they never touch `UserDefaults.standard`. | ||
| @Suite struct MobileOnboardingStoreTests { | ||
| private func makeDefaults() -> UserDefaults { | ||
| let suite = "MobileOnboardingStoreTests.\(UUID().uuidString)" | ||
| let defaults = UserDefaults(suiteName: suite)! | ||
| defaults.removePersistentDomain(forName: suite) | ||
| return defaults | ||
| } | ||
|
|
||
| @Test func startsUnseenAndPersistsSeen() { | ||
| let defaults = makeDefaults() | ||
| let store = MobileOnboardingStore(defaults: defaults) | ||
| #expect(!store.hasSeenOnboarding) | ||
|
|
||
| store.markSeen() | ||
| #expect(store.hasSeenOnboarding) | ||
| #expect(defaults.bool(forKey: MobileOnboardingStore.defaultsKey)) | ||
| } | ||
|
|
||
| @Test func readsAPreviouslyPersistedSeenFlag() { | ||
| let defaults = makeDefaults() | ||
| defaults.set(true, forKey: MobileOnboardingStore.defaultsKey) | ||
| let store = MobileOnboardingStore(defaults: defaults) | ||
| #expect(store.hasSeenOnboarding) | ||
| } | ||
|
|
||
| /// `forceSeen` reports seen without reading or writing the backing defaults, | ||
| /// so the UI-test / dogfood bypass never wedges behind onboarding and never | ||
| /// pollutes the real install's persisted flag. | ||
| @Test func forceSeenReportsSeenWithoutPersisting() { | ||
| let defaults = makeDefaults() | ||
| let store = MobileOnboardingStore(defaults: defaults, forceSeen: true) | ||
| #expect(store.hasSeenOnboarding) | ||
|
|
||
| store.markSeen() | ||
| #expect(!defaults.bool(forKey: MobileOnboardingStore.defaultsKey)) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,19 +1,43 @@ | ||
| import CmuxMobileShell | ||
| import SwiftUI | ||
| #if os(iOS) | ||
| import CmuxMobileShellModel | ||
| @preconcurrency import UIKit | ||
| #elseif os(macOS) | ||
| import AppKit | ||
| #endif | ||
|
|
||
| public struct CMUXMobileAppView: View { | ||
| @State private var store: CMUXMobileShellStore | ||
| #if os(iOS) | ||
| private let onboardingStore: MobileOnboardingStore | ||
| #endif | ||
|
|
||
| #if os(iOS) | ||
| /// Creates the app view. | ||
| /// - Parameters: | ||
| /// - store: The shell store backing the workspace UI. | ||
| /// - onboardingStore: The first-run onboarding "seen" flag store. Defaults | ||
| /// to a `.standard`-backed store marked already-seen, so SwiftUI previews | ||
| /// and ad-hoc construction never present onboarding. | ||
| public init( | ||
| store: CMUXMobileShellStore = .preview(), | ||
| onboardingStore: MobileOnboardingStore = MobileOnboardingStore(defaults: .standard, forceSeen: true) | ||
| ) { | ||
| _store = State(initialValue: store) | ||
| self.onboardingStore = onboardingStore | ||
| } | ||
| #else | ||
| public init(store: CMUXMobileShellStore = .preview()) { | ||
| _store = State(initialValue: store) | ||
| } | ||
| #endif | ||
|
|
||
| public var body: some View { | ||
| #if os(iOS) | ||
| CMUXMobileRootView(store: store, onboardingStore: onboardingStore) | ||
| #else | ||
| CMUXMobileRootView(store: store) | ||
| #endif | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| import Foundation | ||
| import CmuxAuthRuntime | ||
| import CmuxMobileShell | ||
| import CmuxMobileShellModel | ||
| import CmuxMobileSupport | ||
| import CmuxMobileWorkspace | ||
| import SwiftUI | ||
|
|
@@ -16,6 +17,15 @@ struct CMUXMobileRootView: View { | |
| @Environment(AuthCoordinator.self) private var authManager | ||
| #if os(iOS) | ||
| @Environment(MobilePushCoordinator.self) private var pushCoordinator | ||
| /// The persisted first-run onboarding "seen" flag store. The one-time | ||
| /// onboarding screen gates ahead of the never-paired add-device state. | ||
| private let onboardingStore: MobileOnboardingStore | ||
| /// Mirrors ``MobileOnboardingStore/hasSeenOnboarding`` so completing | ||
| /// onboarding (which calls `markSeen()` in the button action) re-renders the | ||
| /// root and falls through to the pairing flow. Seeded synchronously from the | ||
| /// store so the very first frame already reflects a prior install's state and | ||
| /// never flashes onboarding for a returning user. | ||
| @State private var hasSeenOnboarding: Bool | ||
| #endif | ||
| @State private var pendingAttachURL: String? | ||
| @State private var didConsumeUITestAttachURL = false | ||
|
|
@@ -32,6 +42,18 @@ struct CMUXMobileRootView: View { | |
| /// Tailscale guidance. | ||
| @Environment(\.tailscaleStatusMonitor) private var tailscaleStatusMonitor | ||
|
|
||
| #if os(iOS) | ||
| init(store: CMUXMobileShellStore, onboardingStore: MobileOnboardingStore) { | ||
| self.store = store | ||
| self.onboardingStore = onboardingStore | ||
| _hasSeenOnboarding = State(initialValue: onboardingStore.hasSeenOnboarding) | ||
| } | ||
| #else | ||
| init(store: CMUXMobileShellStore) { | ||
| self.store = store | ||
| } | ||
| #endif | ||
|
|
||
| private var shouldShowTerminalLayoutPreview: Bool { | ||
| #if os(iOS) && DEBUG | ||
| return UITestConfig.terminalLayoutPreviewEnabled | ||
|
|
@@ -139,6 +161,13 @@ struct CMUXMobileRootView: View { | |
| // yet know if there is a session to restore. | ||
| MobilePairedMacDeterminingView() | ||
| } | ||
| } else if shouldShowOnboarding { | ||
| // Placed after the reconnect-determining branch so `hasKnownPairedMac` | ||
| // has resolved: a genuine first run (never onboarded, never paired) | ||
| // sees the one-time explainer before the add-device flow; a returning | ||
| // paired-but-offline user (who can reach here after a failed | ||
| // reconnect) is excluded by the gate and falls through to pairing. | ||
| onboardingFlow | ||
| } else if store.connectionState != .connected { | ||
| DisconnectedWorkspaceShellView( | ||
| hasKnownPairedMac: store.hasKnownPairedMac, | ||
|
|
@@ -173,6 +202,38 @@ struct CMUXMobileRootView: View { | |
| } | ||
| } | ||
|
|
||
| /// Whether the one-time first-run onboarding should be presented. Always | ||
| /// `false` off iOS (onboarding is iOS-only). | ||
| private var shouldShowOnboarding: Bool { | ||
| #if os(iOS) | ||
| return MobileOnboardingGate.shouldShowOnboarding( | ||
| hasSeenOnboarding: hasSeenOnboarding, | ||
| hasKnownPairedMac: store.hasKnownPairedMac | ||
|
Comment on lines
+209
to
+211
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a returning install has an active paired Mac record but none of its saved routes are supported by the current runtime, Useful? React with 👍 / 👎. |
||
| ) | ||
| #else | ||
| return false | ||
| #endif | ||
| } | ||
|
|
||
| @ViewBuilder | ||
| private var onboardingFlow: some View { | ||
| #if os(iOS) | ||
| OnboardingFlowView(onComplete: completeOnboarding) | ||
| #else | ||
| EmptyView() | ||
| #endif | ||
| } | ||
|
|
||
| #if os(iOS) | ||
| /// Persists the onboarding "seen" flag and re-renders so the root falls | ||
| /// through to the pairing flow. Called from the onboarding button actions | ||
| /// (Skip / Get started), not a view-lifecycle callback. | ||
| private func completeOnboarding() { | ||
| onboardingStore.markSeen() | ||
| hasSeenOnboarding = true | ||
| } | ||
| #endif | ||
|
|
||
| private var isAuthenticated: Bool { | ||
| MobileRootAuthGate.isAuthenticated( | ||
| stackAuthenticated: authManager.isAuthenticated, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,107 @@ | ||
| #if os(iOS) | ||
| import CmuxMobileSupport | ||
| import SwiftUI | ||
|
|
||
| /// First-run onboarding that explains what cmux is and how the phone connects to | ||
| /// a Mac, then hands off to the existing pairing flow. | ||
| /// | ||
| /// This view is deliberately pairing-ignorant. It owns no auth, store, or | ||
| /// pairing state: it walks the user through three explanatory pages and then | ||
| /// calls ``onComplete``. The caller decides what "complete" means: | ||
| /// | ||
| /// - First launch: presented post-authentication, in front of the never-paired | ||
| /// add-device state. The root view marks onboarding seen and falls through to | ||
| /// `DisconnectedWorkspaceShellView`, which already auto-presents `PairingView`, | ||
| /// so the "pair now" handoff is automatic and nothing here is duplicated. | ||
| /// - Settings ("How pairing works"): the entry just dismisses. | ||
| /// | ||
| /// The `onComplete` closure is the extensibility seam. A future "add your own | ||
| /// Linux/Mac servers" (Hive) path can branch the final CTA without changing this | ||
| /// view's body. | ||
| struct OnboardingFlowView: View { | ||
| /// Called when the user finishes the last page or skips. The caller marks the | ||
| /// flow seen (first launch) and/or dismisses the presentation. | ||
| let onComplete: () -> Void | ||
|
|
||
| @State private var pageIndex = 0 | ||
| @Environment(\.analytics) private var analytics | ||
|
|
||
| private let pages = OnboardingPage.allPages | ||
|
|
||
| var body: some View { | ||
| VStack(spacing: 0) { | ||
| header | ||
|
|
||
| TabView(selection: $pageIndex) { | ||
| ForEach(Array(pages.enumerated()), id: \.offset) { index, page in | ||
| OnboardingPageView(page: page) | ||
| .tag(index) | ||
| } | ||
| } | ||
| .tabViewStyle(.page(indexDisplayMode: .always)) | ||
| .indexViewStyle(.page(backgroundDisplayMode: .always)) | ||
|
|
||
| footer | ||
| } | ||
| .background(PlatformPalette.systemBackground.ignoresSafeArea()) | ||
| .interactiveDismissDisabled() | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! |
||
| .accessibilityIdentifier("MobileOnboardingFlow") | ||
| .onAppear { | ||
| analytics.capture("ios_onboarding_viewed", ["page": .int(0)]) | ||
| } | ||
| .onChange(of: pageIndex) { _, newValue in | ||
| analytics.capture("ios_onboarding_viewed", ["page": .int(newValue)]) | ||
|
Comment on lines
+49
to
+53
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| } | ||
| } | ||
|
|
||
| private var header: some View { | ||
| HStack { | ||
| Spacer() | ||
| Button { | ||
| analytics.capture("ios_onboarding_skipped", ["page": .int(pageIndex)]) | ||
| onComplete() | ||
| } label: { | ||
| Text(L10n.string("mobile.onboarding.skip", defaultValue: "Skip")) | ||
| .font(.subheadline) | ||
| } | ||
| .accessibilityIdentifier("MobileOnboardingSkipButton") | ||
| } | ||
| .padding(.horizontal, 20) | ||
| .padding(.top, 12) | ||
| } | ||
|
|
||
| private var footer: some View { | ||
| VStack(spacing: 12) { | ||
| Button { | ||
| advance() | ||
| } label: { | ||
| Text(isLastPage | ||
| ? L10n.string("mobile.onboarding.getStarted", defaultValue: "Get started") | ||
| : L10n.string("mobile.onboarding.next", defaultValue: "Next")) | ||
| .fontWeight(.semibold) | ||
| .frame(maxWidth: .infinity) | ||
| .contentShape(.capsule) | ||
| } | ||
| .mobileGlassProminentButton() | ||
| .accessibilityIdentifier("MobileOnboardingPrimaryButton") | ||
| } | ||
| .padding(.horizontal, 24) | ||
| .padding(.bottom, 16) | ||
| } | ||
|
|
||
| private var isLastPage: Bool { | ||
| pageIndex >= pages.count - 1 | ||
| } | ||
|
|
||
| private func advance() { | ||
| if isLastPage { | ||
| analytics.capture("ios_onboarding_completed", [:]) | ||
| onComplete() | ||
| return | ||
| } | ||
| withAnimation(.snappy(duration: 0.2)) { | ||
| pageIndex += 1 | ||
| } | ||
| } | ||
| } | ||
| #endif | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Onboarding shows while connected
Medium Severity
The onboarding branch runs before the connected workspace branch and is not gated on
connectionState. If the shell reaches.connectedwhilehasKnownPairedMacis still false, first-run onboarding replaces the live workspace until the user skips or finishes it.Reviewed by Cursor Bugbot for commit 408ea6b. Configure here.