-
Notifications
You must be signed in to change notification settings - Fork 303
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
Dialog component: Publish a11y docs on Primer.style #772
base: main
Are you sure you want to change the base?
Conversation
Added modifications per https://github.com/github/primer/issues/3234
|
||
The React version of the component generates a `<div>` element with `role="dialog"` and `aria-modal="true"`. | ||
|
||
When provided, the `title` prop for the component is rendered as an `<h1>` heading, and is explicitly referenced by the dialog using `aria-labelledby` to provide an accessible name for the dialog. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it always supposed to be a <h1>
? Or would there be a case where a lower heading is appropriate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a properly implemented dialog creates, in essence, its own top-level document (as anything outside of the dialog is hidden). so it generally would make sense that the top-most heading inside the dialog is an <h1>
, it may not necessarily be a failure if the heading structure inside the dialog doesn't start with <h1>
, as long as the order is still hierarchically correct. but it would be odd, if nothing else, if inside the dialog the first heading a screen reader user encounters is an <h3>
or something.
is this purely about styling, which by all means can be overridden, or are there other use cases you're thinking of where there might be a valid reason not to use <h1>
?
Added modifications per https://github.com/github/primer/issues/3234