Skip to content
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

Popover: label missing from Popover.Root #2443

Open
simonxabris opened this issue Oct 10, 2023 · 1 comment
Open

Popover: label missing from Popover.Root #2443

simonxabris opened this issue Oct 10, 2023 · 1 comment
Labels
Package: react/popover Topic: Accessibility This issue has to do with accessibility Type: Enhancement Small enhancement to existing primitive/feature

Comments

@simonxabris
Copy link

Bug report

Current Behavior

The Popover implements the dialog pattern, but is missing one thing, the label on the dialog element itself.

The pattern states the following:

The dialog has either:
A value set for the aria-labelledby property that refers to a visible dialog title.
A label specified by aria-label.

But this does not seem to be implemented.

Expected behavior

There's an API which I can use the set a label for the element that has role="dialog" on it.

Reproducible example

I did not create a reproduction as you can just go to the docs page, open the popover, inspect the element and see that there's no aria-label or aria-labelledby attribute on it.

Suggested solution

I have two suggestions in mind.

  • Creating a Popover.Header element, that people can use to render whatever heading they want for their Popover and internally we can link to content of this element to the role="dialog" element by adding an id to it and using aria-labelledby.
  • Adding a label prop that accepts a string that we can put into the aria-label attribute. This solution could be useful when the popover does not have a clear title, but you still want to provide one for assistive technology users.

I suppose the first solution is more radix-y, but both could be implemented and the label prop can be an escape hatch.

If we can figure out what road to go down on, I'm happy to submit a PR with the preferred solution.

Your environment

It happens on every environment

@benoitgrelard
Copy link
Collaborator

You can already just pass an aria-label or aria-labelledby if you want, as every DOM props are accepted.
That said, seems like a decent suggestion to potentially add to the lib.

@benoitgrelard benoitgrelard added Topic: Accessibility This issue has to do with accessibility Package: react/popover Type: Enhancement Small enhancement to existing primitive/feature labels Mar 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: react/popover Topic: Accessibility This issue has to do with accessibility Type: Enhancement Small enhancement to existing primitive/feature
Projects
None yet
Development

No branches or pull requests

2 participants