fix(onboarding): bind late project adds to their step visit and freeze rows while adding

- Each step visit has an id; a finished add only advances the visit that started it,
  so leaving and returning to Projects, or reopening setup, ignores it.
- Suggestion rows are disabled while their batch is being added.
This commit is contained in:
Mohamed Boudra
2026-10-31 11:12:23 +01:00
parent 0390379e60
commit 004bb6917d
5 changed files with 68 additions and 36 deletions
@@ -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"]);
+23 -16
View File
@@ -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<void> {
@@ -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);
@@ -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,
@@ -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<string, string>;
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({
<PressHighlight
style={rowStyle}
onPress={handlePress}
disabled={disabled}
accessibilityRole="checkbox"
accessibilityLabel={suggestion.displayName}
accessibilityState={accessibilityState}
@@ -29,6 +29,8 @@ export type SuggestionList =
providerLabels: ReadonlyMap<string, string>;
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 };
}