feat(web-ui): onboarding guidance card for new workspaces#489
feat(web-ui): onboarding guidance card for new workspaces#489
Conversation
Show a welcome card explaining the Think→Build→Prove→Ship pipeline when a workspace is initialized but has no tasks yet. Card is dismissible and persists dismiss state per-workspace in localStorage. - Add getOnboardingDismissed/setOnboardingDismissed to workspace-storage.ts - New OnboardingCard component with 4-step pipeline and /prd CTA - Render card in page.tsx when tasksData is loaded and tasks.length === 0 - 7 Jest tests covering all acceptance criteria
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughA client OnboardingCard component and storage helpers were added; the workspace page now conditionally renders the card for empty, initialized workspaces. Tests for the card were added, the component was re-exported from the workspace index, and a small SQLite schema migration was introduced to add Changes
Sequence Diagram(s)sequenceDiagram
participant Page as WorkspacePage
participant Card as OnboardingCard (Client)
participant Storage as localStorage
Page->>Card: render(workspacePath, tasksData)
Card->>Storage: getOnboardingDismissed(workspacePath)
Storage-->>Card: boolean (dismissed?)
alt not dismissed
Card-->>Page: render card (4 steps, CTA -> /prd)
Page->>Card: user clicks Dismiss
Card->>Storage: setOnboardingDismissed(workspacePath)
Storage-->>Card: ack
Card-->>Card: update local state -> hide card
else dismissed
Card-->>Page: render nothing
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
web-ui/src/lib/workspace-storage.ts (1)
85-88: Consider encodingworkspacePathin the localStorage key.Workspace paths may contain special characters (spaces, backslashes on Windows, etc.) that could lead to inconsistent key matching if paths are formatted differently between calls. Using
encodeURIComponentwould ensure consistent key generation.♻️ Optional: Encode workspace path for robustness
export function getOnboardingDismissed(workspacePath: string): boolean { if (typeof window === 'undefined') return false; - return localStorage.getItem(`codeframe_onboarding_dismissed_${workspacePath}`) === 'true'; + return localStorage.getItem(`codeframe_onboarding_dismissed_${encodeURIComponent(workspacePath)}`) === 'true'; }Apply the same pattern to
setOnboardingDismissed:export function setOnboardingDismissed(workspacePath: string): void { if (typeof window === 'undefined') return; - localStorage.setItem(`codeframe_onboarding_dismissed_${workspacePath}`, 'true'); + localStorage.setItem(`codeframe_onboarding_dismissed_${encodeURIComponent(workspacePath)}`, 'true'); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web-ui/src/lib/workspace-storage.ts` around lines 85 - 88, The localStorage key built from workspacePath in getOnboardingDismissed can be inconsistent for paths with special characters; update getOnboardingDismissed to use encodeURIComponent(workspacePath) when constructing the key (`codeframe_onboarding_dismissed_${...}`) and make the identical change in setOnboardingDismissed so both reader and writer use the same encoded key format; keep the existing typeof window === 'undefined' guard and the 'true' string comparison logic unchanged.web-ui/src/components/workspace/OnboardingCard.tsx (2)
33-40: Potential flash of content when card was previously dismissed.Initializing
isDismissedtofalseand then updating it inuseEffectmeans users who previously dismissed the card will briefly see it flash before it disappears on hydration. Consider initializing the state lazily or adding a loading state to prevent this flicker.♻️ Option 1: Use lazy initial state
export function OnboardingCard({ workspacePath }: OnboardingCardProps) { - const [isDismissed, setIsDismissed] = useState(false); - - useEffect(() => { - setIsDismissed(getOnboardingDismissed(workspacePath)); - }, [workspacePath]); + const [isDismissed, setIsDismissed] = useState(() => + getOnboardingDismissed(workspacePath) + ); + + // Re-check if workspacePath changes + useEffect(() => { + setIsDismissed(getOnboardingDismissed(workspacePath)); + }, [workspacePath]); if (isDismissed) return null;This works because
getOnboardingDismissedsafely returnsfalseduring SSR, but will return the actual value during client-side hydration whenwindowis defined.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web-ui/src/components/workspace/OnboardingCard.tsx` around lines 33 - 40, The flash happens because isDismissed starts false then is set in useEffect; instead initialize it lazily and keep the effect to handle workspacePath changes: change useState to useState(() => getOnboardingDismissed(workspacePath)) in OnboardingCard so the correct value is used on first render (prevents flicker), and retain the existing useEffect([workspacePath]) with setIsDismissed(getOnboardingDismissed(workspacePath)) to update when workspacePath changes.
29-31: Consider movingOnboardingCardPropsto the shared types file.Per coding guidelines, TypeScript types should be defined in
web-ui/src/types/index.ts. While this is a simple interface, consistency with the codebase guidelines would suggest moving it there.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web-ui/src/components/workspace/OnboardingCard.tsx` around lines 29 - 31, Move the OnboardingCardProps interface out of OnboardingCard.tsx into the shared types module (export it from web-ui/src/types/index.ts as OnboardingCardProps) and update OnboardingCard.tsx to import { OnboardingCardProps } from that types file; ensure the exported name matches the original interface and update any other files referencing OnboardingCardProps to import from the central types file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web-ui/src/__tests__/components/workspace/OnboardingCard.test.tsx`:
- Around line 6-12: The MockLink in the jest.mock('next/link') block uses the
React.ReactNode type but React isn't imported; add an import for React (e.g.,
import * as React from 'react' or import React from 'react') at the top of the
test file so the React.ReactNode annotation on the MockLink props is valid and
TypeScript errors are resolved.
---
Nitpick comments:
In `@web-ui/src/components/workspace/OnboardingCard.tsx`:
- Around line 33-40: The flash happens because isDismissed starts false then is
set in useEffect; instead initialize it lazily and keep the effect to handle
workspacePath changes: change useState to useState(() =>
getOnboardingDismissed(workspacePath)) in OnboardingCard so the correct value is
used on first render (prevents flicker), and retain the existing
useEffect([workspacePath]) with
setIsDismissed(getOnboardingDismissed(workspacePath)) to update when
workspacePath changes.
- Around line 29-31: Move the OnboardingCardProps interface out of
OnboardingCard.tsx into the shared types module (export it from
web-ui/src/types/index.ts as OnboardingCardProps) and update OnboardingCard.tsx
to import { OnboardingCardProps } from that types file; ensure the exported name
matches the original interface and update any other files referencing
OnboardingCardProps to import from the central types file.
In `@web-ui/src/lib/workspace-storage.ts`:
- Around line 85-88: The localStorage key built from workspacePath in
getOnboardingDismissed can be inconsistent for paths with special characters;
update getOnboardingDismissed to use encodeURIComponent(workspacePath) when
constructing the key (`codeframe_onboarding_dismissed_${...}`) and make the
identical change in setOnboardingDismissed so both reader and writer use the
same encoded key format; keep the existing typeof window === 'undefined' guard
and the 'true' string comparison logic unchanged.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 03213cec-fd31-4533-b077-37ca1af9650e
📒 Files selected for processing (5)
web-ui/src/__tests__/components/workspace/OnboardingCard.test.tsxweb-ui/src/app/page.tsxweb-ui/src/components/workspace/OnboardingCard.tsxweb-ui/src/components/workspace/index.tsweb-ui/src/lib/workspace-storage.ts
|
test edit |
Review: OnboardingCard (#467)Clean, focused PR. SSR hydration approach is correct, test coverage solid, component scoped appropriately. Minor Issue 1 - Flash on first render (dismissed users): The useState(false)+useEffect pattern is the right call for SSR safety. But dismissed users briefly see the card before the effect fires. Standard fix: track a mounted boolean alongside isDismissed, set both in the useEffect, gate the null return on both. Not a blocker. Minor Issue 2 - Dismiss button uses raw element: The className replicates variant=ghost size=icon. Use the Button component for consistency with the design system. Minor Issue 3 - localStorage key uses raw workspace path: codeframe_onboarding_dismissed_workspacePath embeds the path directly - can produce odd keys on Windows or paths with special chars. Wrap with encodeURIComponent(workspacePath). Looks Good: SSR guard (typeof window === undefined) in workspace-storage.ts matches existing helpers. Page conditional on tasksData !== undefined is correct. 7 tests with mock-then-render work because RTL render() flushes via act(). WorkspaceStatsCards placement is right. aria-label on icon-only button is correct. Suggestion: A clearOnboardingDismissed export in workspace-storage.ts would be useful for reset-workspace flows and E2E test setup. Overall: ready to merge after Button component consistency fix and localStorage key encoding. Flash trade-off is acceptable. |
- Add React import to test file (React.ReactNode type annotation) - Use encodeURIComponent in localStorage key for path safety - Use lazy useState initializer to prevent flash-of-content
Review: feat(web-ui): onboarding guidance card for new workspacesGood addition — the feature is well-scoped, tests are solid, and the One correctness issue worth addressing before merge: 🔴 SSR/Hydration Mismatch —
|
…sting DBs Existing workspaces created before github_issue_number was added were missing the column in _ensure_schema_upgrades, causing API 500 errors on tasks endpoints. Matches the migration already in _init_database.
ReviewThe onboarding card implementation is clean and well-scoped. The CodeRabbit suggestions (lazy 🔴 Unrelated change:
|
|
The component is well-scoped and the implementation is clean. The fixes from CodeRabbit's previous pass (lazy One concern: unrelated schema migration in The addition of Minor: CodeRabbit flagged moving the interface to Everything else looks good:
The |
1 participant
Fixes #467
Summary
OnboardingCardcomponent that displays when a workspace is initialized but has no tasks yet/prdChanges
web-ui/src/lib/workspace-storage.ts— addsgetOnboardingDismissedandsetOnboardingDismissedhelpersweb-ui/src/components/workspace/OnboardingCard.tsx— new componentweb-ui/src/components/workspace/index.ts— barrel exportweb-ui/src/app/page.tsx— renders card whentasksData.tasks.length === 0web-ui/src/__tests__/components/workspace/OnboardingCard.test.tsx— 7 unit testsTest plan
npm test— 380/380 unit tests pass, 7 new tests for OnboardingCard/prdSummary by CodeRabbit
New Features
Tests