fix: Resolve sidebar interaction and scroll issues #194
Conversation
This commit fixes multiple UX issues with the mobile/tablet sidebar: Issues Fixed: - Background interaction bleed-through: Users could click/interact with main content while sidebar was open - Missing scroll lock: Background content could scroll while sidebar was active - Non-scrollable sidebar: Drawer content would be cut off if it exceeded viewport height - Accessibility improvements: Added proper ARIA labels Changes Made: 1. Added body scroll lock using useEffect hook - Locks scroll when sidebar opens with overflow:hidden and position:fixed - Preserves and restores scroll position on close - Prevents background scrolling completely 2. Enhanced overlay/scrim interaction blocking - Added touch-none class to prevent touch events - Added explicit pointerEvents: 'auto' for better blocking - Added aria-hidden for accessibility 3. Made sidebar content scrollable - Restructured sidebar with flexbox (flex flex-col) - Header section: flex-shrink-0 (fixed) - Content area: flex-1 overflow-y-auto (scrollable) 4. Improved accessibility - Added aria-label to close button - Added aria-hidden to overlay Testing: - Verified background clicks are blocked (0 interactions) - Verified scroll lock works (0px scroll achieved) - Verified sidebar content is scrollable - Verified overlay closes sidebar on click Resolves issues with sidebar UX and interaction blocking.
WalkthroughThe Header component now includes enhanced mobile sidebar interactions: scroll-lock when the sidebar is open, an overlay backdrop to block page interaction, a dedicated close button, and restructured sidebar layout with separate header and scrollable content areas. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
client/app/components/Header/Header.tsx(5 hunks)
🔇 Additional comments (2)
client/app/components/Header/Header.tsx (2)
139-148: Effective overlay implementation for blocking background interaction.The combination of
touch-none, explicitpointerEvents: 'auto', andaria-hidden="true"properly prevents background interaction while maintaining accessibility. The overlay correctly closes the sidebar on click.
150-187: Well-structured scrollable sidebar implementation.The flexbox layout with
flex-shrink-0header andflex-1 overflow-y-autocontent area correctly enables scrolling when sidebar content exceeds viewport height. The close button'saria-labelenhances accessibility.
| // Lock body scroll when sidebar is open | ||
| useEffect(() => { | ||
| if (isSidebarOpen) { | ||
| // Save current scroll position | ||
| const scrollY = window.scrollY; | ||
|
|
||
| // Lock scroll | ||
| document.body.style.overflow = 'hidden'; | ||
| document.body.style.position = 'fixed'; | ||
| document.body.style.top = `-${scrollY}px`; | ||
| document.body.style.width = '100%'; | ||
| } else { | ||
| // Restore scroll | ||
| const scrollY = document.body.style.top; | ||
| document.body.style.overflow = ''; | ||
| document.body.style.position = ''; | ||
| document.body.style.top = ''; | ||
| document.body.style.width = ''; | ||
|
|
||
| // Restore scroll position | ||
| if (scrollY) { | ||
| window.scrollTo(0, parseInt(scrollY || '0') * -1); | ||
| } | ||
| } | ||
|
|
||
| // Cleanup on unmount | ||
| return () => { | ||
| document.body.style.overflow = ''; | ||
| document.body.style.position = ''; | ||
| document.body.style.top = ''; | ||
| document.body.style.width = ''; | ||
| }; | ||
| }, [isSidebarOpen]); |
There was a problem hiding this comment.
Restore scroll position on unmount to handle edge cases.
The scroll lock implementation correctly handles the open/close flow, but the cleanup function (lines 55-60) doesn't restore the scroll position if the component unmounts while the sidebar is open. This could leave users at scroll position 0 during navigation or hot reloads.
Consider capturing scrollY in the effect and restoring it in the cleanup:
// Lock body scroll when sidebar is open
useEffect(() => {
if (isSidebarOpen) {
// Save current scroll position
const scrollY = window.scrollY;
// Lock scroll
document.body.style.overflow = 'hidden';
document.body.style.position = 'fixed';
document.body.style.top = `-${scrollY}px`;
document.body.style.width = '100%';
- } else {
- // Restore scroll
- const scrollY = document.body.style.top;
- document.body.style.overflow = '';
- document.body.style.position = '';
- document.body.style.top = '';
- document.body.style.width = '';
-
- // Restore scroll position
- if (scrollY) {
- window.scrollTo(0, parseInt(scrollY || '0') * -1);
- }
- }
-
- // Cleanup on unmount
- return () => {
- document.body.style.overflow = '';
- document.body.style.position = '';
- document.body.style.top = '';
- document.body.style.width = '';
- };
+
+ // Cleanup: restore on close or unmount
+ return () => {
+ document.body.style.overflow = '';
+ document.body.style.position = '';
+ document.body.style.top = '';
+ document.body.style.width = '';
+ window.scrollTo(0, scrollY);
+ };
+ }
}, [isSidebarOpen]);🤖 Prompt for AI Agents
In client/app/components/Header/Header.tsx around lines 29 to 61, the cleanup
currently resets body styles but does not restore the saved scroll position if
the component unmounts while the sidebar is open; capture the scrollY value when
locking scroll (e.g., store it in a ref or a local variable closed over by the
cleanup) and in the cleanup restore window.scrollTo using that saved value
(convert the stored negative top back to a positive Y), and ensure the
open/close branch also uses the same stored value so both unmount and
sidebar-close restore the original scroll position and clear body styles.
2 participants
This commit fixes multiple UX issues with the mobile/tablet sidebar:
Issues Fixed:
Changes Made:
Added body scroll lock using useEffect hook
Enhanced overlay/scrim interaction blocking
Made sidebar content scrollable
Improved accessibility
Testing:
Resolves issues with sidebar UX and interaction blocking.
Description
Include a summary of the change and which issue is fixed. List any dependencies that are required for this change.
Fixes # (issue)
Type of change
Please mark the options that are relevant.
Checklist:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.