Skip to content

[ButtonBase] Fix accessibility#10434

Merged
oliviertassinari merged 1 commit into
mui:v1-betafrom
oliviertassinari:button-fix-accessibility
Feb 24, 2018
Merged

[ButtonBase] Fix accessibility#10434
oliviertassinari merged 1 commit into
mui:v1-betafrom
oliviertassinari:button-fix-accessibility

Conversation

@oliviertassinari
Copy link
Copy Markdown
Member

  • Fix a W3C issue on the list demo page.
  • Set role="button" on non <button> elements.
  • Handle the space key event.

Closes #10339

{[0, 1, 2, 3].map(value => (
<ListItem
key={value}
role={undefined}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could ListItem take care of this?

Copy link
Copy Markdown
Member Author

@oliviertassinari oliviertassinari Feb 24, 2018

Choose a reason for hiding this comment

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

It's specific to having the Checkbox inside. The warning is around this invalid DOM structure role=button > input. I'm generally against abstracting the i11y for anything that requires non trivial code. Non trivial code is increasing the bug surface area + the bundle size, for a relative slight added value.

Copy link
Copy Markdown
Member

@mbrookes mbrookes Feb 24, 2018

Choose a reason for hiding this comment

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

👍 Perhaps code comment then?

Copy link
Copy Markdown
Member Author

@oliviertassinari oliviertassinari Feb 24, 2018

Choose a reason for hiding this comment

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

I believe it's not important and that 98% of the people won't care. But why not. I couldn't see an easy way to document it.

@oliviertassinari oliviertassinari merged commit c9ce216 into mui:v1-beta Feb 24, 2018
@oliviertassinari oliviertassinari deleted the button-fix-accessibility branch February 24, 2018 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants