-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
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
[material-ui][Divider] Add aria-orientation #43241
Conversation
Netlify deploy preview
Bundle size reportDetails of bundle changes (Toolpad) |
433ec05
to
cde7379
Compare
docs/data/material/components/dividers/VerticalDividerMiddle.js
Outdated
Show resolved
Hide resolved
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.
I took the opportunity to improve related tests by using semantic queries whenever possible.
@DiegoAndai I'll add a section in the v6 upgrade guide if we agree on it being a breaking change. |
👍🏼 let's do it |
docs/data/material/components/dividers/VerticalDividerMiddle.js
Outdated
Show resolved
Hide resolved
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.
One final request for this is to add a note to https://mui.com/material-ui/react-divider/#orientation that the underlying DOM changes to div
to follow the accessibility guideline.
@siriwatknp I added a note as you suggested: |
@DiegoAndai I added an entry in the v6 upgrade guide. This is now ready for a final review. |
docs/data/material/migration/migrating-to-v6/migrating-to-v6.md
Outdated
Show resolved
Hide resolved
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.
👍 Awesome
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.
I have a small request for besides that RTM.
Resolves #43275
Modifies the
Divider
component to render a<div role="separator" aria-orientation="vertical">
instead of<hr>
when using vertical orientation to adhere to the ARIA spec.This can be considered a breaking change since consumers might be targeting
hr
elements for styling purposes.Benchmarks