[Emotion] Convert EuiCheckbox, EuiRadio, and EuiSwitch #7969

cee-chen · 2024-08-18T20:04:08Z

Summary

Just a small 1.5k diff PR, nothing to see here 🙃

I honestly gave up trying to make my commits as granular as I usually do - there was a lot of old CSS in these components and I just ended up essentially rewriting the styles with more modern day flex/CSS and so forth.

To be totally honest, unless we have people incredibly passionate about CSS on the team (which I'm pretty sure is just me at this point), I would recommend only QAing this PR for visual regressions/bugs and not attempting to code review. 🤷

Intentional changes - technical details

EuiCheckbox, EuiRadio, and EuiSwitch now use JSX <EuiIcon />s to render their various indicators (checked, indeterminate, selected, etc) rather than CSS data-uri SVGs
All labels for the above components now use our generic disabledText token as opposed to the custom form disabled text color. This is because these labels do not need the extra contrast from having a disabled background color.
Updated the rendered DOM of the above components (cleaning up extra divs, reordering now that we don't have to use + siblings selectors for CSS)
Simplified the color tokens/variables being used for switches greatly - there was a lot of custom tinting/shading and transprentizing going on which I tried to limit (aside from a token set specifically to meet WCAG AA requirements), in favor of using existing similar tokens
Fixed checkbox and radio inputs not quite lining up/being vertically centered with their labels (was 1px off on prod)
Deleted every single checkbox/radio/switch Sass variable/mixin, of which there were a lot 🙃

QA

General checklist

Browser QA
- Checked in both light and dark modes
- Checked in mobile
- Checked in Chrome, Safari, Edge, and Firefox
- Checked for accessibility including keyboard-only and screenreader modes
Docs site QA - N/A
Code quality checklist
- ~~Added or~~ updated jest ~~and cypress~~ tests
- Updated visual regression tests
Release checklist
- A changelog entry exists and is marked appropriately.
  ~~- [ ] If applicable, added the breaking change issue label (and filled out the breaking change checklist)~~
Designer checklist
- Updated EUI's Figma library with color token updates

- doesn't need to live in the main form vars fn, these variables are very specific to these components - simplify `customControlDisabledIconColor` to just darkShade - it's close enough and doesn't require extra computation

- more future proof for icon changes and generally less CSS - extra DOM is worth it, IMO

- remove old Emotion mixin, I'm makin' my own - note the different DOM rendering - now that we have `:has` CSS, we don't need to rely on flat `+` selectors - prefer flex for center alignment over absolute positioning

- literally just needs `border-radius: 50%` instead

+ inline conditional label - remove `--noLabel` modifier (replaced with `:has` CSS)

- inline conditional label, remove need for useMemo - inline the border-radius, it's only used here and doesn't need to be a shared var (it technically already is one)

+ misc cleanup while here - fragment syntax, test syntax

- a lot of colors are being simplified and slightly tweaked here to reduce transparency as well as one-off tinting/shading - remove Sass vars

- remove unnecessary min-height

- not sure if this is worth it, may revert

- move thumb to outside body since it has `overflow: hidden` and we want thumbs to be able to scale outside - use `aspect-ratio` CSS instead of static width/height that changes (instead inherits from the button which is already sized) - simplify `-1px` shenanigans by reducing transforms by default, store in thumbScales var + tweak hover/active states accordingly and DRY out a util for it

- simplify DOM - rename `__track` to `__icons`, feels more accurate - prefer flex and padding to center/position icons instead of positioning - no need for border-radius, `__body` already has it set + overflow hidden

+ update VRT to be more useful for permutation testing

+ fix broken EuiDataGrid drag/drop behavior - we can't use the scroll shadow transform Z workaround and drag/drop at the same time 🫠

- will no longer be a thing once the EuiFormRow PR merges, drop this commit then

tkajtoch

QA'd on desktop and mobile. Other than a couple of minor things I noticed the changes look great!

EuiSwitch rendered in EuiFormRows (see the second example) and EuiFormFieldset have now bigger margins applied and look wrong (?)
EuiCheckbox and EuiRadio have lost their regular EUI outlines when focused in light mode. EuiRadioGroup doesn't seem to have an outline when focused at all

cee-chen · 2024-08-19T17:23:36Z

EuiCheckbox and EuiRadio have lost their regular EUI outlines when focused in light mode. EuiRadioGroup doesn't seem to have an outline when focused at all

🙃 Oh that's fun, I totally forgot to cross-browser test in Chrome/Edge - they're fine in FF. Looking into this!

cee-chen · 2024-08-19T17:32:42Z

EuiSwitch rendered in EuiFormRows (see the second example) and EuiFormFieldset have now bigger margins applied and look wrong (?)

This will be fixed once #7968 merges in - it's wrong because of this temporary commit (3d407a7).

- Chrome/Edge's auto style defaults to currentColor which is the empty shade, so its invisible - Safari's is just godawful

cee-chen · 2024-08-19T18:15:15Z

I debugged the EuiFormFieldset one further and it looks like it was a very interesting/weird bug with inline-flex vs flex/block displays - this should be fixed now! Thanks so much for the thorough QA Tomasz!

tkajtoch

Looks and works beautifully! Thanks for quickly addressing my comments 💯

elasticmachine · 2024-08-19T22:47:29Z

💚 Build Succeeded

Buildkite Build
Commit: cd8b659

History

💚 Build #2611 succeeded 7cbbbb1
💚 Build #2601 succeeded 0795d3e
💔 Build #2600 failed 649e99f

`v95.7.0` ⏩ `v95.9.0` > [!CAUTION] > This PR contains the final set of Emotion conversions for all EuiForm components. > If your plugin contains any custom CSS/styling to **EuiFormRow, EuiFormControlLayout, EuiCheckbox, EuiRadio, or EuiSwitch**, ⚠️ make sure to QA your UI to ensure no visual regressions have occurred! ⚠️ --- ## [`v95.9.0`](https://github.com/elastic/eui/releases/v95.9.0) - Updated `EuiSearchBar`'s optional `box.schema` prop with a new `recognizedFields` configuration. This allows specifying the phrases that will be parsed as field clauses ([#7960](elastic/eui#7960)) - Updated `EuiIcon` with a new `tokenSemanticText` glyph ([#7971](elastic/eui#7971)) - Added support for TypeScript 5 ([#7980](elastic/eui#7980)) **Bug fixes** - Fixed `EuiSelectableTemplateSitewide` styles when used within a dark-themed `EuiHeader` ([#7977](elastic/eui#7977)) ## [`v95.8.0`](https://github.com/elastic/eui/releases/v95.8.0) - Updated `EuiHeaderLinks`'s mobile menu to set a slight popover padding by default ([#7961](elastic/eui#7961)) - This can be overridden via `popoverProps.panelPaddingSize` if needed. - Updated `EuiHeaderLink` to default to a size of `s` (down from `m`) ([#7961](elastic/eui#7961)) **Accessibility** - Updated the `aria-label` attribute for the `EuiFieldSearch` clear button ([#7970](elastic/eui#7970)) **Bug fixes** - Fixed a visual bug with `<EuiDualRange showInput="inputWithPopover" />` form controls ([#7957](elastic/eui#7957)) **Deprecations** - Deprecated `EuiFormRow`'s `columnCompressedSwitch` display prop. Use `columnCompressed` instead, which will automatically account for child `EuiSwitch`es ([#7968](elastic/eui#7968)) - Deprecated `EuiFormRow`'s `rowCompressed` display prop. Use `row` instead for vertical forms, or `centerCompressed` for inline forms ([#7968](elastic/eui#7968)) - (Styling) Updated `EuiFormRow`'s `hasEmptySpaceLabel` prop to no longer attempt to automatically align its content to a vertical center. Use the `display="center"` prop for that instead ([#7968](elastic/eui#7968)) **CSS-in-JS conversions** - Converted `EuiFormControlLayout` to Emotion ([#7954](elastic/eui#7954)) - Removed `.euiFormControlLayout--*icons` classNames and `--eui-form-control-layout-icons-padding` CSS var. Use `--euiFormControlRightIconsCount` or `--euiFormControlLeftIconsCount` instead - Converted `EuiFormLayoutDelimited` to Emotion ([#7957](elastic/eui#7957)) - Fixed `cloneElementWithCss` throwing an error when used multiple times without a `key` prop ([#7957](elastic/eui#7957)) - Updated `cloneElementWithCss` utility to support a third argument that allows prepending vs. appending the cloned Emotion css className ([#7957](elastic/eui#7957)) - Removed `@euiFormControlLayoutClearIcon` Sass mixin ([#7959](elastic/eui#7959)) - Converted `EuiDescribedFormGroup` to Emotion ([#7964](elastic/eui#7964)) - Converted `EuiForm`, `EuiFormHelpText`, and `EuiFormErrorText` to Emotion ([#7966](elastic/eui#7966)) - Converted `EuiFormLabel` and `EuiFormLegend` to Emotion; Removed `@euiFormLabel` mixin ([#7967](elastic/eui#7967)) - Converted `EuiFormRow` to Emotion ([#7968](elastic/eui#7968)) - Converted `EuiCheckbox` to Emotion ([#7969](elastic/eui#7969)) - Converted `EuiRadio` to Emotion ([#7969](elastic/eui#7969)) - Converted `EuiSwitch` to Emotion ([#7969](elastic/eui#7969)) - Removed the following Sass variables: ([#7969](elastic/eui#7969)) - `$euiFormCustomControlDisabledIconColor` - `$euiFormCustomControlBorderColor` - `$euiRadioSize` - `$euiCheckBoxSize` - `$euiCheckboxBorderRadius` - `$euiSwitchHeight` (and compressed/mini variants) - `$euiSwitchWidth` (and compressed/mini variants) - `$euiSwitchThumbSize` (and compressed/mini variants) - `$euiSwitchIconHeight` - `$euiSwitchOffColor` - Removed the following Sass mixins: ([#7969](elastic/eui#7969)) - `euiIconBackground` - `euiCustomControl` - `euiCustomControlSelected` - `euiCustomControlDisabled` - `euiCustomControlFocused` --------- Co-authored-by: Marta Bondyra <4283304+mbondyra@users.noreply.github.com>

cee-chen added 16 commits August 18, 2024 10:31

Convert form style variables to its own utility function

701ba82

- doesn't need to live in the main form vars fn, these variables are very specific to these components - simplify `customControlDisabledIconColor` to just darkShade - it's close enough and doesn't require extra computation

Replace @mixin euiIconBackground inline CSS SVGs with <EuiIcon />

c4e6bc8

- more future proof for icon changes and generally less CSS - extra DOM is worth it, IMO

Convert custom control mixins/styles

e676ad9

- remove old Emotion mixin, I'm makin' my own - note the different DOM rendering - now that we have `:has` CSS, we don't need to rely on flat `+` selectors - prefer flex for center alignment over absolute positioning

Remove unnecessary euiCustomControl usage in EuiRange

6ff66df

- literally just needs `border-radius: 50%` instead

[EuiRadio] Convert to use new form util

c716b95

+ inline conditional label - remove `--noLabel` modifier (replaced with `:has` CSS)

[EuiCheckbox] Convert base styles

7974fc0

- inline conditional label, remove need for useMemo - inline the border-radius, it's only used here and doesn't need to be a shared var (it technically already is one)

[EuiCheckbox] Convert readOnly styles

a64bbf4

+ misc cleanup while here - fragment syntax, test syntax

Update downstream snapshots

7dd90d8

[EuiSwitch] Set up style variables + mini size

f25d657

- a lot of colors are being simplified and slightly tweaked here to reduce transparency as well as one-off tinting/shading - remove Sass vars

[EuiSwitch] Convert wrapper/label styles

75b586e

- remove unnecessary min-height

[EuiSwitch] Convert button (height/width) and body (bg color) styles

02ce820

[opinionated] extra disabled light mode tweaking

20ee48a

- not sure if this is worth it, may revert

Restore thumb transition/animation movement

db328c0

[EuiSwitch] Convert check/cross icon styles

2fb2811

- simplify DOM - rename `__track` to `__icons`, feels more accurate - prefer flex and padding to center/position icons instead of positioning - no need for border-radius, `__body` already has it set + overflow hidden

[EuiSwitch] Update snapshots & screenshots

1aa72ab

+ update VRT to be more useful for permutation testing

cee-chen added the emotion label Aug 18, 2024

cee-chen added 4 commits August 18, 2024 19:31

[EuiSwitch] Update downstream snapshots/usages

3df9000

+ fix broken EuiDataGrid drag/drop behavior - we can't use the scroll shadow transform Z workaround and drag/drop at the same time 🫠

Delete Sass files

c48491f

[TEMP] Comment out Sass var usage

3d407a7

- will no longer be a thing once the EuiFormRow PR merges, drop this commit then

changelog

0795d3e

cee-chen force-pushed the emotion/custom-controls branch from 649e99f to 0795d3e Compare August 19, 2024 02:31

cee-chen marked this pull request as ready for review August 19, 2024 16:11

cee-chen requested a review from a team as a code owner August 19, 2024 16:11

JasonStoltz assigned JasonStoltz and unassigned JasonStoltz Aug 19, 2024

tkajtoch requested changes Aug 19, 2024

View reviewed changes

[PR feedback] Fix focus outline for webkit browsers

5abd42a

- Chrome/Edge's auto style defaults to currentColor which is the empty shade, so its invisible - Safari's is just godawful

cee-chen force-pushed the emotion/custom-controls branch from 4d3b9fa to 5abd42a Compare August 19, 2024 17:43

[PR feedback] Fix extra inline strut height on EuiSwitches

7cbbbb1

tkajtoch approved these changes Aug 19, 2024

View reviewed changes

Merge branch 'main' into emotion/custom-controls

cd8b659

cee-chen enabled auto-merge (squash) August 19, 2024 22:22

cee-chen merged commit e66230a into elastic:main Aug 19, 2024
5 checks passed

cee-chen deleted the emotion/custom-controls branch August 19, 2024 22:47

cee-chen mentioned this pull request Aug 20, 2024

Upgrade EUI to v95.9.0 elastic/kibana#190752

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Emotion] Convert EuiCheckbox, EuiRadio, and EuiSwitch #7969

[Emotion] Convert EuiCheckbox, EuiRadio, and EuiSwitch #7969

cee-chen commented Aug 18, 2024 •

edited

Loading

tkajtoch left a comment •

edited

Loading

cee-chen commented Aug 19, 2024 •

edited

Loading

cee-chen commented Aug 19, 2024

cee-chen commented Aug 19, 2024

tkajtoch left a comment

elasticmachine commented Aug 19, 2024

[Emotion] Convert EuiCheckbox, EuiRadio, and EuiSwitch #7969

[Emotion] Convert EuiCheckbox, EuiRadio, and EuiSwitch #7969

Conversation

cee-chen commented Aug 18, 2024 • edited Loading

Summary

Intentional changes - technical details

QA

General checklist

tkajtoch left a comment • edited Loading

Choose a reason for hiding this comment

cee-chen commented Aug 19, 2024 • edited Loading

cee-chen commented Aug 19, 2024

cee-chen commented Aug 19, 2024

tkajtoch left a comment

Choose a reason for hiding this comment

elasticmachine commented Aug 19, 2024

💚 Build Succeeded

History

cee-chen commented Aug 18, 2024 •

edited

Loading

tkajtoch left a comment •

edited

Loading

cee-chen commented Aug 19, 2024 •

edited

Loading