Redesign signup and onboarding as a perspective corridor - #26
Conversation
|
Warning Review limit reachedNext included review available in 32 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (3)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (5)
💤 Files with no reviewable changes (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughChangesThe PR adds a reusable Corridor layout and onboarding flow
Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: ⚪ Minimal · up to The signup and onboarding pages now use the shared corridor presentation while preserving the existing setup and authentication flows. No current merge-blocking risk is identified. Sequence Diagram(s)sequenceDiagram
participant GetStartedPage
participant CorridorPage
participant OnboardingCard
participant SetupPlan
GetStartedPage->>CorridorPage: Render page layout
CorridorPage->>OnboardingCard: Render onboarding when open
OnboardingCard->>GetStartedPage: Complete or cancel callback
GetStartedPage->>SetupPlan: Render setup plan after onboarding
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (4 skipped: 4 unsupported.) ✨ 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 |
|
🤖 review-bot (run 13:12 · HEAD Generated by Claude Code |
|
Generated by Claude Code |
|
🤖 review-bot (run 13:12 · HEAD Generated by Claude Code |
|
🤖 review-bot (run 13:12 · HEAD Generated by Claude Code |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 5 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/lib/onboarding/OnboardingCard.svelte">
<violation number="1" location="src/lib/onboarding/OnboardingCard.svelte:39">
P3: `OnboardingCard` duplicates the onboarding flow in `OnboardingModal`, so changes to steps and analytics must stay synchronized manually. Extract the shared state and handlers into a reusable onboarding controller or component.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| if (!usage && Object.values(USAGE).includes(initialUsage)) usage = initialUsage; | ||
| }); | ||
|
|
||
| function toggle(values, value) { |
There was a problem hiding this comment.
P3: OnboardingCard duplicates the onboarding flow in OnboardingModal, so changes to steps and analytics must stay synchronized manually. Extract the shared state and handlers into a reusable onboarding controller or component.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/lib/onboarding/OnboardingCard.svelte, line 39:
<comment>`OnboardingCard` duplicates the onboarding flow in `OnboardingModal`, so changes to steps and analytics must stay synchronized manually. Extract the shared state and handlers into a reusable onboarding controller or component.</comment>
<file context>
@@ -0,0 +1,155 @@
+ if (!usage && Object.values(USAGE).includes(initialUsage)) usage = initialUsage;
+ });
+
+ function toggle(values, value) {
+ return values.includes(value) ? values.filter((item) => item !== value) : [...values, value];
+ }
</file context>
|
🤖 review-bot (run 13:12 · HEAD Generated by Claude Code |
|
🤖 review-bot (run 13:12 · HEAD Generated by Claude Code |
- Focus the step heading on mount so reopening onboarding via "Change" doesn't strand keyboard/screen-reader focus at the document body, matching OnboardingModal's existing behavior. - Don't render OnboardingCard/SetupPlan until stored preferences are resolved, so returning visitors no longer see a flash of the questionnaire and no longer fire a false Onboarding Opened event before their saved preferences load. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X5b9PuMmCLDhSWwZvAdz9y
Automated review-bot triage
Trusted review (current HEAD Plan: one commit — gate Generated by Claude Code |
|
@cubic-dev-ai re: the Deferred — real concern, not fixed in this PR.
Revisit when a third onboarding placement is added, or the two flows drift out of sync on a future change. Marked with Generated by Claude Code |
|
🤖 review-bot (run 13:12 · HEAD Generated by Claude Code |
@Acksell Agreed — deferring is the right call. The duplication is real (the step state machine, The Tip: get faster answers by chatting with cubic’s review copilot in the review UI |
|
🤖 review-bot (run 13:26 UTC · HEAD Generated by Claude Code |
|
🤖 review-bot (run 13:26 UTC · HEAD Generated by Claude Code |
|
🤖 review-bot (run 13:26 UTC · HEAD Generated by Claude Code |
|
🤖 review-bot (run 13:26 UTC · HEAD Generated by Claude Code |
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/lib/onboarding/OnboardingCard.svelte">
<violation number="1" location="src/lib/onboarding/OnboardingCard.svelte:2">
P3: This auto-generated `TODO(ai-review)` comment from the review bot was left in committed source. It references a PR URL rather than a maintained issue and the `ai-review` tag is tooling noise. Drop the `(ai-review)` tag and link an actual tracked issue, or remove the comment.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| @@ -0,0 +1,159 @@ | |||
| <script> | |||
| // TODO(ai-review): duplicates OnboardingModal's step flow; extract a shared controller. https://github.com/supaclank/web/pull/26 | |||
There was a problem hiding this comment.
P3: This auto-generated TODO(ai-review) comment from the review bot was left in committed source. It references a PR URL rather than a maintained issue and the ai-review tag is tooling noise. Drop the (ai-review) tag and link an actual tracked issue, or remove the comment.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/lib/onboarding/OnboardingCard.svelte, line 2:
<comment>This auto-generated `TODO(ai-review)` comment from the review bot was left in committed source. It references a PR URL rather than a maintained issue and the `ai-review` tag is tooling noise. Drop the `(ai-review)` tag and link an actual tracked issue, or remove the comment.</comment>
<file context>
@@ -1,4 +1,5 @@
<script>
+ // TODO(ai-review): duplicates OnboardingModal's step flow; extract a shared controller. https://github.com/supaclank/web/pull/26
import { onMount, tick, untrack } from 'svelte';
import ChoiceCard from './ChoiceCard.svelte';
</file context>
There was a problem hiding this comment.
Won't do — TODO(ai-review) is an intentional, repo-wide convention (not tooling noise): it's the bot-agnostic marker this repo's automated review-bot triage routine leaves for every Defer verdict, so git grep "TODO(ai-review)" finds every deferred review-bot suggestion across vendors (CodeRabbit, Cubic, Greptile, etc.). Linking the PR discussion thread (#discussion_r3924946619, where the duplication tradeoff is explained) is the anchor by design — there's no separate tracked issue to link because the PR thread is the record of the decision. Not removing it.
Generated by Claude Code
|
🤖 review-bot (run 13:26 UTC · HEAD Generated by Claude Code |
|
🤖 review-bot (run 13:38 UTC · HEAD Generated by Claude Code |
|
Generated by Claude Code |
Prerendered/no-JS visitors saw a blank corridor since the ready gate rendered nothing, unlike other async routes (e.g. welcome) which show a loading placeholder. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019gDWM1pQoTmQ9k7HxRvkiE
Automated review-bot triage
Trusted review (current HEAD, before this push): Cubic ✓ (2 new findings on Plan: one commit — add a "Loading…" fallback for the pre- Note: while this was in flight, an unrelated commit ( Generated by Claude Code |
|
🤖 review-bot (run 13:26 UTC · HEAD Generated by Claude Code |
|
🤖 review-bot (run 13:38 UTC · HEAD Generated by Claude Code |
|
🤖 review-bot (run started · HEAD Generated by Claude Code |
|
🤖 review-bot — Generated by Claude Code |
|
🤖 review-bot (run 13:38 UTC · HEAD Generated by Claude Code |
|
🤖 review-bot (run started · HEAD Generated by Claude Code |
|
🤖 review-bot (run started · HEAD Generated by Claude Code |
✅ Review-bot convergedrun 13:38 UTC · HEAD While this run was waiting on cubic's re-check of Loop totals so far (both runs): 2 rounds, 6 findings (3 do, 1 defer, 1 won't-do, 1 skip). Generated by Claude Code |
|
🤖 review-bot (run started · HEAD Generated by Claude Code |
2 participants
Summary
Testing
bun run buildbun test(75 passed, 1 skipped)Summary by cubic
Signup and get-started now share a perspective-grid corridor, replacing the get-started modal with a centered onboarding card and giving signup the same visual treatment. Longer setup content scrolls without changing the corridor geometry.
Written for commit db5bb96. Summary will update on new commits.
Summary by CodeRabbit
New Features
Improvements