Fix the mobile menu and prevent the page scrolling when the mobile menu open #392
Conversation
|
Hi @Swiftb0y |
acolombier
left a comment
acolombier
left a comment
There was a problem hiding this comment.
Hi @Deepak-cell311, thanks for your contribution!
Could you clarify what does your change? I can see it has extended to menu drawer to fill the window, is there something else this is intending to fix?
(Before/after video)
Screencast.From.2025-05-16.09-18-10.mp4
theme/static/css/custom.css
Outdated
| --button-primary-color: white; | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
nit: unnecessary change
theme/static/css/custom.css
Outdated
| border-bottom: 1px solid var(--border-color-softer); | ||
| padding: 2rem; | ||
| max-height: unset; | ||
|
|
There was a problem hiding this comment.
nit: unnecessary change
theme/static/css/custom.css
Outdated
| /* height: 100vh; */ | ||
|
|
There was a problem hiding this comment.
nit: comment cruft
| /* height: 100vh; */ |
theme/static/css/custom.css
Outdated
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
nit: unnecessary spaces
theme/templates/base.html
Outdated
| const navToggle = document.getElementById('navbar-hamburger-button'); | ||
|
|
||
| navToggle.addEventListener('change', () => { | ||
| if (navToggle.checked) { | ||
| document.body.classList.add('scroll-lock'); | ||
| } else { | ||
| document.body.classList.remove('scroll-lock'); | ||
| } | ||
| }); |
There was a problem hiding this comment.
#navbar-hamburger-button is a hidden element used to persist the expected state of the menu AFAIU, so that scroll-lock won't have any effect.
|
Hi @acolombier, Thank you for the review and helpful feedback. In my previous implementation, I used the hidden checkbox input (#navbar-hamburger-button) to detect when the mobile menu was toggled and then added a scroll-lock class on the to prevent the background from scrolling. However, as you rightly pointed out, since the input is visually hidden and used only to preserve state, it doesn’t reliably control page scrolling across all browsers and screen readers. As a result, the page behind the open menu remained scrollable in some environments, which wasn’t the expected behavior. It only works on my local machine. In my current approach, I've updated the structure to wrap the entire page content inside a #page-wrapper element. When the mobile menu is opened, the scroll-lock class is now applied to #page-wrapper instead of body. This gives more precise control over what gets locked and ensures the background remains fixed while the menu is open. I've also updated the CSS accordingly to disable overflow and enforce fixed height: This approach keeps the scroll-lock behavior isolated, avoids unintended side effects, and works consistently across devices. Let me know if you’d like me to further tweak or simplify anything. Thanks again! |
2 participants
Closes #262