fix(navmenu): enable scrolling at 400% zoom for accessibility#2184
fix(navmenu): enable scrolling at 400% zoom for accessibility#2184omerbakr wants to merge 1 commit intoexpressjs:gh-pagesfrom
Conversation
Signed-off-by: omerbakr <y.omerbakir@gmail.com>
✅ Deploy Preview for expressjscom-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
@expressjs/docs-wg could someone please take a look at this? My schedule is pretty full with the redesign and other topics. Note: I don’t think we should spend more time fixing the current site’s issues. Let’s focus on making sure the redesign doesn’t have these problems and on launching it as soon as possible, rather than investing more time in the current site. |
| display: block; | ||
| max-height: calc(100dvh - var(--header-height)); | ||
| overflow-y: auto; | ||
| overscroll-behavior: contain; |
There was a problem hiding this comment.
| overscroll-behavior: contain; |
I think overscroll-behavior property is redundant because body get applied with no-scroll when menus open. But this property will stop outer body scroll if js will fail to add 'no-scroll' class.
@omerbakr WDYT? Should we keep it or not?
There was a problem hiding this comment.
You’re right, it works fine without it. I’m just +1 on keeping it as a tiny CSS safety net in case the no-scroll class doesn’t get applied - it’s only one line, so feels harmless to keep.
ShubhamOulkar
left a comment
There was a problem hiding this comment.
LGTM, design breaks after 220% zoom level and scroll appears after 220% break point. Ref bug fix

This change constrains the menu height relative to the fixed header using a CSS variable and enables vertical scrolling within the menu container. The layout and visual design remain unchanged.
Fixes #1731