Skip to content

Commit 88ecdfa

Browse files
vdavidvdavid
committed
Onboarding: real step 3 (optional setup)
- Four toggle blocks: networking, drive indexing, automatic updates, MTP — each bound to its existing registry setting via `<SettingSwitch>`. Defaults stay on; step 3 is about giving users a chance to opt out with full context, not asking for opt-in. - Single primary "Start using Cmdr" footer button registered via `setFooterOverride()`; click bumps `finishRequestTick` so the wizard's `onComplete` runs `notifyOnboardingComplete()` and closes the sheet. - Verbatim copy from David's spec (`docs/specs/onboarding-revamp-context.md` § "Step 3 (optional)"). - Live-apply audit results: `network.enabled`, `indexing.enabled`, `fileOperations.mtpEnabled` already wired in `passthroughBackendHandlers`. `updates.autoCheck` was the gap — wizard / Settings flips previously required restart. - M4 wires `updates.autoCheck` end-to-end. New `applyAutoCheckEnabled(enabled)` in `updates/updater.svelte.ts` lifts the poll-loop interval handle to module scope and toggles it in place (off cancels, on restarts + fires one immediate check so users don't wait the full cadence). Settings-applier's `passthroughBackendHandlers` calls it; same handler covers the wizard, the Settings UI switch, and any future MCP/IPC writer. - Tests: `StepOptional.test.ts` (footer registration, click bumps `finishRequestTick`, destroy clears override, all four toggle round-trip into `setSetting`); `StepOptional.a11y.test.ts` (default + one-off-toggle states); two `applyAutoCheckEnabled` cases added to `updater.test.ts` (on fires one check, off fires none). - Docs: `onboarding/CLAUDE.md` § "Step 3 (optional setup)" with the toggle ↔ setting ↔ live-apply table; `settings/CLAUDE.md` notes the new `updates.autoCheck` applier entry; `updates/CLAUDE.md` documents the `startUpdateChecker` opt-out path and the new helper.
1 parent 963b4bf commit 88ecdfa

9 files changed

Lines changed: 532 additions & 39 deletions

File tree

‎apps/desktop/src/lib/onboarding/CLAUDE.md‎

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,14 @@ path.
1414
| `StepAi.svelte` | Step 2: AI provider picker. Three FDA-outcome banners, three radio choices, dual-button footer (Start vs Continue). |
1515
| `CloudProviderPicker.svelte` | Step 2 left column: scrollable listbox of all 15 cloud providers. Arrow / Home / End / type-to-jump keyboard nav. |
1616
| `CloudProviderSetup.svelte` | Step 2 right column: per-provider numbered tutorial with API-key persist + auto-check + model combobox. |
17-
| `StepOptional.svelte` | Step 3 (optional): networking, indexing, updates, MTP toggles. M2 ships a stub; M4 wires the toggles. |
17+
| `StepOptional.svelte` | Step 3 (optional): networking, indexing, updates, MTP toggles bound to existing registry settings. |
1818
| `onboarding-state.svelte.ts` | Wizard state machine: step cursor, step-1 variant, step-1 footer mode, step-2 banner mode, `openWizard()` / `resumeStepFor()` etc. |
1919

2020
## Status
2121

22-
M3-shipping. Step 1 (FDA) and step 2 (AI provider) are real; step 3 is a stub until M4. The legacy
23-
`FullDiskAccessPrompt.svelte` modal is gone — the wizard is the single first-launch path on macOS.
24-
`CMDR_FORCE_ONBOARDING=1` forces the wizard regardless of persisted state for dev / E2E iteration.
22+
M4-shipping. All three steps are real. The legacy `FullDiskAccessPrompt.svelte` modal is gone — the wizard is the single
23+
first-launch path on macOS. `CMDR_FORCE_ONBOARDING=1` forces the wizard regardless of persisted state for dev / E2E
24+
iteration.
2525

2626
## Step 1 (Full Disk Access)
2727

@@ -105,6 +105,29 @@ state. The reason it's there: the listener fires per-setting-change, so if the u
105105
get three async invocations racing the wizard's `onComplete()`. The explicit `await pushConfigToBackend()` in
106106
`StepAi.persist()` orders the backend reconfigure before the user lands in the app deterministically.
107107

108+
## Step 3 (optional setup)
109+
110+
Four toggle blocks, each bound to an existing registry setting via `<SettingSwitch>`. Defaults stay ON; the step is
111+
about letting the user turn things OFF with full context, not about asking for opt-in.
112+
113+
| Toggle | Setting ID | Live-apply wiring |
114+
| ----------------- | --------------------------- | ------------------------------------------------------------------------------------------- |
115+
| Networking | `network.enabled` | `passthroughBackendHandlers` → `setNetworkEnabled` (pre-existing) |
116+
| Drive indexing | `indexing.enabled` | `passthroughBackendHandlers` → `setIndexingEnabled` (pre-existing) |
117+
| Automatic updates | `updates.autoCheck` | `passthroughBackendHandlers` → `applyAutoCheckEnabled` in `updater.svelte.ts` (added in M4) |
118+
| MTP | `fileOperations.mtpEnabled` | `passthroughBackendHandlers` → `setMtpEnabled` (pre-existing) |
119+
120+
Because `<SettingSwitch>` writes via `setSetting()` on every flip, the toggles take effect the moment the user clicks
121+
them — the wizard doesn't need its own persist queue. The footer's single primary button (`Start using Cmdr`, registered
122+
via `setFooterOverride()`) just bumps `finishRequestTick`; the wizard shell's `onComplete` callback then runs
123+
`notifyOnboardingComplete()` (which flips `isOnboarded: true`) and closes the sheet.
124+
125+
`updates.autoCheck` live-apply was the M4 net-new wiring. Before M4 the setting existed in the registry and the UI but
126+
no listener watched it, so flipping it required an app restart. M4 added `applyAutoCheckEnabled(enabled)` to
127+
`updates/updater.svelte.ts` (lifts the poll-loop interval handle to module scope, starts/stops it in place, fires one
128+
immediate `checkForUpdates()` on re-enable so users don't wait the full cadence) plus an entry in
129+
`settings-applier.ts`'s `passthroughBackendHandlers` table so the toggle works from anywhere (wizard, Settings UI, MCP).
130+
108131
## Resume rule
109132

110133
`onboarding-state.svelte.ts::resumeStepFor(ctx)` picks the right step from the persisted flags + an FDA probe:

‎apps/desktop/src/lib/onboarding/StepOptional.a11y.test.ts‎

Lines changed: 66 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,86 @@
11
/**
2-
* Tier 3 a11y test for the `StepOptional.svelte` stub (M2). M4 ships the real toggles
3-
* and expands this coverage to default + one-off-toggle states.
2+
* Tier 3 axe a11y tests for `StepOptional.svelte` (M4).
3+
*
4+
* Two states: default (all four toggles on) and one-off (networking off). Each switch
5+
* is labelled by its registry definition; the section heading gives the question
6+
* context. Axe runs in jsdom (no contrast checks; we cover those in tier-1 scripts).
47
*/
58

6-
import { describe, it, afterEach } from 'vitest'
7-
import { mount, tick, unmount } from 'svelte'
9+
import { describe, it, vi, beforeEach, afterEach } from 'vitest'
10+
import { mount, tick, unmount, flushSync } from 'svelte'
811
import StepOptional from './StepOptional.svelte'
12+
import { closeWizard, resetForTesting, openWizard, setCurrentStep } from './onboarding-state.svelte'
913
import { expectNoA11yViolations } from '$lib/test-a11y'
1014

15+
const settingsMap: Record<string, unknown> = {
16+
'network.enabled': true,
17+
'indexing.enabled': true,
18+
'updates.autoCheck': true,
19+
'fileOperations.mtpEnabled': true,
20+
}
21+
22+
vi.mock('$lib/settings', async (importOriginal) => {
23+
const actual = (await importOriginal()) as Record<string, unknown>
24+
return {
25+
...actual,
26+
getSetting: (id: string) => settingsMap[id],
27+
setSetting: (id: string, value: unknown) => {
28+
settingsMap[id] = value
29+
},
30+
onSpecificSettingChange: () => () => {},
31+
}
32+
})
33+
1134
let mounted: { target: HTMLElement; instance: ReturnType<typeof mount> } | undefined
1235

36+
async function settle(): Promise<void> {
37+
for (let i = 0; i < 10; i++) {
38+
await Promise.resolve()
39+
}
40+
await tick()
41+
flushSync()
42+
}
43+
44+
function mountStep(): HTMLElement {
45+
const target = document.createElement('div')
46+
document.body.appendChild(target)
47+
const instance = mount(StepOptional, { target, props: {} })
48+
mounted = { target, instance }
49+
return target
50+
}
51+
52+
beforeEach(() => {
53+
settingsMap['network.enabled'] = true
54+
settingsMap['indexing.enabled'] = true
55+
settingsMap['updates.autoCheck'] = true
56+
settingsMap['fileOperations.mtpEnabled'] = true
57+
closeWizard()
58+
resetForTesting()
59+
openWizard('force')
60+
setCurrentStep(3)
61+
})
62+
1363
afterEach(() => {
1464
if (mounted) {
1565
unmount(mounted.instance)
1666
mounted.target.remove()
1767
mounted = undefined
1868
}
69+
closeWizard()
70+
resetForTesting()
1971
})
2072

2173
describe('StepOptional a11y', () => {
22-
it('default stub state has no a11y violations', async () => {
23-
const target = document.createElement('div')
24-
document.body.appendChild(target)
25-
const instance = mount(StepOptional, { target, props: {} })
26-
mounted = { target, instance }
27-
await tick()
74+
it('default state (all toggles on) has no a11y violations', async () => {
75+
const target = mountStep()
76+
await settle()
77+
await expectNoA11yViolations(target)
78+
})
79+
80+
it('one-off state (networking off) has no a11y violations', async () => {
81+
settingsMap['network.enabled'] = false
82+
const target = mountStep()
83+
await settle()
2884
await expectNoA11yViolations(target)
2985
})
3086
})

‎apps/desktop/src/lib/onboarding/StepOptional.svelte‎

Lines changed: 175 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,195 @@
11
<script lang="ts">
2+
import { onMount, onDestroy } from 'svelte'
23
import OnboardingStepShell from './OnboardingStepShell.svelte'
4+
import SettingSwitch from '$lib/settings/components/SettingSwitch.svelte'
5+
import { setFooterOverride, requestWizardComplete } from './onboarding-state.svelte'
36
47
/**
5-
* M1 placeholder. M4 wires the four optional toggles (networking, drive indexing,
6-
* automatic updates, MTP) with their long-form descriptions, all bound to existing
7-
* registry settings. See `docs/specs/onboarding-revamp-plan.md` § M4.
8+
* Step 3 — Optional setup.
9+
*
10+
* Four toggles, each bound to an existing registry setting via `<SettingSwitch>`.
11+
* The switch component reads + writes the setting directly, so the toggles
12+
* live-apply the moment the user flips them: `network.enabled` /
13+
* `indexing.enabled` / `updates.autoCheck` / `fileOperations.mtpEnabled` all
14+
* have entries in `settings-applier.ts`'s `passthroughBackendHandlers` table that
15+
* fire the matching Rust-side helper (`updates.autoCheck` got wired in M4; the
16+
* other three were already in place).
17+
*
18+
* Defaults stay ON. Step 3's purpose is to let the user turn things OFF with full
19+
* context, not to ask for opt-in. Verbatim copy from David's spec in
20+
* `docs/specs/onboarding-revamp-context.md` § "Step 3 (optional)".
21+
*
22+
* Footer: single primary "Start using Cmdr" button registered via
23+
* `setFooterOverride()`. Clicking it asks the wizard to finish: the wizard's
24+
* `onComplete` callback persists `isOnboarded: true` (via
25+
* `notifyOnboardingComplete`), drops the suppress-update-toast gate, and closes
26+
* the sheet. No safety-net persist call here: each switch already wrote its
27+
* setting on flip, so there's nothing pending to drain.
828
*/
29+
30+
onMount(() => {
31+
// Footer button has no reactive deps (the click handler closes over module-level
32+
// functions only), so register once on mount rather than re-running an `$effect`.
33+
setFooterOverride([
34+
{
35+
label: 'Start using Cmdr',
36+
variant: 'primary',
37+
onclick: () => {
38+
requestWizardComplete()
39+
},
40+
},
41+
])
42+
})
43+
44+
onDestroy(() => {
45+
// Clear the footer override so other steps' default buttons render again, and
46+
// so any teardown-then-remount (Vitest hot reload, future re-entry) doesn't
47+
// leak stale closures.
48+
setFooterOverride(null)
49+
})
950
</script>
1051

1152
<OnboardingStepShell>
12-
<h2 class="step-title">Step 3 — Optional setup</h2>
13-
<p class="step-placeholder">M4 wires the four optional toggles here.</p>
53+
<h2 class="step-title">You're almost ready</h2>
54+
<p class="lede">
55+
You chose to walk through a detailed setup, so here are a few easy choices. If you don't care too much, just
56+
click the button below. These are all options, and the defaults are picked for your benefit.
57+
</p>
58+
59+
<section class="toggle-block" aria-labelledby="toggle-networking-title">
60+
<header class="toggle-header">
61+
<div class="toggle-text">
62+
<h3 id="toggle-networking-title" class="toggle-title">Networking</h3>
63+
<p class="toggle-desc">
64+
Having this <em>on</em> means you can connect to SMB servers like company network shares, a home
65+
NAS, and the like. The only cost is a macOS permission dialog that pops up and asks you to allow
66+
"Local network access", and one for "Accepting incoming connections". Both dialogs are harmless,
67+
but if you don't know what these are, they might be scary or annoying.
68+
</p>
69+
</div>
70+
<div class="toggle-control">
71+
<SettingSwitch id="network.enabled" />
72+
</div>
73+
</header>
74+
</section>
75+
76+
<section class="toggle-block" aria-labelledby="toggle-indexing-title">
77+
<header class="toggle-header">
78+
<div class="toggle-text">
79+
<h3 id="toggle-indexing-title" class="toggle-title">Drive indexing</h3>
80+
<p class="toggle-desc">
81+
Drive indexing is totally cool. It gives you two main things: instant search of your whole drive
82+
(think Spotlight, but even faster), and real-time folder sizes for your whole drive (you always
83+
know how much stuff you have in each folder). If you turn this off, you only get
84+
<code>&lt;DIR&gt;</code> for the sizes. The cost is a 300 MB index on your drive, but no extra CPU
85+
or memory use after the first two or three minutes of you first starting the app, or starting it
86+
after a long time. It's a cheap feature considering the benefits.
87+
</p>
88+
</div>
89+
<div class="toggle-control">
90+
<SettingSwitch id="indexing.enabled" />
91+
</div>
92+
</header>
93+
</section>
94+
95+
<section class="toggle-block" aria-labelledby="toggle-updates-title">
96+
<header class="toggle-header">
97+
<div class="toggle-text">
98+
<h3 id="toggle-updates-title" class="toggle-title">Automatic updates</h3>
99+
<p class="toggle-desc">
100+
If you enable this, Cmdr makes a tiny network request to a central license server at each app
101+
start plus once every 24 hours, and you always get the latest updates. If disabled, you'll keep
102+
your current version, and zero automated network requests (except for periodic license checks, if
103+
you have a commercial license).
104+
</p>
105+
</div>
106+
<div class="toggle-control">
107+
<SettingSwitch id="updates.autoCheck" />
108+
</div>
109+
</header>
110+
</section>
111+
112+
<section class="toggle-block" aria-labelledby="toggle-mtp-title">
113+
<header class="toggle-header">
114+
<div class="toggle-text">
115+
<h3 id="toggle-mtp-title" class="toggle-title">MTP (Android phones, Kindles, cameras)</h3>
116+
<p class="toggle-desc">
117+
If you enable this, Cmdr can connect to Android phones, Kindles, cameras, some music players, and
118+
any other device that supports the protocols called MTP or PTP. The cost is that macOS also wants
119+
to connect to these (and it usually fails, which is why you can't just use Finder to copy photos
120+
from Android phones), so Cmdr has to suppress that macOS process while it's running. When you quit
121+
Cmdr, this is politely restored. A bit of a cost, but worth it for the connectivity.
122+
</p>
123+
</div>
124+
<div class="toggle-control">
125+
<SettingSwitch id="fileOperations.mtpEnabled" />
126+
</div>
127+
</header>
128+
</section>
14129
</OnboardingStepShell>
15130

16131
<style>
17132
.step-title {
18-
margin: 0 0 var(--spacing-md) 0;
133+
margin: 0 0 var(--spacing-md);
19134
font-size: var(--font-size-lg);
20135
font-weight: 600;
136+
color: var(--color-text-primary);
137+
}
138+
139+
.lede {
140+
margin: 0 0 var(--spacing-lg);
141+
line-height: 1.5;
142+
color: var(--color-text-primary);
143+
}
144+
145+
.toggle-block {
146+
margin-bottom: var(--spacing-md);
147+
padding: var(--spacing-lg);
148+
border: 1px solid var(--color-border);
149+
border-radius: var(--radius-md);
150+
background: var(--color-bg-primary);
151+
}
152+
153+
.toggle-block:last-child {
154+
margin-bottom: 0;
21155
}
22156
23-
.step-placeholder {
157+
.toggle-header {
158+
display: flex;
159+
align-items: flex-start;
160+
gap: var(--spacing-lg);
161+
}
162+
163+
.toggle-text {
164+
flex: 1;
165+
min-width: 0;
166+
}
167+
168+
.toggle-control {
169+
flex-shrink: 0;
170+
padding-top: var(--spacing-xxs);
171+
}
172+
173+
.toggle-title {
174+
margin: 0 0 var(--spacing-xs);
175+
font-size: var(--font-size-md);
176+
font-weight: 600;
177+
color: var(--color-text-primary);
178+
}
179+
180+
.toggle-desc {
24181
margin: 0;
182+
font-size: var(--font-size-sm);
183+
line-height: 1.5;
25184
color: var(--color-text-secondary);
26185
}
186+
187+
.toggle-desc code {
188+
font-family: var(--font-mono);
189+
font-size: var(--font-size-xs);
190+
background: var(--color-bg-tertiary);
191+
padding: var(--spacing-xxs) var(--spacing-xs);
192+
border-radius: var(--radius-sm);
193+
color: var(--color-text-primary);
194+
}
27195
</style>

0 commit comments

Comments
 (0)
⚡