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

[Divider] Add aria role if it's not implicit #16256

Merged
merged 2 commits into from
Jun 17, 2019

Conversation

eps1lon
Copy link
Member

@eps1lon eps1lon commented Jun 16, 2019

Follow-up on #15339 (spotted due to line not being covered)

Setting the role only if a non-default component prop is passed follows the behavior in our Button.

Rules of aria attributes for hr elements

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

mui-pr-bot commented Jun 16, 2019

Details of bundle changes.

Comparing: f69797f...a21f225

bundle parsed diff gzip diff prev parsed current parsed prev gzip current gzip
@material-ui/core +0.01% 🔺 +0.01% 🔺 318,954 318,982 87,562 87,573
@material-ui/core/Paper 0.00% 0.00% 68,281 68,281 20,353 20,353
@material-ui/core/Paper.esm 0.00% 0.00% 61,578 61,578 19,133 19,133
@material-ui/core/Popper 0.00% 0.00% 28,968 28,968 10,411 10,411
@material-ui/core/Textarea 0.00% 0.00% 5,513 5,513 2,374 2,374
@material-ui/core/TrapFocus 0.00% 0.00% 3,755 3,755 1,580 1,580
@material-ui/core/styles/createMuiTheme 0.00% 0.00% 16,012 16,012 5,791 5,791
@material-ui/core/useMediaQuery 0.00% 0.00% 2,597 2,597 1,102 1,102
@material-ui/lab 0.00% 0.00% 140,580 140,580 43,454 43,454
@material-ui/styles 0.00% 0.00% 51,703 51,703 15,337 15,337
@material-ui/system 0.00% 0.00% 15,303 15,303 4,342 4,342
Button 0.00% 0.00% 84,279 84,279 25,630 25,630
Modal 0.00% 0.00% 20,345 20,345 6,689 6,689
Slider 0.00% 0.00% 74,681 74,681 23,221 23,221
colorManipulator 0.00% 0.00% 3,904 3,904 1,544 1,544
docs.landing 0.00% 0.00% 55,232 55,232 13,946 13,946
docs.main 0.00% +0.01% 🔺 651,023 651,051 205,042 205,054
packages/material-ui/build/umd/material-ui.production.min.js +0.01% 🔺 +0.01% 🔺 292,198 292,226 83,491 83,498

Generated by 🚫 dangerJS against a21f225

@eps1lon eps1lon marked this pull request as ready for review June 16, 2019 19:31
assert.strictEqual(wrapper.find('div').props().role, 'separator');
});

it('passed roles override', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm trying to think of a better way to word this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
it('passed roles override', () => {
test('passed roles override', () => {

?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess, or overrides the computed role with a provided one

@eps1lon eps1lon merged commit 545612c into mui:master Jun 17, 2019
@eps1lon eps1lon deleted the feat/Divider/roles branch June 17, 2019 08:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility a11y component: divider 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.

4 participants