fix: improve onboarding flow navigation from step 1 to step 3 - #137
Conversation
|
@Smrithi-krishna is attempting to deploy a commit to the itzzavdhesh's projects Team on Vercel. A member of the Team first needs to authorize it. |
DCO Sign-off NeededHi @Smrithi-krishna, one or more commits need a Fix these commits:
For the latest commit: git commit --amend --signoff
git push --force-with-leaseFor multiple commits, replace git rebase --signoff HEAD~N
git push --force-with-leaseThis comment will update after you push. VoiceForge automation |
📝 WalkthroughWalkthroughOnboarding.jsx now uses persisted explicit steps ( ChangesPersisted 3-step onboarding
Sequence DiagramssequenceDiagram
actor User
participant UI as Onboarding.jsx
participant CloneHook as useVoiceClone / cloneVoice
participant Storage as localStorage
User->>UI: Open onboarding page
UI->>Storage: read activeStep, maxUnlockedStep
User->>UI: Record sample (VoiceRecorder)
User->>UI: Click "Clone voice"
UI->>CloneHook: handleClone(recording, voiceName) [requires API key + recording]
alt API key + recording present
CloneHook->>CloneHook: call cloneVoice(recording, voiceName)
CloneHook-->>UI: returns successProfile
UI->>Storage: persist maxUnlockedStep=2, activeStep=2
else missing API key or recording or clone fails
CloneHook-->>UI: returns apiError
end
User->>UI: Click step dot
UI->>UI: handleManualStepNavigation (checks maxUnlockedStep)
User->>UI: Finish Step 3
UI->>User: call onReady()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
PR Ready For Mentor ReviewHi @Smrithi-krishna, your PR passed automated validation and moved into the GSSoC review queue.
@sabeenaviklar @1754riya @Anushreebasics @itsdakshjain, this GSSoC PR is ready for review. Mentor next steps: confirm scope, review behavior and tests, then approve, comment, or request changes. This does not mean the PR is approved yet. Please wait for mentor feedback before expecting merge. If changes are requested, update the same branch and keep the PR focused on the linked issue. VoiceForge automation |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@client/src/pages/Onboarding.jsx`:
- Around line 91-93: The auto-advance setTimeout that calls setActiveStep(2)
must be tracked and cleared to avoid jumping back when users navigate manually:
store the timer id in a ref (e.g., autoAdvanceTimerRef), assign the result of
setTimeout to it where you schedule the advance,
clearTimeout(autoAdvanceTimerRef.current) inside any manual navigation handler
(e.g., the "Continue Now" click handler) and in a useEffect cleanup on unmount;
apply the same pattern to the other timer occurrences referenced around lines
228-235 and 256-259 so all delayed advances are cancellable.
- Around line 81-86: The onboarding currently treats missing API key/recording
or cloneVoice failures as success by creating a mock profile; instead only allow
progress when cloneVoice actually succeeds. Update the logic in Onboarding.jsx
(places around the blocks using hasApiKey, recording, cloneVoice, profile,
voiceName — e.g., the if at lines ~81 and the similar blocks at ~95-101 and
~211-215) to: 1) call cloneVoice only when hasApiKey && recording, 2) await it
and on success set the real profile, 3) catch and surface any thrown error into
an error state (do not swallow), and 4) remove the artificial mock fallback so
Step 2 remains locked unless cloneVoice succeeded. Ensure any step-unlocking
logic reads the real success/error state rather than assuming a mock profile.
- Around line 169-179: The step-dot <button> elements are unlabeled and don't
expose the active state to assistive tech; update the button rendering in
Onboarding.jsx (the element using activeStep, stepNum, isAccessible, and
handleStepClick) to include an accessible name like aria-label={`Step ${stepNum
+ 1}`} (or a localized equivalent) and add aria-current="step" when activeStep
=== stepNum; keep aria-disabled or disabled as is for inaccessible steps. Ensure
the aria attributes are applied per-button so screen readers receive both the
label and the current-step state.
- Around line 47-55: Autoplay is advancing the shared activeStep and bypassing
the unlock guard (maxUnlockedStep), so change the effect and related UI to use a
separate preview state (e.g., previewStep) driven by the
React.useEffect/isAutoplayActive interval instead of setActiveStep, and keep
activeStep navigation constrained by maxUnlockedStep; update any header/preview
rendering to read previewStep while click handlers and the real step body still
validate against activeStep and maxUnlockedStep (also audit other uses around
the component, lines ~243-279, to ensure interactive controls always check
maxUnlockedStep before allowing navigation).
- Around line 16-22: activeStep is always initialized to 1 and never restored
from storage; change its initializer to read "voiceforge:onboardingStep" from
localStorage, parse it as an integer, read the stored maxUnlockedStep (or parse
"voiceforge:maxUnlockedStep") and clamp the restored activeStep to
Math.min(savedActive, savedMax) with a fallback of 1, then return that value
from the initializer used by React.useState so activeStep and setActiveStep
start correctly; apply the same restoration/clamping logic to the other
onboarding-step initialization spots in this file and keep referencing
activeStep, setActiveStep, maxUnlockedStep, and setMaxUnlockedStep.
🪄 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 Plus
Run ID: 34699f29-5cbf-4091-af6d-0a5abaf89b7c
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
client/src/pages/Onboarding.jsx
2f0bc5e to
81af6c3
Compare
There was a problem hiding this comment.
4 issues found across 2 files
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Fix all with cubic | Re-trigger cubic
|
Hi, it’s ready for review. Could you please take a look when you have time? |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@client/src/pages/Onboarding.jsx`:
- Around line 14-15: The code is checking the hasApiKey function object instead
of its boolean result; replace usages of the bare identifier hasApiKey with the
computed boolean apiKeyPresent (or call hasApiKey()) wherever gating UI/logic
(e.g., the warning render, clone button disabled state, and in handleClone) is
performed so the missing-API-key warning displays, the Clone button is disabled
when no key exists, and handleClone avoids making API calls without a key;
search for references to hasApiKey in Onboarding.jsx (including the branches
around the initial apiKeyPresent assignment and the logic near handleClone) and
update them to use apiKeyPresent or hasApiKey().
🪄 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 Plus
Run ID: bd0077c8-5989-4c9e-8136-c7637a39a42c
📒 Files selected for processing (1)
client/src/pages/Onboarding.jsx
| const apiKeyPresent = hasApiKey(); | ||
|
|
There was a problem hiding this comment.
Use the boolean result of hasApiKey(), not the function object.
These branches are checking hasApiKey instead of apiKeyPresent/hasApiKey(), so the “missing API key” warning never renders, the clone button stays enabled without a key, and handleClone() still attempts the API call as long as a recording exists.
Suggested fix
const { cloneVoice, status, error: apiError } = useVoiceClone();
const isCloning = status === "cloning";
const apiKeyPresent = hasApiKey();
@@
async function handleClone() {
// 1. Strict validation guards: Don't run without API key or a recorded sample
- if (!hasApiKey || !recording) return;
+ if (!apiKeyPresent || !recording) return;
@@
- {!hasApiKey && (
+ {!apiKeyPresent && (
@@
<button
type="button"
onClick={handleClone}
- disabled={isCloning || !hasApiKey || !recording}
+ disabled={isCloning || !apiKeyPresent || !recording}
className="inline-flex min-h-11 items-center justify-center gap-2 rounded-md bg-coral px-5 font-bold text-white transition hover:bg-coral/90 disabled:cursor-not-allowed disabled:opacity-50"
>Also applies to: 63-66, 159-160, 183-186
🤖 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 `@client/src/pages/Onboarding.jsx` around lines 14 - 15, The code is checking
the hasApiKey function object instead of its boolean result; replace usages of
the bare identifier hasApiKey with the computed boolean apiKeyPresent (or call
hasApiKey()) wherever gating UI/logic (e.g., the warning render, clone button
disabled state, and in handleClone) is performed so the missing-API-key warning
displays, the Clone button is disabled when no key exists, and handleClone
avoids making API calls without a key; search for references to hasApiKey in
Onboarding.jsx (including the branches around the initial apiKeyPresent
assignment and the logic near handleClone) and update them to use apiKeyPresent
or hasApiKey().
PR Merged SuccessfullyHi @Smrithi-krishna, thank you for your contribution to VoiceForge. This pull request has been merged and marked complete. Linked issue(s): #129. Maintainers may still handle final cleanup, release notes, or follow-up tracking after merge. VoiceForge automation |
🚀 Program
GSSoC
📝 Description
This PR enhances the Onboarding.jsx experience by adding an auto-playing onboarding preview slider in the header banner.
When the onboarding screen loads, the header automatically cycles through Step 1 → Step 2 → Step 3 at a fixed interval (5 seconds) to give users a quick preview of the onboarding flow.
To ensure a smooth user experience, the autoplay is disabled immediately upon user interaction, allowing users to take full control of the onboarding process.
User interactions that stop autoplay include:
🔗 Related Issue
Closes #129
🔄 Type of Change
🧪 How to Test
http://localhost:5173in a browser📸 Screenshots (if applicable)
Recording.2026-06-06.144622.mp4
✅ Checklist
feat: add voice preview)Summary by cubic
Refactors onboarding into a gated, persistent 3-step flow with step-aware headers, accessible step dots, and clearer progress. On clone success, navigation now reliably advances to Step 2; users can proceed to Step 3 and finish setup.
New Features
activeStep/maxUnlockedStepinlocalStorage, clamp to unlocked max, and restore on load; restrict manual navigation to unlocked steps.Bug Fixes
Written for commit cc0fc2d. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes