Skip to content

docs: minor story-only accessibility fixes in v9 stories#36406

Open
smhigley wants to merge 1 commit into
microsoft:masterfrom
smhigley:story-a11y-fixes
Open

docs: minor story-only accessibility fixes in v9 stories#36406
smhigley wants to merge 1 commit into
microsoft:masterfrom
smhigley:story-a11y-fixes

Conversation

@smhigley

Copy link
Copy Markdown
Contributor

This fixes three issues:

  1. Popovers with no inner focusables that set focus to the popover surface didn't have an accessible name defined for the surface (this is only necessary if it gets focus, so it's not added for examples that set focus to inner buttons)
  2. There was one dialog example that wasn't correctly returning focus
  3. Some carousel examples had content get cut off at the top & bottom in high zoom since the height was set with px. Now the min-height is set, which allows the slides to grow if necessary without impacting the appearance at larger widths.

Fixes ADO issues:

@smhigley
smhigley requested review from a team and Mitch-At-Work as code owners July 17, 2026 08:10
@github-actions

Copy link
Copy Markdown

📊 Bundle size report

✅ No changes found

@github-actions

Copy link
Copy Markdown

Pull request demo site: URL

@PaulGMardling

Copy link
Copy Markdown
Contributor

a follow up issue has been created
#36421

@PaulGMardling PaulGMardling left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving because these changes address the accessibility issues reported in the linked ADOs for the affected Storybook examples.

This resolves the audited scenarios by ensuring that focusable PopoverSurface elements have accessible names.

However, it does not change the underlying PopoverSurface behavior: non-modal surfaces still receive role="group" by default, regardless of whether their content represents a logical group of UI objects.

@bsunderhus

Copy link
Copy Markdown
Contributor

@smhigley is this a doc fix for the conclusion of this PR not being merged? #36383

@smhigley

Copy link
Copy Markdown
Contributor Author

hm, sort of. The changes in that PR are not ones I'd recommend -- if a Popover isn't named, that doesn't make it a dialog, and group is still a more appropriate role. The dialog role has an impact on screen reader virtual cursor interactions and voice control overlays as well, and we wouldn't want that for popovers by default. (The impact I'm talking about is solely from the role, not from other methods of making the background inert or unfocusable).

Additionally, the dialog role also wouldn't fix the root issue in all screen readers. The problem is for a popover (or dialog) with no focusable content to reliably read its content when opened for screen reader users, it needs to both set focus to the PopoverSurface and have an accessible name defined. It's hard for us to handle that out of the box though, since we'd need to do something iffy like try to query for focusable descendants and also guess at what the accName should be. There are multiple accName approaches that are either bad or good depending on the length and type of popover content, so really a case-by-case authoring concern 😬

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.

3 participants