fix: improve mobile UX for Specification Explorer.#5455
Conversation
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughVisualizer detects when the viewport is narrower than 1024px and prevents schema loading/rendering; DocsLayout adds a mobile-only “Specification Explorer is not available” screen and renders the desktop explorer layout only on large viewports. ChangesResponsive Explorer UI
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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 |
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-5455--asyncapi-website.netlify.app/ |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #5455 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 22 22
Lines 906 906
Branches 171 171
=========================================
Hits 906 906 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@components/layout/DocsLayout.tsx`:
- Line 93: DocsLayout currently hardcodes the fallback spec link
href='/docs/reference/specification/v3.0.0'; update the component to derive the
reference URL from the current page slug/path instead of using a fixed version.
In the DocsLayout component, read the current route (e.g., via the router or
path/slug prop used in this layout) and construct the href dynamically (for
example using the extracted version segment or full slug) so mobile users are
routed to the same explorer version they opened; replace the hardcoded string in
the link with this computed URL (ensure you update uses of the link target in
DocsLayout to reference the new computed variable).
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 25d4c529-8fa0-41d4-a637-190e0ab06e26
📒 Files selected for processing (2)
components/docs/Visualizer.tsxcomponents/layout/DocsLayout.tsx
There was a problem hiding this comment.
🧹 Nitpick comments (1)
components/docs/Visualizer.tsx (1)
36-42: 💤 Low valueSimplify the early-return cleanup.
When the explorer is unavailable, the cleanup function sets
isMounted = false, but no async operation is running that would check this flag. The cleanup can be omitted entirely in this branch for clarity.♻️ Proposed simplification
if (!isExplorerAvailable) { setSchema(null); - - return () => { - isMounted = false; - }; + return; }🤖 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 `@components/docs/Visualizer.tsx` around lines 36 - 42, The early-return branch that checks isExplorerAvailable and calls setSchema(null) should drop the unnecessary cleanup; remove the returned function that sets isMounted = false from that branch because no async work relies on isMounted there. Update the Visualizer effect/handler containing isExplorerAvailable, setSchema, and isMounted so that when !isExplorerAvailable you only call setSchema(null) and return immediately without a cleanup function.
🤖 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.
Nitpick comments:
In `@components/docs/Visualizer.tsx`:
- Around line 36-42: The early-return branch that checks isExplorerAvailable and
calls setSchema(null) should drop the unnecessary cleanup; remove the returned
function that sets isMounted = false from that branch because no async work
relies on isMounted there. Update the Visualizer effect/handler containing
isExplorerAvailable, setSchema, and isMounted so that when !isExplorerAvailable
you only call setSchema(null) and return immediately without a cleanup function.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 243bd853-95f6-4542-8528-050487d401a4
📒 Files selected for processing (2)
components/docs/Visualizer.tsxcomponents/layout/DocsLayout.tsx
|
@princerajpoot20 |
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
components/layout/DocsLayout.tsx (1)
127-127: ⚡ Quick winGate explorer UI using
post.sluginstead ofrouter.pathname
DocsLayoutalready receives the resolvedpost, and this codebase also usesrouter.pathnameto look up that same post by exact slug (getDocBySlug(..., pathname)), so the “route template vs concrete URL” pitfall is unlikely to break the current flow. Still, the explorer-specific branch atcomponents/layout/DocsLayout.tsx:127can be made more direct/robust by switching topost.slug.includes('-explorer')(orpost.slug.endsWith('-explorer')) rather thanrouter.pathname.includes(...).🤖 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 `@components/layout/DocsLayout.tsx` at line 127, The conditional in DocsLayout uses router.pathname.includes('-explorer') which can be fragile; switch the gate to inspect the resolved post slug instead (e.g., use post.slug.includes('-explorer') or post.slug.endsWith('-explorer')) so the explorer UI branch is determined from the actual post object passed into DocsLayout; update the conditional at the branch that currently checks router.pathname to use post.slug and keep behavior otherwise unchanged.
🤖 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.
Nitpick comments:
In `@components/layout/DocsLayout.tsx`:
- Line 127: The conditional in DocsLayout uses
router.pathname.includes('-explorer') which can be fragile; switch the gate to
inspect the resolved post slug instead (e.g., use
post.slug.includes('-explorer') or post.slug.endsWith('-explorer')) so the
explorer UI branch is determined from the actual post object passed into
DocsLayout; update the conditional at the branch that currently checks
router.pathname to use post.slug and keep behavior otherwise unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ca0f024f-1542-4bd6-aa3f-06897694fbe8
📒 Files selected for processing (1)
components/layout/DocsLayout.tsx
|
@Sourya07 please attach before and after both evidence so what we can easily compare. |
|
also let's take this PR after dark theme rollout planned by end of this month. otherwise we need to resolve conflicts |
|
okay got it 👍 |
Description
fixes -> #5107
before ->
as recommended by -> #5107 (comment)
current fix ->https://deploy-preview-5455--asyncapi-website.netlify.app/docs/reference/specification/v3.0.0-explorer
Summary by CodeRabbit
Bug Fixes
Enhancements