diff --git a/packages/app/src/onboarding/internal/model.test.ts b/packages/app/src/onboarding/internal/model.test.ts index 548311274e..ee3a8b00c9 100644 --- a/packages/app/src/onboarding/internal/model.test.ts +++ b/packages/app/src/onboarding/internal/model.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest"; import { createOnboardingModel, type OnboardingCompletionStorage, + type OnboardingModel, type OnboardingSetupHost, } from "./model"; @@ -28,6 +29,19 @@ const FRESH_HOST: OnboardingSetupHost = { hasProjects: false, }; +/** Presses Continue on whatever step is showing. */ +function advance(model: OnboardingModel) { + const state = model.getState(); + if (state.status !== "open") throw new Error("setup is not open"); + model.next(state.visit); +} + +function currentVisit(model: OnboardingModel): number { + const state = model.getState(); + if (state.status !== "open") throw new Error("setup is not open"); + return state.visit; +} + async function openModel(storage = new MemoryCompletionStorage()) { const model = createOnboardingModel({ storage }); await model.ready; @@ -40,7 +54,7 @@ describe("onboarding model", () => { model.applySetupHost(FRESH_HOST, { includePhoneStep: true }); - expect(model.getState()).toEqual({ + expect(model.getState()).toMatchObject({ status: "open", serverId: "local", steps: ["appearance", "providers", "projects", "orchestration", "phone"], @@ -93,8 +107,8 @@ describe("onboarding model", () => { it("keeps the modal open when the host gains its first project during setup", async () => { const model = await openModel(); model.applySetupHost(FRESH_HOST, { includePhoneStep: true }); - model.next("appearance"); - model.next("providers"); + advance(model); + advance(model); model.applySetupHost({ ...FRESH_HOST, hasProjects: true }, { includePhoneStep: true }); @@ -117,10 +131,10 @@ describe("onboarding model", () => { model.back(); expect(model.getState()).toMatchObject({ stepIndex: 0 }); - model.next("appearance"); - model.next("providers"); - model.next("projects"); - model.next("orchestration"); + advance(model); + advance(model); + advance(model); + advance(model); expect(model.getState()).toMatchObject({ stepIndex: 3 }); model.goTo("providers"); expect(model.getState()).toMatchObject({ stepIndex: 1 }); @@ -128,21 +142,24 @@ describe("onboarding model", () => { expect(model.getState()).toMatchObject({ stepIndex: 0 }); }); - it("ignores a late advance from a step the user already left", async () => { + it("ignores a late advance from a step visit the user already left", async () => { const model = await openModel(new MemoryCompletionStorage(true)); model.reopen("local", { includePhoneStep: true }); model.goTo("projects"); - // Projects are still being added when the user picks Appearance in the step rail. + // Projects are still being added when the user leaves Projects and comes back. + const addingFrom = currentVisit(model); model.goTo("appearance"); - model.next("projects"); - expect(model.getState()).toMatchObject({ stepIndex: 0 }); + model.goTo("projects"); + model.next(addingFrom); + expect(model.getState()).toMatchObject({ stepIndex: 2 }); - // Or closes setup and opens it again before the add finishes. + // Or closes setup, opens it again and returns to Projects before the add finishes. await model.complete(); model.reopen("local", { includePhoneStep: true }); - model.next("projects"); - expect(model.getState()).toMatchObject({ status: "open", stepIndex: 0 }); + model.goTo("projects"); + model.next(addingFrom); + expect(model.getState()).toMatchObject({ status: "open", stepIndex: 2 }); }); it("records completion when setup closes, so it does not open again", async () => { @@ -177,7 +194,7 @@ describe("onboarding model", () => { }); model.applySetupHost(FRESH_HOST, { includePhoneStep: true }); - model.next("appearance"); + advance(model); await model.complete(); expect(seen).toEqual(["open:0", "open:1", "hidden"]); diff --git a/packages/app/src/onboarding/internal/model.ts b/packages/app/src/onboarding/internal/model.ts index 66f91953fa..1ac2f01f4a 100644 --- a/packages/app/src/onboarding/internal/model.ts +++ b/packages/app/src/onboarding/internal/model.ts @@ -34,6 +34,8 @@ export type OnboardingState = serverId: string; steps: readonly OnboardingStepId[]; stepIndex: number; + /** Identifies this stay on the current step; it changes on every move, close or reopen. */ + visit: number; }; export interface OnboardingModel { @@ -43,8 +45,11 @@ export interface OnboardingModel { subscribe(listener: () => void): () => void; applySetupHost(host: OnboardingSetupHost | null, options: OnboardingStepOptions): void; reopen(serverId: string, options: OnboardingStepOptions): void; - /** Leaves `from` for the following step. A no-op once the user is on another step or closed setup. */ - next(from: OnboardingStepId): void; + /** + * Moves on from the step visit the caller saw. A no-op once that visit ended, so an action that + * finishes late never moves a step the user left or a setup they closed and reopened. + */ + next(visit: number): void; back(): void; goTo(step: OnboardingStepId): void; /** Closes setup, whether the user finished or skipped it, and remembers that on this client. */ @@ -62,15 +67,6 @@ function isReadyForSetup(host: OnboardingSetupHost | null): host is OnboardingSe return host !== null && host.isConnected && host.hasHydratedWorkspaces; } -function openState(serverId: string, options: OnboardingStepOptions): OnboardingState { - return { - status: "open", - serverId, - steps: options.includePhoneStep ? STEPS_WITH_PHONE : STEPS_WITHOUT_PHONE, - stepIndex: 0, - }; -} - export function createOnboardingModel(input: { storage: OnboardingCompletionStorage; }): OnboardingModel { @@ -78,6 +74,18 @@ export function createOnboardingModel(input: { let state: OnboardingState = { status: "deciding" }; let completion: Completion = "loading"; let pendingHost: PendingHost | null = null; + let visits = 0; + + function openState(serverId: string, options: OnboardingStepOptions): OnboardingState { + visits += 1; + return { + status: "open", + serverId, + steps: options.includePhoneStep ? STEPS_WITH_PHONE : STEPS_WITHOUT_PHONE, + stepIndex: 0, + visit: visits, + }; + } function setState(next: OnboardingState) { state = next; @@ -105,7 +113,8 @@ export function createOnboardingModel(input: { if (state.status !== "open") return; const clamped = Math.max(0, Math.min(stepIndex, state.steps.length - 1)); if (clamped === state.stepIndex) return; - setState({ ...state, stepIndex: clamped }); + visits += 1; + setState({ ...state, stepIndex: clamped, visit: visits }); } async function loadCompletion(): Promise { @@ -136,10 +145,8 @@ export function createOnboardingModel(input: { reopen(serverId, options) { setState(openState(serverId, options)); }, - next(from) { - if (state.status === "open" && state.steps[state.stepIndex] === from) { - moveTo(state.stepIndex + 1); - } + next(visit) { + if (state.status === "open" && state.visit === visit) moveTo(state.stepIndex + 1); }, back() { if (state.status === "open") moveTo(state.stepIndex - 1); diff --git a/packages/app/src/onboarding/internal/onboarding-modal.tsx b/packages/app/src/onboarding/internal/onboarding-modal.tsx index 793235e24e..f292fda2c0 100644 --- a/packages/app/src/onboarding/internal/onboarding-modal.tsx +++ b/packages/app/src/onboarding/internal/onboarding-modal.tsx @@ -34,12 +34,13 @@ export function OnboardingModal({ model, state }: { model: OnboardingModel; stat void model.complete(); }, [model]); const handleBack = useCallback(() => model.back(), [model]); - const handleNext = useCallback(() => model.next(step), [model, step]); + const { visit } = state; + const handleNext = useCallback(() => model.next(visit), [model, visit]); const handleAddSuggestions = useCallback(() => { void (async () => { - if (await addChecked()) model.next("projects"); + if (await addChecked()) model.next(visit); })(); - }, [addChecked, model]); + }, [addChecked, model, visit]); const primary = usePrimaryAction({ step, isLast, diff --git a/packages/app/src/project-suggestions/internal/project-suggestions.tsx b/packages/app/src/project-suggestions/internal/project-suggestions.tsx index 18cbcb03d0..a98b070d69 100644 --- a/packages/app/src/project-suggestions/internal/project-suggestions.tsx +++ b/packages/app/src/project-suggestions/internal/project-suggestions.tsx @@ -47,6 +47,7 @@ export function ProjectSuggestionList({ row={row} first={index === 0} providerLabels={list.providerLabels} + disabled={list.isAdding} onToggle={list.toggle} /> ))} @@ -69,17 +70,19 @@ function SuggestionRow({ row, first, providerLabels, + disabled, onToggle, }: { row: SuggestionRowModel; first: boolean; providerLabels: ReadonlyMap; + disabled: boolean; onToggle: (suggestion: ProjectSuggestion) => void; }) { const { t } = useTranslation(); const { suggestion, checked, addFailure } = row; const handlePress = useCallback(() => onToggle(suggestion), [onToggle, suggestion]); - const accessibilityState = useMemo(() => ({ checked }), [checked]); + const accessibilityState = useMemo(() => ({ checked, disabled }), [checked, disabled]); const lastActivity = useTimeAgo(useMemo(() => new Date(suggestion.lastActivityAt), [suggestion])); const providers = suggestion.providerIds.map((id) => providerLabels.get(id) ?? id).join(" ยท "); const rowStyle = useCallback( @@ -95,6 +98,7 @@ function SuggestionRow({ ; reading: ProviderSuggestions[]; failed: ProviderSuggestions[]; + /** True while checked rows are being added; the selection is frozen until it settles. */ + isAdding: boolean; toggle: (suggestion: ProjectSuggestion) => void; retry: (provider: string) => void; }; @@ -100,10 +102,11 @@ export function useProjectSuggestions(serverId: string): ProjectSuggestionsContr ), reading: state.providers.filter((provider) => provider.status === "loading"), failed: state.providers.filter((provider) => provider.status === "failed"), + isAdding: isPending, toggle, retry: state.retry, }; - }, [state, suggestions, checkedRootPaths, addFailures, toggle]); + }, [state, suggestions, checkedRootPaths, addFailures, isPending, toggle]); return { list, checkedCount: checkedRootPaths.length, isAdding: isPending, addChecked }; }