feat(onboarding): redesign WelcomeStep as auto-advancing carousel#427
Conversation
Replace the static welcome screen with a 3-slide auto-rotating carousel (Welcome, Integrations, Automation) that cycles every 5 seconds. The "Let Start" button advances to the next onboarding step as before. - WelcomeStep now contains internal carousel with dot pagination - ProgressIndicator changed from bar segments to dot indicators - Removed top-level ProgressIndicator from Onboarding.tsx - Slides 2 and 3 have image placeholders for future visuals
|
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)
📝 WalkthroughWalkthroughWelcomeStep now renders a 3-slide, auto-advancing carousel with its own dot-style ProgressIndicator. The global onboarding layout no longer mounts the progress UI. ProgressIndicator active logic and visuals changed to single-step dots. Onboarding completion now navigates to /conversations. Changes
Sequence DiagramsequenceDiagram
participant User
participant WelcomeStep
participant Timer as Auto-Advance Timer
participant Carousel
participant ProgressIndicator
User->>WelcomeStep: Mount
activate WelcomeStep
WelcomeStep->>Timer: start setInterval(AUTO_ADVANCE_MS)
WelcomeStep->>Carousel: render slide(0)
WelcomeStep->>ProgressIndicator: set currentStep=0
rect rgba(100,150,200,0.5)
Note over Timer,WelcomeStep: Auto-advance cycle
Timer->>WelcomeStep: interval tick
WelcomeStep->>Carousel: increment slide (wrap)
WelcomeStep->>ProgressIndicator: update currentStep
Carousel->>Carousel: re-render active slide
end
rect rgba(200,150,100,0.5)
Note over User,WelcomeStep: Manual navigation
User->>WelcomeStep: click Next
WelcomeStep->>WelcomeStep: call onNext() / advance onboarding
end
deactivate WelcomeStep
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/pages/onboarding/steps/WelcomeStep.tsx`:
- Around line 67-84: The carousel in WelcomeStep auto-advances via setInterval
but has no manual controls or pause behavior; update WelcomeStep to add
Prev/Next buttons and make ProgressIndicator clickable to setSlide(index), and
modify the auto-advance effect to pause when the component is hovered or
focused, resume on blur/mouseleave, and respect prefers-reduced-motion by
disabling auto-advance when reduced motion is requested; implement this by
tracking a boolean (e.g., isPaused) updated on onMouseEnter/onMouseLeave and
onFocus/onBlur and by checking window.matchMedia('(prefers-reduced-motion:
reduce)') before starting the interval, and ensure to clear and restart the
interval whenever isPaused or slide changes so the interval is always managed
correctly.
- Around line 31-33: The heading in WelcomeStep's JSX uses a literal newline
token "{'\n'}" which browsers collapse and won't create a visible break; update
the H1 element in the WelcomeStep component (the <h1 className="text-2xl
font-bold font-display text-stone-900 mb-3">) to use an explicit <br /> tag
where you want the line break after "without" so the line renders as intended.
🪄 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: c9a9a0b0-44e3-4943-b904-b33ce3cc9145
📒 Files selected for processing (3)
app/src/components/ProgressIndicator.tsxapp/src/pages/onboarding/Onboarding.tsxapp/src/pages/onboarding/steps/WelcomeStep.tsx
Replace placeholder divs with actual images for onboarding carousel slides 2 (manage work / integrations) and 3 (automate it all / tasks).
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
app/src/pages/onboarding/steps/WelcomeStep.tsx (2)
31-33:⚠️ Potential issue | 🟡 MinorUse
<br />instead of{'\\n'}for the heading break.At Line 32,
{'\\n'}is collapsed by HTML whitespace rules, so the intended line break won’t render.Suggested fix
<h1 className="text-2xl font-bold font-display text-stone-900 mb-3"> - Manage work without{'\n'}switching apps + Manage work without + <br /> + switching apps </h1>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/pages/onboarding/steps/WelcomeStep.tsx` around lines 31 - 33, The heading in the WelcomeStep component uses a literal newline token "{'\n'}" which will be collapsed by HTML; replace that token with an explicit line break element <br /> inside the <h1> (the h1 in WelcomeStep.tsx with className "text-2xl font-bold font-display text-stone-900 mb-3") so the break renders as intended.
69-74:⚠️ Potential issue | 🟠 MajorCarousel still needs an operable pause/manual navigation path.
At Line 69 the carousel auto-advances every 5s, and at Lines 83-85 the dots are visual-only. Users currently can’t pause/revisit slides before rotation.
Suggested direction
const WelcomeStep = ({ onNext }: WelcomeStepProps) => { const [slide, setSlide] = useState(0); + const [isPaused, setIsPaused] = useState(false); useEffect(() => { + if (isPaused || window.matchMedia('(prefers-reduced-motion: reduce)').matches) { + return; + } const timer = setInterval(() => { setSlide(prev => (prev + 1) % TOTAL_SLIDES); }, AUTO_ADVANCE_MS); return () => clearInterval(timer); - }, []); + }, [isPaused]); return ( - <div className="rounded-2xl bg-white p-10 shadow-soft animate-fade-up"> + <div + className="rounded-2xl bg-white p-10 shadow-soft animate-fade-up" + onMouseEnter={() => setIsPaused(true)} + onMouseLeave={() => setIsPaused(false)} + onFocusCapture={() => setIsPaused(true)} + onBlurCapture={() => setIsPaused(false)} + > <div className="h-[340px] flex flex-col items-center justify-center"> {slide === 0 && <WelcomeSlide />} {slide === 1 && <IntegrationsSlide />} {slide === 2 && <AutomationSlide />} </div> + <div className="mt-4 flex justify-center gap-2"> + <button type="button" onClick={() => setSlide(prev => (prev + TOTAL_SLIDES - 1) % TOTAL_SLIDES)}>Prev</button> + <button type="button" onClick={() => setSlide(prev => (prev + 1) % TOTAL_SLIDES)}>Next</button> + </div> <div className="mt-8 mb-6 flex justify-center"> <ProgressIndicator currentStep={slide} totalSteps={TOTAL_SLIDES} /> </div>Also applies to: 83-85
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/pages/onboarding/steps/WelcomeStep.tsx` around lines 69 - 74, The carousel auto-advances in the useEffect timer (AUTO_ADVANCE_MS increments via setSlide and TOTAL_SLIDES) but the dots are visual-only; update the component so users can pause and manually navigate: make the dots clickable/keyboard-focusable buttons that call setSlide(index) and stop the auto-advance timer, add Prev/Next controls that call setSlide(prev => (prev +/- 1 + TOTAL_SLIDES) % TOTAL_SLIDES), and implement pause/resume behavior (e.g., pause on mouseEnter/focus and resume on mouseLeave/blur by clearing and recreating the interval used in the useEffect). Ensure the dot elements get proper aria/current or aria-label attributes and keyboard handlers so navigation is accessible.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/pages/onboarding/steps/WelcomeStep.tsx`:
- Around line 35-37: Update the slide copy in the WelcomeStep component so the
paragraph text uses correct capitalization: replace "single Place" with "single
place" in the string rendered (look for the paragraph inside WelcomeStep.tsx
that contains "Handle GitHub reviews, track Notion tasks, check Slack messages,
manage your community and many more — all from a single Place" and adjust the
word "Place" to lowercase).
---
Duplicate comments:
In `@app/src/pages/onboarding/steps/WelcomeStep.tsx`:
- Around line 31-33: The heading in the WelcomeStep component uses a literal
newline token "{'\n'}" which will be collapsed by HTML; replace that token with
an explicit line break element <br /> inside the <h1> (the h1 in WelcomeStep.tsx
with className "text-2xl font-bold font-display text-stone-900 mb-3") so the
break renders as intended.
- Around line 69-74: The carousel auto-advances in the useEffect timer
(AUTO_ADVANCE_MS increments via setSlide and TOTAL_SLIDES) but the dots are
visual-only; update the component so users can pause and manually navigate: make
the dots clickable/keyboard-focusable buttons that call setSlide(index) and stop
the auto-advance timer, add Prev/Next controls that call setSlide(prev => (prev
+/- 1 + TOTAL_SLIDES) % TOTAL_SLIDES), and implement pause/resume behavior
(e.g., pause on mouseEnter/focus and resume on mouseLeave/blur by clearing and
recreating the interval used in the useEffect). Ensure the dot elements get
proper aria/current or aria-label attributes and keyboard handlers so navigation
is accessible.
🪄 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: 34b60164-df81-43ee-859a-4209d020402d
⛔ Files ignored due to path filters (2)
app/public/onboarding-automate-all.pngis excluded by!**/*.pngapp/public/onboarding-manage-work.pngis excluded by!**/*.png
📒 Files selected for processing (1)
app/src/pages/onboarding/steps/WelcomeStep.tsx
Redirect user to /conversations after onboarding completes or is skipped, so they land directly on the chat page.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/components/OnboardingOverlay.tsx (1)
42-49:⚠️ Potential issue | 🟠 MajorNavigation executes even when persistence fails, leaving the overlay in a re-showing loop.
If
setOnboardingCompletedFlagthrows (e.g., Tauri call fails), the catch logs a warning butcommitStatenever runs, sosnapshot.onboardingCompletedstaysfalse. The unconditionalnavigate('/conversations')then fires, but on re-rendershouldShowevaluates totrueand the overlay reappears—trapping the user in a loop.Move the navigation inside the
tryblock so it only fires on successful persistence:Proposed fix
const handleDone = useCallback(async () => { try { await setOnboardingCompletedFlag(true); + navigate('/conversations'); } catch { console.warn('[onboarding] Failed to persist onboarding_completed'); } - navigate('/conversations'); }, [setOnboardingCompletedFlag, navigate]);This ensures users only leave the overlay once the flag is successfully persisted, preventing the re-show loop. If persistence fails, they remain on the overlay and can retry.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/OnboardingOverlay.tsx` around lines 42 - 49, The handleDone handler currently calls navigate('/conversations') unconditionally, so if setOnboardingCompletedFlag throws the onboarding flag never persists and the overlay re-shows; fix by moving the navigate('/conversations') call inside the try block immediately after await setOnboardingCompletedFlag(true) so navigation only occurs on successful persistence, and keep the catch to log/handle the error (but do not navigate there). Ensure this change is applied to the handleDone function and retains the existing dependency references to setOnboardingCompletedFlag and navigate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@app/src/components/OnboardingOverlay.tsx`:
- Around line 42-49: The handleDone handler currently calls
navigate('/conversations') unconditionally, so if setOnboardingCompletedFlag
throws the onboarding flag never persists and the overlay re-shows; fix by
moving the navigate('/conversations') call inside the try block immediately
after await setOnboardingCompletedFlag(true) so navigation only occurs on
successful persistence, and keep the catch to log/handle the error (but do not
navigate there). Ensure this change is applied to the handleDone function and
retains the existing dependency references to setOnboardingCompletedFlag and
navigate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2838a027-014c-49ba-8073-0a0d6f52a8ba
📒 Files selected for processing (1)
app/src/components/OnboardingOverlay.tsx
The useNavigate hook added in the previous commit requires a Router context. Wrap all test renders in MemoryRouter to fix the failures.
…nyhumansai#427) * feat(onboarding): redesign WelcomeStep as auto-advancing carousel Replace the static welcome screen with a 3-slide auto-rotating carousel (Welcome, Integrations, Automation) that cycles every 5 seconds. The "Let Start" button advances to the next onboarding step as before. - WelcomeStep now contains internal carousel with dot pagination - ProgressIndicator changed from bar segments to dot indicators - Removed top-level ProgressIndicator from Onboarding.tsx - Slides 2 and 3 have image placeholders for future visuals * style: fix prettier formatting in Onboarding.tsx * feat(onboarding): add visuals for integration and automation slides Replace placeholder divs with actual images for onboarding carousel slides 2 (manage work / integrations) and 3 (automate it all / tasks). * feat(onboarding): navigate to conversations page after completion Redirect user to /conversations after onboarding completes or is skipped, so they land directly on the chat page. * fix(test): wrap OnboardingOverlay tests in MemoryRouter The useNavigate hook added in the previous commit requires a Router context. Wrap all test renders in MemoryRouter to fix the failures. * style: fix prettier formatting in OnboardingOverlay test
Summary
Test plan
VITE_DEV_FORCE_ONBOARDING=trueor reset onboarding state)yarn typecheckpassesyarn lint— no new errorsSummary by CodeRabbit
New Features
Updates
Tests