Reintroduce EuiSwitch a11y changes#2491
Conversation
| const onClick = (e: React.MouseEvent<HTMLButtonElement>) => { | ||
| const event = (e as unknown) as EuiSwitchEvent; | ||
| event.target.checked = !checked; | ||
| onChange(event); | ||
| }; |
There was a problem hiding this comment.
Given that this is a breaking change even with this shim, do we still want to go this route?
I think it's fine if we want to decide that, I just didn't want this to slide in without being considered.
There was a problem hiding this comment.
I think the shim still has value. Others can chime in if they disagree, though.
There was a problem hiding this comment.
Agreed that there is still value in providing the checked state through the event's target.
chandlerprall
left a comment
chandlerprall
left a comment
There was a problem hiding this comment.
Changes LGTM, pulled and tested locally.
|
While doing some testing, I noticed that the So I did some more looking and it seems that Thoughts on reverting that change, @myasonik? |
|
Button is labelable but support for it is really poor. The label will be the only thing read by many screen readers and you won't get the button text. Sometimes you'll also lose the other button attributes as well (like, the We could get around it by adding a bunch of content to the |
4 participants
Summary
Closes #2385 and re-implements code reverted in #2255, resolving a11y issues with
EuiSwitchand converting the component to TypeScript.Added a label-less version, which uses
aria-label.Compressed and mini styles were added after the reversion, so those have been refactored.
The difficult work will come in a Kibana PR (already passing), although the addition of
typeon thebuttonremedied some major problems (why issubmitthe default, anyways?)Checklist
- [ ] Checked in dark mode- [ ] Checked in mobile- [ ] Checked in IE11 and Firefox- [ ] Added or updated jest tests- [ ] Checked for breaking changes and labeled appropriately- [ ] Checked for accessibility including keyboard-only and screenreader modes