-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
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] Automatically label by its DialogTitle #26814
Conversation
@@ -225,6 +227,11 @@ const Dialog = React.forwardRef(function Dialog(inProps, ref) { | |||
} | |||
}; | |||
|
|||
const ariaLabelledby = useId(ariaLabelledbyProp); |
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.
What's the story for dialogs that don't render a <DialogTitle>
? For instance https://next.material-ui.com/components/dialogs/#full-screen-dialogs.
Does an aria-labelledby
pointing to a nonexisting DOM node create an a11y issue?
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.
What's the story for dialogs that don't render a ?
They are already problematic.
Does an aria-labelledby pointing to a non existing DOM node an a11y issue?
Nope. It will be treated just like it has no aria-labelledby
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.
Nope. It will be treated just like it has no aria-labelledby
Great, and we even have the optionality to have DialogContent host the label` if needed, in the future.
Removes the need to manually wire up Dialog and DialogTitle
Instead of
you can write
instead which automatically labels the
role="dialog"
with the contents ofDialogTitle
.