Redirect to root when starting demo #3323
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughUpdated Shepherd import/entrypoints and CDN references (v15 → v17). Added cross-page tour coordination: Header sets Changes
Sequence DiagramssequenceDiagram
participant User as User
participant Header as Header (client script)
participant Session as sessionStorage
participant Router as Browser / Router
participant Index as Index Page (init)
participant Shepherd as Shepherd instance
User->>Header: Click "Demo"
alt not on "/"
Header->>Session: set startTourOnLoad = "1"
Header->>Router: navigate to "/"
Router->>Index: load / astro:page-load
Index->>Session: read startTourOnLoad
alt flag present
Index->>Session: remove startTourOnLoad
Index->>Shepherd: initialize → start()
else
Index->>Shepherd: initialize (ready)
end
else already on "/"
Header->>Index: dispatch "startTour"
Index->>Shepherd: start()
end
Note right of Index: re-attaches init on astro:page-load for SPA navigation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs-src/src/content/docs/guides/install.mdx (1)
47-58: Inconsistent version reference in documentation.Line 47 mentions "v10.0.1" as an example version, but the code snippet uses v17.0.0. Update the text to match the actual version shown in the code.
🔎 Suggested fix
-You can use jsDelivr to pull down any release from npm. For example, you could include v10.0.1 with styles in your app +You can use jsDelivr to pull down any release from npm. For example, you could include v17.0.0 with styles in your app
🧹 Nitpick comments (1)
landing/src/components/Header.astro (1)
177-187: Consider normalizing pathname for edge cases.The pathname check
window.location.pathname !== '/'may not handle edge cases where the pathname could be an empty string or have a trailing slash variation. Consider normalizing:🔎 Suggested improvement
newBtn.addEventListener('click', () => { // Check if we're on the home page - if (window.location.pathname !== '/') { + const isHome = window.location.pathname === '/' || window.location.pathname === ''; + if (!isHome) { // Store flag to start tour after navigation sessionStorage.setItem('startTourOnLoad', 'true'); window.location.href = '/'; } else {
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs-src/src/content/docs/guides/install.mdxlanding/src/components/Header.astrolanding/src/pages/index.astroshepherd.js/dummy/index.htmlshepherd.js/package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Tests
🔇 Additional comments (4)
shepherd.js/dummy/index.html (1)
125-125: LGTM!The import path update aligns with the new distribution structure.
landing/src/components/Header.astro (1)
190-195: LGTM!The dual event listener setup correctly handles both the initial page load (
DOMContentLoaded) and Astro's client-side navigation (astro:page-load).landing/src/pages/index.astro (2)
305-309: LGTM!The
ready()function correctly sets up listeners for both initial load and Astro page transitions.
26-28: LGTM!The script path in the code example is consistent with the updated distribution structure.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
landing/src/pages/index.astro (2)
97-103: Auto-start logic looks good.The sessionStorage-based coordination correctly enables the cross-page demo flow: the Header component sets the flag and navigates, then this page auto-starts the tour on load. The check-and-remove pattern prevents duplicate starts.
💡 Optional: Add comment explaining the cross-page coordination
- // Check if we should auto-start the tour (after redirect from another page) + // Cross-page coordination: Header's Demo button sets this flag and navigates here, + // triggering an auto-start. Remove the flag to prevent re-starting on refresh. const shouldStartTour = sessionStorage.getItem('startTourOnLoad'); if (shouldStartTour) { sessionStorage.removeItem('startTourOnLoad'); shepherd.start(); }
95-95: Document the timeout duration.The 400ms timeout is a magic number with no explanation. While it may work in practice, documenting why this specific duration was chosen would help future maintainers.
💡 Optional: Add explanation for the timeout
- // wait for shepherd to be ready + // Wait for Shepherd to be ready and DOM to fully render + // 400ms empirically determined to be sufficient for initialization setTimeout(function () {
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
landing/src/pages/index.astro
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Tests
🔇 Additional comments (1)
landing/src/pages/index.astro (1)
105-115: AbortController pattern correctly addresses past review feedback.The implementation properly prevents event listener accumulation across Astro page transitions by:
- Aborting any existing listener before adding a new one
- Storing the AbortController on
windowfor lifecycle management- Passing the signal to
addEventListenerThis matches the recommended fix from the previous review.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
landing/src/pages/index.astro (1)
105-115: Excellent fix for listener accumulation.The AbortController pattern correctly prevents duplicate event listeners from accumulating across page transitions. The implementation properly aborts the previous listener before registering a new one.
Optional: Improve type safety
Consider extending the Window interface instead of using
anycasts:Add at the top of the script block:
declare global { interface Window { __startTourAbortController?: AbortController; } }Then replace the casts:
- if ((window as any).__startTourAbortController) { - (window as any).__startTourAbortController.abort(); + if (window.__startTourAbortController) { + window.__startTourAbortController.abort(); } const controller = new AbortController(); - (window as any).__startTourAbortController = controller; + window.__startTourAbortController = controller;
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
landing/src/pages/index.astro
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Tests
🔇 Additional comments (3)
landing/src/pages/index.astro (3)
315-316: Correct fix for double initialization.Removing the
DOMContentLoadedlistener and relying solely onastro:page-loadis the right approach. This event fires on both initial page load and subsequent navigations, eliminating the redundant initialization that occurred previously.
97-103: Auto-start logic is correct.The sessionStorage key
startTourOnLoadis consistently used across both the Header component (sets the flag and redirects) and the index page (reads, removes, and starts the tour). The implementation properly cleans up the flag after use to prevent duplicate auto-starts.
27-27: The script path update toshepherd.js/dist/js/shepherd.mjsis correct and matches the package's configured exports inpackage.json.
Closes #3316
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.