search

react-bootstrap / react-overlays

Utilities for creating robust overlay components
https://react-bootstrap.github.io/react-overlays
MIT License
899 stars 222 forks source link

ARIA role="dialog" must have an accessible label #965

Closed dualcyclone closed 3 years ago

dualcyclone commented 3 years ago

Describe the bug

Related to https://github.com/react-bootstrap/react-overlays/issues/964

Adding any aria attributes to the Modal adds these to the child DOM element, which means the main parent element still does not have an accessible label.

To Reproduce

Create a Modal and add aria-label="test" to it:

import Modal from "react-bootstrap/Modal";

...
<Modal
  ...
  aria-label="test"
>
...

Observe that the aria-label added does not get added to the element with role="dialog", but instead onto its first child.

<div role="dialog" aria-modal="true">
    <div aria-label="test">
    ....
    </div>
</div>

Reproducible Example

Minimal example by using CodeSandbox.

Expected behavior

The main element with role="dialog" should have the aria attributes added to it.

Additional context

MDN - ARIA: dialog role

However, adding role="dialog" alone is not sufficient to make a dialog accessible. Additionally, the following needs to be done:

The dialog must be properly labeled

kyletsang commented 3 years ago

Did you mean to post this in the react-bootstrap project?

If you're using react-bootstrap, you can pass aria-labelledby to the Modal, which passes it to the role="dialog" element

dualcyclone commented 3 years ago

Did you mean to post this in the react-bootstrap project?

The example demonstrates this behaviour on react-bootstrap/Modal

If you're using react-bootstrap, you can pass aria-labelledby to the Modal, which passes it to the role="dialog" element

Yes, I should be able to use aria-label to label the model directly, if I do not have a DOM element that contains a label.

If it's possible to use aria-labelledby, it should also accept aria-label

As noted in this comment:

It's expected that you will add the aria-labelledby or other form of labelling.

However it seems it does not accept any other form of labelling

kyletsang commented 3 years ago

Ok sounds like you are not using react-overlays directly. The other comment by @jquense was in regards to using the Modal from react-overlays which does forward all props to the underlying element. I think there's some confusion here because this issue is posted in react-overlays project rather than react-bootstrap

dualcyclone commented 3 years ago

Apologies, I'll move the issue there