feat(desktop): allow skipping every onboarding step and the whole flow#4109
Conversation
- Add `skipAll` action and let skipped steps satisfy the dashboard gate - Always-visible "Skip for now" link on providers, permissions, and project steps - Add "Skip onboarding" button to the setup chrome
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a skip-onboarding capability: a store-level ChangesOnboarding Skip Flow
Sequence DiagramsequenceDiagram
participant User as User
participant UI as Onboarding UI
participant Store as OnboardingStore
participant Router as Router
participant Telemetry as Telemetry
User->>UI: Click "Skip onboarding" / "Skip for now"
UI->>Store: call markSkipped(step) or skipAll()
Store->>Store: mark steps skipped, update timestamps
Store->>Telemetry: emit onboarding_finished (skipped_all) and per-step skip events
UI->>Router: navigate(to: nextStep or /welcome, replace:true)
Router-->>User: show destination screen
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR adds per-step "Skip for now" links to the providers, permissions, and project onboarding steps, adds a global "Skip onboarding" button in the setup chrome, and relaxes the dashboard gate so that skipping a required step (
Confidence Score: 4/5Safe to merge; the skip paths work correctly and the gate change is intentional. The main concern is the 'Skip onboarding' button in the chrome having no confirmation step, making it easy to accidentally skip the entire flow. The logic in skipAll(), the per-step skip handlers, and the updated dashboard gate all behave as intended. The analytics outcome label in skipAll() will say 'skipped_all' even for users who completed some steps first, which could distort funnel metrics. The 'Skip onboarding' chrome button is permanently visible with no confirmation, so a single misclick irreversibly skips everything until the user manually resets from Settings. onboardingStore.ts (analytics outcome accuracy) and OnboardingProgress.tsx (no guard on the global skip button)
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/stores/onboarding/onboardingStore.ts | Adds skipAll() action and relaxes selectRequiredStepsComplete to accept skipped required steps; the analytics outcome label may be inaccurate when some steps were already completed before skipAll is invoked |
| apps/desktop/src/renderer/routes/_authenticated/setup/components/OnboardingProgress/OnboardingProgress.tsx | Adds 'Skip onboarding' button in the chrome bar; button is always visible and fires skipAll() then replaces history with /welcome — no confirmation guard |
| apps/desktop/src/renderer/routes/_authenticated/setup/permissions/page.tsx | Adds 'Skip for now' link that calls markSkipped('permissions') and advances to the project step; permissions is a non-required step so this is low risk |
| apps/desktop/src/renderer/routes/_authenticated/setup/project/page.tsx | Adds 'Skip for now' to both the hasProjects and no-projects branches of the project step; correctly calls markSkipped('project') before navigating to adopt-worktrees |
| apps/desktop/src/renderer/routes/_authenticated/setup/providers/page.tsx | Removes conditional rendering of the skip/continue link so it is always visible; three-branch click handler correctly routes to cancel, complete, or skip depending on connection state |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User enters onboarding] --> B[providers step]
B --> C{Action}
C -->|Complete| D[markComplete providers]
C -->|Skip for now - NEW| E[markSkipped providers]
C -->|Skip onboarding - NEW| SK[skipAll]
D --> F[gh-cli step]
E --> F
F --> G[permissions step]
G --> H{Action}
H -->|Complete| I[markComplete permissions]
H -->|Skip for now - NEW| J[markSkipped permissions]
H -->|Skip onboarding - NEW| SK
I --> K[project step]
J --> K
K --> L{Action}
L -->|Complete| M[markComplete project]
L -->|Skip for now - NEW| N[markSkipped project]
L -->|Skip onboarding - NEW| SK
M --> O[adopt-worktrees step]
N --> O
SK -->|sets completedAt + all skipped| W[welcome page]
O --> P[Onboarding done]
P --> W
subgraph Gate[Dashboard Gate CHANGED]
Q[selectRequiredStepsComplete\nproviders AND project\nmust be completed OR skipped]
end
W --> Gate
Gate -->|pass| R[Dashboard renders]
Gate -->|fail| B
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
apps/desktop/src/renderer/routes/_authenticated/setup/components/OnboardingProgress/OnboardingProgress.tsx:110-119
**No confirmation on destructive action**
"Skip onboarding" marks every remaining step as skipped and sets `completedAt` — it's irreversible without going to Settings and resetting. A single accidental click during onboarding (e.g., fat-finger instead of Back) silently bypasses the whole flow. The other skip affordances ("Skip for now") are step-scoped and easy to undo by re-entering the step; this one is not. A brief confirmation popover or at minimum a `window.confirm` would prevent accidental invocations.
### Issue 2 of 2
apps/desktop/src/renderer/stores/onboarding/onboardingStore.ts:125-128
**`"skipped_all"` outcome is inaccurate when some steps were already completed**
The `outcome` is hardcoded to `"skipped_all"` even when the user had previously completed some steps before clicking "Skip onboarding". A user who finishes `providers` and `gh-cli` and then hits Skip would emit `skipped_all`, despite having done real configuration work — making funnel analysis misleading. Consider computing `anyCompleted = ONBOARDING_STEP_ORDER.some((s) => prev.completed[s])` and emitting `outcome: anyCompleted ? "partial_skip" : "skipped_all"` instead.
Reviews (1): Last reviewed commit: "feat(desktop): allow skipping every onbo..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| <button | ||
| type="button" | ||
| onClick={handleSkipAll} | ||
| className={cn( | ||
| pillBase, | ||
| "border-transparent text-[#a8a5a3] hover:bg-white/5 hover:text-[#eae8e6]", | ||
| )} | ||
| > | ||
| Skip onboarding | ||
| </button> |
There was a problem hiding this comment.
"Skip onboarding" marks every remaining step as skipped and sets completedAt — it's irreversible without going to Settings and resetting. A single accidental click during onboarding (e.g., fat-finger instead of Back) silently bypasses the whole flow. The other skip affordances ("Skip for now") are step-scoped and easy to undo by re-entering the step; this one is not. A brief confirmation popover or at minimum a window.confirm would prevent accidental invocations.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/setup/components/OnboardingProgress/OnboardingProgress.tsx
Line: 110-119
Comment:
**No confirmation on destructive action**
"Skip onboarding" marks every remaining step as skipped and sets `completedAt` — it's irreversible without going to Settings and resetting. A single accidental click during onboarding (e.g., fat-finger instead of Back) silently bypasses the whole flow. The other skip affordances ("Skip for now") are step-scoped and easy to undo by re-entering the step; this one is not. A brief confirmation popover or at minimum a `window.confirm` would prevent accidental invocations.
How can I resolve this? If you propose a fix, please make it concise.| track("onboarding_finished", { | ||
| outcome: "skipped_all", | ||
| duration_ms: prev.startedAt ? Date.now() - prev.startedAt : null, | ||
| }); |
There was a problem hiding this comment.
"skipped_all" outcome is inaccurate when some steps were already completed
The outcome is hardcoded to "skipped_all" even when the user had previously completed some steps before clicking "Skip onboarding". A user who finishes providers and gh-cli and then hits Skip would emit skipped_all, despite having done real configuration work — making funnel analysis misleading. Consider computing anyCompleted = ONBOARDING_STEP_ORDER.some((s) => prev.completed[s]) and emitting outcome: anyCompleted ? "partial_skip" : "skipped_all" instead.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/stores/onboarding/onboardingStore.ts
Line: 125-128
Comment:
**`"skipped_all"` outcome is inaccurate when some steps were already completed**
The `outcome` is hardcoded to `"skipped_all"` even when the user had previously completed some steps before clicking "Skip onboarding". A user who finishes `providers` and `gh-cli` and then hits Skip would emit `skipped_all`, despite having done real configuration work — making funnel analysis misleading. Consider computing `anyCompleted = ONBOARDING_STEP_ORDER.some((s) => prev.completed[s])` and emitting `outcome: anyCompleted ? "partial_skip" : "skipped_all"` instead.
How can I resolve this? If you propose a fix, please make it concise.![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/routes/_authenticated/setup/permissions/page.tsx (1)
105-112:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win"Skip for now" is available even when required permissions are already granted
When
requiredSatisfiedis true, both "Continue" (which callsmarkComplete) and "Skip for now" (which callsmarkSkipped) are active simultaneously. A user who accidentally clicks "Skip for now" after granting all permissions will record the step as skipped rather than completed, resulting in inaccurate analytics and an inconsistent state where granted permissions are recorded as skipped.Consider disabling or hiding the skip option once
requiredSatisfiedis true, or at minimum callingmarkCompleteinstead ofmarkSkippedinsidehandleSkipwhenrequiredSatisfied.🛡️ Minimal fix — promote skip to complete when satisfied
const handleSkip = () => { - markSkipped("permissions"); + if (requiredSatisfied) { + markComplete("permissions"); + } else { + markSkipped("permissions"); + } navigate({ to: STEP_ROUTES.project }); };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/routes/_authenticated/setup/permissions/page.tsx` around lines 105 - 112, The "Skip for now" action can record the step as skipped even when requiredSatisfied is true; update the UI/handler so users can't accidentally mark a completed step as skipped: either disable or hide the skip SetupButton when requiredSatisfied is true, or change handleSkip to check requiredSatisfied and call markComplete instead of markSkipped (or delegate to handleContinue) when requiredSatisfied is true; locate the SetupButton for "Skip for now" and the handleSkip/handleContinue functions to implement the conditional behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/setup/components/OnboardingProgress/OnboardingProgress.tsx`:
- Around line 109-120: The "Skip onboarding" button currently calls
handleSkipAll which unconditionally invokes skipAll and emits
onboarding_finished with outcome "skipped_all"; change handleSkipAll in
OnboardingProgress.tsx to first check whether all steps are already completed
(e.g., using the same store/selector that marks steps as completed or
steps.every(s => s.completedAt)) and return early if so, and/or disable the
button when allStepsCompleted is true to prevent clicks; this ensures skipAll is
only called when there are incomplete steps and prevents emitting a misleading
onboarding_finished/skipped_all event or mutating the store unnecessarily.
In `@apps/desktop/src/renderer/routes/_authenticated/setup/providers/page.tsx`:
- Around line 261-280: The button label is misleading when atLeastOneConnected
&& !isReconfiguring: it says "Skip — continue to next step" while the click
handler calls handleContinueToNextStep() (which marks the step complete); update
the label logic inside the SetupButton render (the ternary that references
isReconfiguring and atLeastOneConnected) so that when atLeastOneConnected and
not isReconfiguring the text reads "Continue to next step" (or equivalent that
does not use "Skip"), leaving the onClick handlers (handleContinueToNextStep,
handleSkipStep) unchanged; locate the button/ternary near SetupButton,
isReconfiguring, atLeastOneConnected, handleContinueToNextStep, and
handleSkipStep to make the change.
---
Outside diff comments:
In `@apps/desktop/src/renderer/routes/_authenticated/setup/permissions/page.tsx`:
- Around line 105-112: The "Skip for now" action can record the step as skipped
even when requiredSatisfied is true; update the UI/handler so users can't
accidentally mark a completed step as skipped: either disable or hide the skip
SetupButton when requiredSatisfied is true, or change handleSkip to check
requiredSatisfied and call markComplete instead of markSkipped (or delegate to
handleContinue) when requiredSatisfied is true; locate the SetupButton for "Skip
for now" and the handleSkip/handleContinue functions to implement the
conditional behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2938a147-ef48-4405-8930-4c2c1dd61a47
📒 Files selected for processing (5)
apps/desktop/src/renderer/routes/_authenticated/setup/components/OnboardingProgress/OnboardingProgress.tsxapps/desktop/src/renderer/routes/_authenticated/setup/permissions/page.tsxapps/desktop/src/renderer/routes/_authenticated/setup/project/page.tsxapps/desktop/src/renderer/routes/_authenticated/setup/providers/page.tsxapps/desktop/src/renderer/stores/onboarding/onboardingStore.ts
| <div className="flex items-center justify-end"> | ||
| <button | ||
| type="button" | ||
| onClick={handleSkipAll} | ||
| className={cn( | ||
| pillBase, | ||
| "border-transparent text-[#a8a5a3] hover:bg-white/5 hover:text-[#eae8e6]", | ||
| )} | ||
| > | ||
| Skip onboarding | ||
| </button> | ||
| </div> |
There was a problem hiding this comment.
"Skip onboarding" button remains active even when all steps are already completed
handleSkipAll calls skipAll() unconditionally. When all steps are already completed, skipAll() still fires onboarding_finished with outcome: "skipped_all" (since completedAt is reset to Date.now() and manualWalkthrough is cleared), which produces a misleading analytics event and mutates the store unnecessarily.
🛡️ Proposed guard
+ const allStepsDone = useOnboardingStore((s) =>
+ ONBOARDING_STEP_ORDER.every((step) => s.completed[step] || s.skipped[step]),
+ );
+
const handleSkipAll = () => {
skipAll();
navigate({ to: "/welcome", replace: true });
}; <button
type="button"
onClick={handleSkipAll}
+ disabled={allStepsDone}
className={cn(
pillBase,
"border-transparent text-[`#a8a5a3`] hover:bg-white/5 hover:text-[`#eae8e6`]",
)}
>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/desktop/src/renderer/routes/_authenticated/setup/components/OnboardingProgress/OnboardingProgress.tsx`
around lines 109 - 120, The "Skip onboarding" button currently calls
handleSkipAll which unconditionally invokes skipAll and emits
onboarding_finished with outcome "skipped_all"; change handleSkipAll in
OnboardingProgress.tsx to first check whether all steps are already completed
(e.g., using the same store/selector that marks steps as completed or
steps.every(s => s.completedAt)) and return early if so, and/or disable the
button when allStepsCompleted is true to prevent clicks; this ensures skipAll is
only called when there are incomplete steps and prevents emitting a misleading
onboarding_finished/skipped_all event or mutating the store unnecessarily.
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
#4109) * feat(desktop): allow skipping every onboarding step and the whole flow - Add `skipAll` action and let skipped steps satisfy the dashboard gate - Always-visible "Skip for now" link on providers, permissions, and project steps - Add "Skip onboarding" button to the setup chrome * fix(desktop): drop misleading "Skip" prefix when providers step is complete
1 participant
Summary
/welcome.providers,project) count as satisfied — previously skipping wasn't enough to leave the flow.Test plan
/welcomeand doesn't bounce back into setupSummary by cubic
Adds skip controls to onboarding so users can bypass any step or the entire flow. Skipped required steps now unlock the dashboard and no longer bounce users back into setup.
New Features
providers,permissions, andprojectsteps; marks the step as skipped and moves to the next.skipAll(), records analytics, and navigates to/welcome.providers,project) as satisfied.Bug Fixes
providersstep, the secondary action now reads “Continue to next step” when a provider is connected (removes the misleading “Skip” prefix).Written for commit 02b0d67. Summary will update on new commits.
Summary by CodeRabbit