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

[Dialog] Automatically label by its DialogTitle #26814

Merged
merged 5 commits into from
Jun 24, 2021

Conversation

eps1lon
Copy link
Member

@eps1lon eps1lon commented Jun 18, 2021

Removes the need to manually wire up Dialog and DialogTitle

Instead of

<Dialog aria-labelledby={id}>
  <DialogTitle id={id} />
</Dialog>

you can write

<Dialog>
  <DialogTitle />
</Dialog>

instead which automatically labels the role="dialog" with the contents of DialogTitle.

@eps1lon eps1lon added accessibility a11y component: dialog This is the name of the generic UI component, not the React module! labels Jun 18, 2021
@mui-pr-bot
Copy link

mui-pr-bot commented Jun 18, 2021

Details of bundle changes (experimental)

Generated by 🚫 dangerJS against 1f496e2

@eps1lon eps1lon marked this pull request as ready for review June 18, 2021 20:01
@eps1lon eps1lon changed the title [Dialog] Automatically label by it by its DialogTitle [Dialog] Automatically label by its DialogTitle Jun 19, 2021
@@ -225,6 +227,11 @@ const Dialog = React.forwardRef(function Dialog(inProps, ref) {
}
};

const ariaLabelledby = useId(ariaLabelledbyProp);
Copy link
Member

@oliviertassinari oliviertassinari Jun 19, 2021

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?

Copy link
Member Author

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

Copy link
Member

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.

@eps1lon eps1lon requested a review from oliviertassinari June 20, 2021 16:27
@oliviertassinari oliviertassinari deleted the branch mui:next June 22, 2021 00:28
@oliviertassinari oliviertassinari changed the base branch from foo to next June 22, 2021 00:42
@eps1lon eps1lon merged commit e220945 into mui:next Jun 24, 2021
@eps1lon eps1lon deleted the feat/Dialog/labeling branch June 24, 2021 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility a11y component: dialog This is the name of the generic UI component, not the React module!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants