fix(react-popover): prevent incorrect non-modal screen reader announcement#36383
Open
PaulGMardling wants to merge 14 commits into
Open
fix(react-popover): prevent incorrect non-modal screen reader announcement#36383PaulGMardling wants to merge 14 commits into
PaulGMardling wants to merge 14 commits into
Conversation
PaulGMardling
changed the title
📊 Bundle size report
Unchanged fixtures
|
|
Pull request demo site: URL |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -0,0 +1,7 @@ | |||
| { | |||
There was a problem hiding this comment.
🕵🏾♀️ visual changes to review in the Visual Change Report
vr-tests-react-components/Menu 1 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-react-components/Menu.Nested Submenus Small Viewport Stacked.nested menu.chromium.png | 856 | Changed |
vr-tests-react-components/Positioning 2 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-react-components/Positioning.Positioning end.updated 2 times.chromium.png | 618 | Changed |
| vr-tests-react-components/Positioning.Positioning end.chromium.png | 728 | Changed |
vr-tests-react-components/ProgressBar converged 3 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-react-components/ProgressBar converged.Indeterminate + thickness - Dark Mode.default.chromium.png | 51 | Changed |
| vr-tests-react-components/ProgressBar converged.Indeterminate + thickness.default.chromium.png | 105 | Changed |
| vr-tests-react-components/ProgressBar converged.Indeterminate + thickness - High Contrast.default.chromium.png | 114 | Changed |
vr-tests-react-components/TagPicker 2 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-react-components/TagPicker.disabled - Dark Mode.chromium.png | 658 | Changed |
| vr-tests-react-components/TagPicker.disabled.disabled input hover.chromium.png | 677 | Changed |
There were 2 duplicate changes discarded. Check the build logs for more information.
PaulGMardling
force-pushed
the
fix/a11y-popover-default
branch
from
July 14, 2026 10:15
eaac4f0 to
ad5e853
Compare
PaulGMardling
changed the title
PaulGMardling
force-pushed
the
fix/a11y-popover-default
branch
from
July 14, 2026 13:43
1a1be75 to
ed33516
Compare
PaulGMardling
marked this pull request as ready for review
July 14, 2026 14:35
Hotell
reviewed
Jul 14, 2026
PaulGMardling
force-pushed
the
fix/a11y-popover-default
branch
from
July 14, 2026 14:55
ed33516 to
a2e2d0e
Compare
PaulGMardling
force-pushed
the
fix/a11y-popover-default
branch
3 times, most recently
from
July 15, 2026 15:06
2357010 to
e0a2c69
Compare
mainframev
reviewed
Jul 17, 2026
…to fix Narrator style group announcement
…o focusable content + test coverage
…faceAriaAttributes - Remove isolated unit tests for getPopoverSurfaceAriaAttributes function - Keep component-level integration tests that verify aria attributes through rendered component - Update function comment to remove mention of isolated testing - Component logic is now tested as a whole rather than in isolation
…shaking - Change condition from 'process.env.NODE_ENV === production' to '!== production' - Move development-only warning logic inside the production check - Ensures entire dev warning useEffect is dropped in production builds - Follows Fluent UI pattern for dev-only code with guaranteed tree-shaking
- Replace double negation operator (!!) with explicit Boolean() call - Improves code readability and follows codebase conventions - No functional change, same type coercion behavior
- Remove nonInteractiveContentWarning constant from public API (constants.ts) - Inline warning message directly in console.warn() call in usePopover.ts - Removes logging noise from public API exports - Update tests to use inlined warning message instead of constant reference
PaulGMardling
force-pushed
the
fix/a11y-popover-default
branch
from
July 17, 2026 13:28
37dacef to
b02f3bb
Compare
mainframev
approved these changes
Jul 17, 2026
mainframev
reviewed
Jul 17, 2026
| @@ -0,0 +1,7 @@ | |||
| { | |||
| "type": "patch", | |||
| "comment": "omit role=group from PopoverSurface when no accessible name is provided, fixing Narrator announcing contains style group on non-modal popovers", | |||
Contributor
There was a problem hiding this comment.
Suggested change
| "comment": "omit role=group from PopoverSurface when no accessible name is provided, fixing Narrator announcing contains style group on non-modal popovers", | |
| "comment": "fix: omit group role from PopoverSurface when no accessible name is provided", |
mainframev
approved these changes
Jul 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes the incorrect Narrator announcement for non-modal Popover when content is non-interactive.
Previously, activating this scenario could produce:
contains style group
What Changed
PopoverSurface no longer applies role="group" unconditionally when trapFocus is false.
role="group" is now applied only when an accessible name is provided via aria-label or aria-labelledby, aligning with WAI-ARIA 1.2 requirements.
If no accessible name is provided, the surface renders as a plain div, so Narrator no longer announces contains style group.
Aria attribute logic was extracted into a pure function named getPopoverSurfaceAriaAttributes for isolated unit testing.
The default Popover story was updated to include aria-label as the recommended usage pattern.
Added a dev-only warning for non-modal popovers that open with non-interactive, non-focusable content to guide consumers toward accessible patterns.
Previous Behavior
On activating a popover with no interactive elements, Narrator announced:
contains style group
New Behavior
Narrator no longer announces contains style group for a non-modal popover surface without an accessible name.
Notes
This change resolves the incorrect announcement and adds consumer guidance for the non-interactive-content edge case.
Work Item
Fixes #18661