[EuiCheckbox][EuiRadio] Fix checkboxes/radios without labels not being clickable#5149
[EuiCheckbox][EuiRadio] Fix checkboxes/radios without labels not being clickable#5149cee-chen wants to merge 3 commits intoelastic:masterfrom
Conversation
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_5149/ |
| opacity: 0; /* 1 */ | ||
| z-index: 1; /* 1 */ | ||
| margin: 0; /* 1 */ | ||
| left: 0; /* 1 */ |
There was a problem hiding this comment.
If you check out the "Before" screenshot, left: 0 was presumably added to override left: -10000px being set by euiScreenReaderOnly CSS. If you remove the CSS from being loaded at all on this input, there's no need to set left: 0 - position absolute will typically float the element where it would originally have been placed
thompsongl
left a comment
There was a problem hiding this comment.
LGTM
Also tested the other "checkable" components that use the mixin (EuiKeyPadMenuItem, EuiResizableCollapseButton) and saw no regression
|
@thompsongl Apologies for any wasted time on your part, @cchaos pinged me to let me know she'd rather revert #5130 for the next patch release. I'm going to close this PR, open a reversion PR, and then open a PR with #5130 plus this fix with a target release of the next breaking major (38.x). CC @chandlerprall as a heads up |
Summary
closes #5145 (NB: This bug only exists in 37.6.1, and should be fixed in 37.6.2)
#5130 updated the
euiScreenReaderOnlymixin (whichEuiCheckboxandEuiRadioboth use) to includeclip(0 0 0 0)CSS (to prevent scrolling issues caused by absolute positioning). Per the QA section of that PR, I tested the change against the normal checkbox/radio examples in our docs, but did not test them against checkbox/radios with no labels (which I hadn't realized functioned slightly differently than their counterparts with labels).Checkboxes and radios without labels actually do not want the standard
euiScreenReaderOnlybehavior. The visually hidden input actually overlays the visually displayed element with a set height/width, unlike SR behavior which moves the hidden element totally offscreen.As such, I decided the cleanest solution would be to only apply the
euiScreenReaderOnlyCSS to checkboxes/radios with labels (using:not()), thus negating the need for checkbox/radios to override any partseuiScreenReaderOnlyCSS.Before
After
Checkboxes/radios with labels remain the same:
QA
Labelinput. Confirm the inputs are still interactable (NB: the checkbox playground doesn't change state, but the radio playground does)Checklist
- [ ] Check against all themes for compatibility in both light and dark modes- [ ] Checked in mobile- [ ] Props have proper autodocs and playground toggles- [ ] Added documentation- [ ] Checked Code Sandbox works for the any docs examples- [ ] Added or updated jest tests- [ ] Checked for breaking changes and labeled appropriately