feat: guided tour component#842
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
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 (9)
💤 Files with no reviewable changes (3)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThis PR adds a new Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
packages/raystack/components/tour/use-tour-target.ts (1)
118-125: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoffObserving the entire
document.bodysubtree on every active step.
childList: true, subtree: trueondocument.bodymeans every DOM mutation anywhere on the page re-runsreconcile()for every active tour instance. On pages with frequent unrelated DOM churn this adds continuous overhead while a tour step is active. Given the target is usually scoped to a small part of the page, consider scoping the observer to a narrower ancestor when derivable, or debouncingreconcile().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/raystack/components/tour/use-tour-target.ts` around lines 118 - 125, The MutationObserver setup in useTourTarget is watching the entire document.body subtree, which causes reconcile() to run on unrelated DOM churn during active steps. Update the observer in useTourTarget to scope the observation to a narrower ancestor when the target can be derived, or add debouncing/throttling around reconcile() so frequent mutations don’t trigger continuous rechecks.apps/www/src/content/docs/components/tour/props.ts (1)
144-153: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDoc type mirrors lose fidelity for the render-function signature and the event union.
TourContentProps.children's render-function parameter is typedunknownhere, but the real signature isTourRenderProps(step/index/totalSteps/isFirstStep/isLastStep/status/actions) pertour-content.tsx. Similarly,TourEventis flattened into one interface with all fields optional, hiding thatstep/statusare only present for specifictypevalues (pertypes.ts's discriminated union). Readers relying solely on the auto-generated table won't see either shape.Consider exporting a
TourRenderPropsinterface here (referenced from an<auto-type-table>block) and adding a short prose note next to theTourEventtable clarifying which fields apply to whichtype.Also applies to: 186-188
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/www/src/content/docs/components/tour/props.ts` around lines 144 - 153, The generated docs types are losing the real shapes for `TourContentProps.children` and `TourEvent`. Update the docs in this section to reference the actual `TourRenderProps` shape used by `tour-content.tsx` instead of `unknown`, and make `TourEvent` reflect the discriminated union from `types.ts` so `step` and `status` are only shown where they apply. Use the existing `TourContentProps`, `TourRenderProps`, and `TourEvent` symbols to place the correction, and add a brief note near the event table clarifying which fields belong to each `type`.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/www/src/components/playground/tour-examples.tsx`:
- Around line 29-37: The `settings` tour example has a content/position
mismatch: the description says the popover is anchored on the right, but the
`settings` object in `tour-examples.tsx` uses `side: 'bottom'`. Update the
`content` text or the `side` value so they agree, and keep the example in sync
with the intended behavior of `settings` and its
`spotlightRadius`/`spotlightPadding` configuration.
In `@packages/raystack/components/tour/tour-overlay.tsx`:
- Around line 192-199: The TourOverlay div is letting consumer props override
implementation-critical attributes because `{...rest}` is spread after
`aria-hidden`, `data-status`, `data-entered`, and `data-hole-open`. Update
`TourOverlay` so these chrome attributes are applied last, or otherwise
explicitly filter/override conflicting props from `rest`, ensuring
`aria-hidden`, `data-entered`, `data-hole-open`, and `data-status` always come
from the component state.
In `@packages/raystack/components/tour/tour-root.tsx`:
- Around line 416-431: The scroll-into-view effect in tour-root.tsx is
re-running on unrelated re-renders because it depends on the unstable step
object, which can fight user scrolling during an active tour. Update the effect
in TourRoot to key off the stable index primitive instead of step, while keeping
the existing guards for popoverOpen, anchor, and reducedMotion so it only
scrolls when the active step truly changes.
In `@packages/raystack/components/tour/use-tour-target.ts`:
- Around line 64-131: Function-type TourTarget values are re-triggering
useTourTarget’s effect on every parent render because the dependency list keys
off target identity, which keeps recreating the MutationObserver and restarting
the timeout. Update useTourTarget to avoid relying on referential equality for
function targets in the effect lifecycle—either resolve/comparison-cache
function targets per render or otherwise stabilize them before the effect
logic—so onNotFound can still fire reliably. Keep the fix within useTourTarget
and the resolveTourTarget/reconcile flow, and ensure inline function targets no
longer reset the timeout on unrelated renders.
- Around line 86-131: The timeout handling in useTourTarget allows
onNotFoundRef.current to be scheduled again after it has already fired, which
breaks the “fires once” behavior. Add a one-time latch in the
reconcile/startTimer/timeout path so that once the not-found callback runs,
subsequent reconcile calls for the same unresolved target do not schedule
another timeout. Keep the fix localized to useTourTarget and preserve the
existing clearTimer behavior for resolve/reconnect cases.
- Around line 6-36: resolveTourTarget currently returns Element and ref targets
without enforcing the documented connectedness guarantee, so direct callers can
receive detached nodes. Update resolveTourTarget in use-tour-target.ts so the
Element and RefObject branches also verify .isConnected and return null for
orphaned nodes, matching the behavior already enforced by useTourTarget’s
reconcile() and preserving the docstring contract.
---
Nitpick comments:
In `@apps/www/src/content/docs/components/tour/props.ts`:
- Around line 144-153: The generated docs types are losing the real shapes for
`TourContentProps.children` and `TourEvent`. Update the docs in this section to
reference the actual `TourRenderProps` shape used by `tour-content.tsx` instead
of `unknown`, and make `TourEvent` reflect the discriminated union from
`types.ts` so `step` and `status` are only shown where they apply. Use the
existing `TourContentProps`, `TourRenderProps`, and `TourEvent` symbols to place
the correction, and add a brief note near the event table clarifying which
fields belong to each `type`.
In `@packages/raystack/components/tour/use-tour-target.ts`:
- Around line 118-125: The MutationObserver setup in useTourTarget is watching
the entire document.body subtree, which causes reconcile() to run on unrelated
DOM churn during active steps. Update the observer in useTourTarget to scope the
observation to a narrower ancestor when the target can be derived, or add
debouncing/throttling around reconcile() so frequent mutations don’t trigger
continuous rechecks.
🪄 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: 481fc9fa-773c-41e2-b85f-368b5e06a592
📒 Files selected for processing (22)
apps/www/src/app/examples/tour/page.tsxapps/www/src/components/demo/demo.tsxapps/www/src/components/playground/index.tsapps/www/src/components/playground/tour-examples.tsxapps/www/src/components/tour-demo.tsxapps/www/src/content/docs/components/tour/demo.tsapps/www/src/content/docs/components/tour/index.mdxapps/www/src/content/docs/components/tour/props.tsdocs/rfcs/003-guided-tour-component.mdpackages/raystack/components/tour/__tests__/tour.test.tsxpackages/raystack/components/tour/index.tsxpackages/raystack/components/tour/tour-content.tsxpackages/raystack/components/tour/tour-context.tsxpackages/raystack/components/tour/tour-overlay.tsxpackages/raystack/components/tour/tour-parts.tsxpackages/raystack/components/tour/tour-root.tsxpackages/raystack/components/tour/tour.module.csspackages/raystack/components/tour/tour.tsxpackages/raystack/components/tour/types.tspackages/raystack/components/tour/use-prefers-reduced-motion.tspackages/raystack/components/tour/use-tour-target.tspackages/raystack/index.tsx
Shreyag02
requested changes
Jul 7, 2026
| * Whether two rects match within half a pixel on every side. Shared with the | ||
| * root's reveal loop, which feeds it `DOMRect`s (`x`/`y` mirror `left`/`top`). | ||
| */ | ||
| export function rectsEqual(a: SpotlightRect, b: SpotlightRect) { |
Contributor
There was a problem hiding this comment.
Move rectsEqual (and SpotlightRect) out of the overlay into a shared utils.ts.
Right now rectsEqual lives in the overlay component but tour-root.tsx imports it — so the root is reaching into a leaf component to grab geometry helpers.
That's backwards: the parent shouldn't depend on internals of a child.
Shreyag02
approved these changes
Jul 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
tour-root,tour-content,tour-overlay,tour-parts,tour-context) with supporting hooks (use-tour-target,use-prefers-reduced-motion) and shared types