fix(landing): improve responsive sidebar on smaller screens#126
Conversation
Lock body scroll and close on Escape when the drawer is open, cap sidebar width on narrow viewports, hide the mobile backdrop on lg+, and fix header overlap with a flex-based navbar layout. Fixes SamXop123#24
|
@SamXop123 is attempting to deploy a commit to the Dot_NotSam's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe PR improves sidebar responsiveness by adding scroll lock, keyboard dismissal, and responsive auto-close behavior. App manages sidebar state with toggleSidebar and a closeSidebar helper, plus an effect that disables page scrolling and registers an Escape key handler. Sidebar receives closeSidebar and wires it to the backdrop and close button, while navigation items trigger closeSidebar on small screens via a conditional handleNavClick. ChangesSidebar Responsive Close Behavior
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 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)
⚔️ Resolve merge conflicts
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 |
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 `@landing/src/components/Sidebar.jsx`:
- Around line 23-31: Replace the current use of aria-hidden on the motion.aside
with the boolean inert prop tied to isSidebarOpen (e.g., inert={!isSidebarOpen})
so the closed sidebar subtree is removed from the accessibility tree and tab
order; remove aria-hidden from the motion.aside and keep role="dialog" and
aria-modal="true". Additionally, implement focus management when the dialog
opens/closes in the same component (motion.aside) by moving focus into the
dialog and trapping focus while isSidebarOpen is true (use a focus-trap utility
or a small hook to save/restore previously focused element and trap
tab/shift+tab inside the dialog). Ensure these changes reference the existing
isSidebarOpen state and motion.aside element so the inert prop and focus-trap
target the same DOM node.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3b31ff3c-9218-464b-b2cf-6ab684ac0c10
📒 Files selected for processing (2)
landing/src/App.jsxlanding/src/components/Sidebar.jsx
| <motion.aside | ||
| initial={false} | ||
| animate={{ x: isSidebarOpen ? 0 : "-100%" }} | ||
| transition={{ type: "spring", stiffness: 450, damping: 30 }} | ||
| className="flex flex-col h-screen lg:w-[22vw] md:w-[40vw] sm:w-[50vw] w-[75vw] fixed top-0 left-0 z-50 bg-[#050816]/95 backdrop-blur-2xl shadow-[10px_0_50px_rgba(0,0,0,0.8)] border-r border-white/[0.08] overflow-hidden" | ||
| role="dialog" | ||
| aria-modal="true" | ||
| aria-hidden={!isSidebarOpen} | ||
| className="fixed top-0 left-0 z-50 flex h-screen w-[min(85vw,20rem)] flex-col overflow-hidden border-r border-white/[0.08] bg-[#050816]/95 shadow-[10px_0_50px_rgba(0,0,0,0.8)] backdrop-blur-2xl sm:w-[min(70vw,18rem)] md:w-[min(50vw,16rem)] lg:w-[min(22vw,15rem)]" | ||
| > |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does React 19 support rendering the native inert attribute as a boolean prop on DOM elements?
💡 Result:
Yes, React 19 natively supports the inert attribute as a boolean prop [1][2]. In React 19, you can pass a boolean value to the inert prop (e.g.,
Citations:
- 1: React DOM: Support boolean values for
inertprop facebook/react#24730 - 2: https://github.com/facebook/react/blob/ee0855f427832e899767f7659c5289364218ab9e/CHANGELOG.md
- 3: https://stackoverflow.com/questions/72720469/error-when-using-inert-attribute-with-typescript
- 4: [DOM] Add support for the
inertattribute facebook/react#17157 - 5: React doesn't play nice with inert property WICG/inert#58
Fix off-screen sidebar accessibility by swapping aria-hidden for inert.
When the sidebar is closed it remains mounted and still tabbable; aria-hidden on a container with focusable descendants is also flagged by accessibility tooling. In React 19, inert is supported as a boolean prop on DOM elements, so use it to remove the subtree from the tab order/accessibility tree.
♿ Proposed fix using inert
transition={{ type: "spring", stiffness: 450, damping: 30 }}
role="dialog"
aria-modal="true"
- aria-hidden={!isSidebarOpen}
+ inert={!isSidebarOpen}
className="fixed top-0 left-0 z-50 flex h-screen w-[min(85vw,20rem)] flex-col overflow-hidden border-r border-white/[0.08] bg-[`#050816`]/95 shadow-[10px_0_50px_rgba(0,0,0,0.8)] backdrop-blur-2xl sm:w-[min(70vw,18rem)] md:w-[min(50vw,16rem)] lg:w-[min(22vw,15rem)]"Also, role="dialog" + aria-modal="true" implies focus should be moved into the dialog and trapped while open; that focus management isn’t present here.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <motion.aside | |
| initial={false} | |
| animate={{ x: isSidebarOpen ? 0 : "-100%" }} | |
| transition={{ type: "spring", stiffness: 450, damping: 30 }} | |
| className="flex flex-col h-screen lg:w-[22vw] md:w-[40vw] sm:w-[50vw] w-[75vw] fixed top-0 left-0 z-50 bg-[#050816]/95 backdrop-blur-2xl shadow-[10px_0_50px_rgba(0,0,0,0.8)] border-r border-white/[0.08] overflow-hidden" | |
| role="dialog" | |
| aria-modal="true" | |
| aria-hidden={!isSidebarOpen} | |
| className="fixed top-0 left-0 z-50 flex h-screen w-[min(85vw,20rem)] flex-col overflow-hidden border-r border-white/[0.08] bg-[#050816]/95 shadow-[10px_0_50px_rgba(0,0,0,0.8)] backdrop-blur-2xl sm:w-[min(70vw,18rem)] md:w-[min(50vw,16rem)] lg:w-[min(22vw,15rem)]" | |
| > | |
| <motion.aside | |
| initial={false} | |
| animate={{ x: isSidebarOpen ? 0 : "-100%" }} | |
| transition={{ type: "spring", stiffness: 450, damping: 30 }} | |
| role="dialog" | |
| aria-modal="true" | |
| inert={!isSidebarOpen} | |
| className="fixed top-0 left-0 z-50 flex h-screen w-[min(85vw,20rem)] flex-col overflow-hidden border-r border-white/[0.08] bg-[`#050816`]/95 shadow-[10px_0_50px_rgba(0,0,0,0.8)] backdrop-blur-2xl sm:w-[min(70vw,18rem)] md:w-[min(50vw,16rem)] lg:w-[min(22vw,15rem)]" | |
| > |
🤖 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 `@landing/src/components/Sidebar.jsx` around lines 23 - 31, Replace the current
use of aria-hidden on the motion.aside with the boolean inert prop tied to
isSidebarOpen (e.g., inert={!isSidebarOpen}) so the closed sidebar subtree is
removed from the accessibility tree and tab order; remove aria-hidden from the
motion.aside and keep role="dialog" and aria-modal="true". Additionally,
implement focus management when the dialog opens/closes in the same component
(motion.aside) by moving focus into the dialog and trapping focus while
isSidebarOpen is true (use a focus-trap utility or a small hook to save/restore
previously focused element and trap tab/shift+tab inside the dialog). Ensure
these changes reference the existing isSidebarOpen state and motion.aside
element so the inert prop and focus-trap target the same DOM node.
Summary
Fixes #24
Test plan
Made with Cursor
Summary by CodeRabbit
Release Notes