Skip to content

fix: Overlay NavDrawer needs to preserve dialog role #34942

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

smhigley
Copy link
Contributor

Previous Behavior

The overlay variant of NavDrawer overrode the underlying Drawer's role=dialog, which is required for a lot of built-in screen reader behavior around virtual cursor behavior and setting user expectations for being in a modal dialog. Without the role, aria-modal=true also doesn't really do anything.

New Behavior

NavDrawer keeps role=dialog, and the role=navigation is moved to NavDrawerBody. While that means the header is no longer part of the navigation region, that seems generally fine to me.

Related Issue(s)

None, noticed this while working on a different FAI bug.

@smhigley smhigley requested review from marcosmoura and a team as code owners July 29, 2025 21:20
@smhigley smhigley requested a review from mltejera July 29, 2025 21:21
Copy link

📊 Bundle size report

Package & Exports Baseline (minified/GZIP) PR Change
react-components
react-components: entire library
1.274 MB
321.042 kB
1.274 MB
321.042 kB
5 B
Unchanged fixtures
Package & Exports Size (minified/GZIP)
react-components
react-components: Button, FluentProvider & webLightTheme
68.79 kB
19.902 kB
react-components
react-components: Accordion, Button, FluentProvider, Image, Menu, Popover
235.675 kB
68.193 kB
react-components
react-components: FluentProvider & webLightTheme
43.624 kB
14.255 kB
react-portal-compat
PortalCompatProvider
8.386 kB
2.624 kB
react-timepicker-compat
TimePicker
109.052 kB
36.053 kB
🤖 This report was generated against ea2fbd636af8cd5d6b78448c595f9d102ca8a1f9

Copy link

Pull request demo site: URL

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant