feat: Onboarding Pill#2440
Conversation
🎩 PreviewA preview build has been created at: |
a27cdb0 to
72eadf3
Compare
c38d713 to
5f5d39b
Compare
72eadf3 to
14fbe7b
Compare
5f5d39b to
c994062
Compare
14fbe7b to
a39fbd9
Compare
c994062 to
bbdf96e
Compare
a39fbd9 to
ebc0b79
Compare
bbdf96e to
5ea2aaa
Compare
ebc0b79 to
3b09b65
Compare
7b70ec6 to
601fa2a
Compare
30b5b30 to
5d180fd
Compare
601fa2a to
0da37a4
Compare
5d180fd to
4e68a1c
Compare
4e68a1c to
c818dc8
Compare
0da37a4 to
5a2f860
Compare
camielvs
left a comment
There was a problem hiding this comment.
Pill + readiness gating + toasts look good. The isResolved = (backendReady || !configured) && isReady contract is a clean way to avoid the flash, and the completedRef baseline correctly suppresses toast spam on first load. Two inline notes.
b5d4e02 to
f838ac6
Compare
8bdff0e to
2741cce
Compare
f838ac6 to
0b64d0e
Compare
2741cce to
7e5fb3b
Compare
61f4548 to
97f6ae5
Compare
7e5fb3b to
3691b8e
Compare
97f6ae5 to
9daead8
Compare
3691b8e to
12d1304
Compare
| <Button | ||
| variant="ghost" | ||
| size="sm" | ||
| className="h-8 gap-1.5 rounded-full bg-stone-700 px-3 text-xs font-semibold text-white hover:bg-stone-600 hover:text-white" |
There was a problem hiding this comment.
🤖 This is an AI-generated code review comment.
The new component passes raw Tailwind className values into Tangle UI primitives (Button and PopoverContent), including hard-coded sizing, spacing, rounded, and color classes. That bypasses the primitive API and makes the pill harder to keep consistent with the design system.
Suggestion: Move this styling behind a semantic primitive/variant or a co-located higher-level pattern for the onboarding pill, and avoid passing className directly to UI primitives. For the popover width, prefer a semantic prop/pattern or a dedicated wrapper rather than className="w-96".
Rule: CLAUDE.md UI rule: never pass className to a Tangle UI primitive; .cursorrules UI Primitives: prefer primitives and semantic props over raw utility styling.
|
|
||
| const completedRef = useRef<Set<string> | null>(null); | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
🤖 This is an AI-generated code review comment.
FTA reports OnboardingProvider.tsx cyclomatic complexity increased from 13 to 24 after this PR, crossing the >20 threshold. The new effect combines resolution gating, set diffing, completion handling, label lookup, and toast dispatch in one block, and it also depends on desiredSteps, a fresh object created during render, so the effect is scheduled on every render.
Suggestion: Extract pure helpers for completed-step calculation and newly-completed diffing, then keep the effect focused on side effects. Replace the object dependency with stable primitive dependencies such as individual step booleans or a derived primitive key.
Rule: static-analysis skill: flag cyclomatic complexity >20; docs/react-best-practices.md: dependencies should be stable.

Description
Second PR in the onboarding series (stacked on #2421). Makes onboarding progress visible from anywhere in the app, not just the Learning Hub.
OnboardingNavPill— a compactOnboarding · X/Ypill that opens a popover with the full checklist. Mounted in both the v1 and v2 editor navs (hidden in tour mode). It hides itself once onboarding is complete or dismissed.isReady/isResolvedso the pill (and other surfaces) only render once backend state has actually resolved, avoiding a flash of default/incorrect state on load.No new routes or page-level changes — purely an always-available entry point plus the supporting provider state.
Related Issue and Pull requests
Progresses https://github.com/Shopify/oasis-frontend/issues/622
Stacked on #2421 → followed by #2435
Type of Change
Checklist
Screenshots (if applicable)
Test Instructions
Onboarding · X/Ypill appears in the top nav in both the v1 and v2 editors.Additional Comments
Gating is on
isResolved(backend resolved + state loaded) rather thanisReady, so the pill never renders against unresolved state.