-
-
Notifications
You must be signed in to change notification settings - Fork 8.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(theme): only render secondaryMenu if it should be shown #10705
Conversation
⚡️ Lighthouse report for the deploy preview of this PR
|
fixes facebook#10704 Issue was that with keyboard, you could reach the primary menu, while it was made invisible by CSS. By not rendering secondaryMenu based on the secondaryMenuShown logic, , we make sure it isn't accessible by keyboard while it is hidden.
✅ [V2]
To edit notification comments on pull requests, go to your Netlify site configuration. |
Unfortunately your solution breaks the animated transition we have between drawer panes, notably when pressing the back button Instead I'm proposing to use HTML
There are probably many other places we should use this attribute now that it's widely supported. |
Thanks for checking and fixing!Note, that in your solution, only the menu is inerted, and not ‘everything but the dialog’ as the dialog with showModal would have done.Anyway, this improves it a bunch already, thank you!On 27 Feb 2025, at 16:31, Sébastien Lorber ***@***.***> wrote:
Unfortunately your solution breaks the animated transition we have between drawer panes, notably when pressing the back button
CleanShot.2025-02-27.at.15.58.43.png (view on web)
Instead I'm proposing to use HTML inert that looks like a better option for this use case: https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/inert
The inert attribute can also be added to elements that should be offscreen or hidden. An inert element, along with its descendants, gets removed from the tab order and accessibility tree.
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: ***@***.***>
slorber left a comment (facebook/docusaurus#10705)
Unfortunately your solution breaks the animated transition we have between drawer panes, notably when pressing the back button
CleanShot.2025-02-27.at.15.58.43.png (view on web)
Instead I'm proposing to use HTML inert that looks like a better option for this use case: https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/inert
The inert attribute can also be added to elements that should be offscreen or hidden. An inert element, along with its descendants, gets removed from the tab order and accessibility tree.
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: ***@***.***>
|
Yes, but there's a separate PR for that you opened here :) I'm open to merging it too if it doesn't break our existing UX |
thank you, will give that a go in #10706 |
fixes #10704
Issue was that with keyboard, you could reach the primary menu, while it was made invisible by CSS. By not rendering secondaryMenu based on the secondaryMenuShown logic, , we make sure it isn't accessible by keyboard while it is hidden.
Pre-flight checklist
Motivation
Test Plan
Test links
Deploy preview: https://deploy-preview-_____--docusaurus-2.netlify.app/
Related issues/PRs