fix(docs): keep the sidebar pinned on click, and one scroll bar for every scroll region - #159
Merged
Merged
Conversation
The sidebar is sticky under a header and banner that scroll away, so a reader reaches its lower groups by scrolling the window until the nav pins. Next's scroll-to-top on navigation then dropped the nav back under the banner and pushed those groups below the fold. Sidebar rows now opt out of that scroll, and a click handler clamps the window to the shell's top before the route changes, so the sidebar stays where it was and the new page still opens at its breadcrumbs. A Playwright spec covers both paths. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Contributor
|
React Doctor found no new issues. 🎉 Reviewed by React Doctor for commit |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Limit details: You’ve used the included review currently available. 📝 SummarySummary by CodeRabbit
WalkthroughDocumentation scrolling now uses a shared ChangesDocumentation scroll areas
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to Documentation navigation now preserves the sidebar position and avoids unwanted page scroll resets, with coverage for navigation from scrolled states. No actionable merge-blocking risk is identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Warning Some tools did not complete. Review the errors below. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Usage-based review receipt
Note This review was completed with usage-based billing: files reviewed beyond your plan's included limits are billed at $0.25/file. View usage-based billing. Comment |
One shared ScrollArea now owns the thin fading bar the control panel introduced, with a horizontal orientation for code. The docs sidebar, the control panel, the Usage and props-table code blocks, the guides' fenced code, the search results, and the guides' table of contents all scroll through it. Shiki's unconditional tabindex comes off its <pre>, since the viewport is already a tab stop whenever the code overflows. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
hunterbecton
changed the title
scrollbar-width and scrollbar-color on the root give the page's own bar the gray-600 thumb on a transparent track that the inner scroll areas draw, so every bar on the site matches. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
React Doctor flagged the click handler on the nav as a handler on a non-interactive element with no keyboard counterpart. Each row's Link now carries it instead, which also drops the target checks the delegation needed. Enter on a focused row fires a click event, so keyboard readers get the same pin. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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
1 participant
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.
Why
Scrolling the components sidebar to its last group and clicking a row opened the new page with the sidebar back at the top (SHA-130). While in there, every scroll bar on the site now matches the thin one the control panel draws.
What changes
The sidebar pins instead of dropping back under the banner
The nav survives navigation and keeps its inner scroll. What moves is the window. The sidebar is sticky under 200px of header and banner that scroll away, so a reader reaches its lower groups by scrolling the window until the nav pins, and Next's scroll-to-top on navigation drops it back down and pushes those groups below the fold. A Playwright probe showed the clicked row off screen at every viewport height below 1329px.
Sidebar rows now pass
scroll={false}, and a click handler on the nav clamps the window to the shell's top before the router swaps the page. A reader who was past the banner lands with the sidebar exactly where it was and the new page's breadcrumbs, title, and demo at the top. A reader who was not sees nothing move. The clamp runs on click rather than after the route change so back and forward keep the browser's own scroll restoration.One scroll bar for every scroll region
components/scroll-areawraps Base UI's Scroll Area and owns the bar and thumb styles the control panel had, plus a horizontal orientation. The control panel, the sidebar, the Usage and props-table code blocks, the guides' fenced code, the search results, and the guides' table of contents all scroll through it. Shiki's unconditionaltabindexcomes off its<pre>, because the viewport is already a tab stop whenever the code overflows. The window's own bar gets the same thumb throughscrollbar-widthandscrollbar-coloron the root.A Playwright spec for both sidebar paths
apps/docs-tests/docs/sidebar-scroll.spec.tsreproduces the bug at a 720px viewport and checks that a click from deep in a page still opens the new page at its top. Red before the fix, green after, and the a11y suite stays green.Known limitations
When the reader was already past the banner, the new page opens with the banner scrolled off, which is what the page looked like before the click. Bringing the banner back would mean a sticky header, a design change rather than a bug fix.
🤖 Generated with Claude Code