feat: Guided Tours Custom Navigation#2340
Conversation
🎩 PreviewA preview build has been created at: |
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
7f83eb8 to
9899ee2
Compare
84f39e0 to
d6d13ae
Compare
9899ee2 to
a773cce
Compare
d6d13ae to
e97b036
Compare
a773cce to
b9acf61
Compare
e97b036 to
d308ae7
Compare
9b16f8f to
fadf84a
Compare
95b9b2a to
01da4c7
Compare
fadf84a to
5cd15ef
Compare
01da4c7 to
d9c0e95
Compare
dd40a33 to
47cfe52
Compare
febd689 to
68175d1
Compare
47cfe52 to
606887b
Compare
68175d1 to
aaa1a46
Compare
606887b to
d29cf67
Compare
6b7ed93 to
af8b204
Compare
d29cf67 to
5455cd2
Compare
af8b204 to
7938fe4
Compare
| ); | ||
| } | ||
|
|
||
| export function TourNavigation(props: NavigationProps) { |
There was a problem hiding this comment.
🤖 This is an AI-generated code review comment.
This PR adds new exported React components/hooks under src/providers/TourProvider, but react-compiler.config.js is unchanged and src/providers/TourProvider is not covered by REACT_COMPILER_ENABLED_DIRS. The review standard treats missing compiler coverage for new components/hooks as High because it bypasses the project's incremental React Compiler adoption.
Suggestion: Add the new TourProvider files, or the cleaned-up TourProvider directory, to react-compiler.config.js after fixing compiler-incompatible patterns in this PR.
Rule: tangle-ui-review: React Compiler adoption; .claude/skills/react-patterns/SKILL.md: Adding Files to React Compiler
|
|
||
| const stepsLength = steps.length; | ||
|
|
||
| recordTourVisited(currentStep); |
There was a problem hiding this comment.
🤖 This is an AI-generated code review comment.
recordTourVisited(currentStep) mutates module-level state while TourNavigation renders. This is not React Compiler compatible and also makes visited navigation state global to the module instead of scoped to a TourProvider instance.
Suggestion: Keep visited progress in provider-scoped React state/context, and update it from an effect or navigation action rather than mutating module state during render.
Rule: .claude/skills/react-patterns/SKILL.md: React Compiler compatible code must not mutate values during render
| ); | ||
| } | ||
|
|
||
| export function TourNavigation(props: NavigationProps) { |
There was a problem hiding this comment.
🤖 This is an AI-generated code review comment.
FTA reports TourNavigation.tsx at 61.75 with cyclomatic complexity 40 across 280 lines, which is over the strong-flag thresholds. The new file combines navigation buttons, revisit tracking, checklist rendering, progress dots, and auto-advance timing.
Suggestion: Split this into smaller sibling components/hooks, e.g. TourNavButton, TourProgressDots, useVisitedTourSteps, and TourAutoAdvance, so each unit has one responsibility and lower complexity.
Rule: static-analysis skill: FTA > 60 and cyclomatic complexity > 20 are strong flags; tangle-ui-review: Readability & component size
| stepIndex: number; | ||
| }) { | ||
| const { isStepComplete } = useTourProgress(); | ||
| const interactiveStep = step as ChecklistStep | undefined; |
There was a problem hiding this comment.
🤖 This is an AI-generated code review comment.
The new checklist casts StepType to ChecklistStep instead of narrowing or typing the tour step metadata at the source. That bypasses the TypeScript standard's preference for type guards and proper source typing over as Type assertions.
Suggestion: Define a project tour-step type that extends Reactour's StepType with the supported interaction metadata, or add a small type guard that checks interaction before passing the step to tourActionLabel.
Rule: .claude/skills/typescript-standards/SKILL.md: avoid unsafe type casting; prefer proper typing/type guards
| <InlineStack | ||
| gap="2" | ||
| blockAlign="center" | ||
| className="rounded-md bg-muted px-3 py-2" |
There was a problem hiding this comment.
🤖 This is an AI-generated code review comment.
The new navigation UI passes Tailwind className values into Tangle UI primitives such as InlineStack, Icon, Text, BlockStack, and Badge. The UI primitive standard says to use semantic props, tone, variants, or a local raw-HTML primitive rather than styling primitives through className.
Suggestion: Move layout/background needs into supported primitive props or a small local wrapper/pattern component, and use semantic tone/variant props for text/icon/dot colors instead of hardcoded color classes.
Rule: tangle-ui-review: UI primitives; .claude/skills/ui-primitives/SKILL.md: prefer primitives and semantic props over raw class styling

Description
Replaces reactour's default Prev/Next with a custom tour navigation and the interactive-step UX layer that sits on top of it: progress dots, a completion-gated "Next", an action checklist in the popover, and auto-progress. Visible behavior becomes observable once the registry has tours (#2306 onwards).
What's in it
Custom navigation
current/visited/unvisited). Non-clickable — bouncing between arbitrary steps would leave reactour inconsistent on tours with interaction gates.setSteps.Step completion + gated "Next"
TourProgressContexttracks per-step completion (aSetof completed step indices), reset whenever a tour opens.Action checklist
TourStepChecklistrenders the step's required action in the popover and ticks it off —Circle→CircleCheckwith a check animation and strike-through — when complete.tourActionLabeland may embed bold targets (e.g. "Select the Greet task"), rendered viarenderInline(exported fromTourContent). Each interaction's label is authored in the PR that introduces that interaction (feat: Guided Tours Framework (Navigating the Editor) #2299 / feat: Guided Tours Framework (First Pipeline) #2347 / feat: Guided Tours Framework (Subgraphs) #2365).Auto-progress
TourAutoAdvanceadvances ~800ms after a fresh completion — long enough to see the tick register. Steps already complete on arrival (e.g. revisited via Back) stay on a manual click; clicking "Next" advances immediately and cancels the pending auto-advance.File layout
TourNavigation.tsx(TourNavigation,NavButton,GatedNextButton/renderNextButton,TourStepChecklist,TourAutoAdvance, and the module-level visit-tracking helpers).TourPopover.tsxkeeps popover styling/positioning, the viewport clamp bridge, and the completion actions.TourProgressContext.tsxholds the completion state +useTourProgresshook. Tests:TourNavigation.test.tsxandTourProgressContext.test.tsx.Popover polish
Related Issue and Pull requests
Progresses https://github.com/Shopify/oasis-frontend/issues/583
Depends on #2348 (foundation). The interaction detection that drives completion lands in #2299; interaction-specific checklist labels are authored in #2299 / #2347 / #2365.
Type of Change
Checklist
Test Instructions
With a tour registered (use this branch with #2306 on top):
Dots
Checklist + gated Next
Visit-aware Next + revisits
Reset between tours
/learn/tours, start a different one. Completion + visited-max reset for the new tour.