[Emotion] Convert EuiFormControlLayoutDelimited (2/3) #7957

cee-chen · 2024-08-12T21:11:47Z

Summary

Follow up to #7954

As always, please follow along by commit - this PR also contains a few necessary enhancements/fixes to EuiFormControlLayout and cloneElementWithCss.

It also contains several updates/fixes to EuiDualRange, EuiDatePickerRange and EuiSuperDatePicker, as those 3 components are the main dogfooders of EuiFormControlLayoutDelimited.

QA

Confirm the below links look the same as production:

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 - N/A

Emotion checklist

General

Output CSS matches the previous CSS (works as expected in all browsers)
Rendered className(s) read as expected in snapshots and browsers
~~[ ] Checked component playground~~

Unit tests

shouldRenderCustomStyles() test was added and passes with parent component and any nested childProps (e.g. tooltipProps)
~~[ ] Converted Enzyme to RTL~~

Sass/Emotion conversion process

Removed component from src/components/index.scss
Deleted any src/amsterdam/overrides/{component}.scss files (styles within should have been converted to the baseline Emotion styles)
Converted all global Sass vars/mixins to JS (e.g. $euiSize to euiTheme.size.base)
~~[ ] Listed var/mixin removals in changelog~~
~~[ ] Added an @warn deprecation message within the global_styling/mixins/{component}.scss file~~
~~[ ] Removed or converted component-specific Sass vars/mixins to exported JS versions~~
~~[ ] Ran yarn compile-scss to update var/mixin JSON files~~
~~[ ] Simplified calc() to mathWithUnits if possible (if mixing different unit types, this may not be possible)~~

CSS tech debt

~~[ ] Wrapped all animations or transitions in euiCanAnimate~~
~~[ ] Used gap property to add margin between items if using flex~~
Converted side specific padding, margin, and position to -inline and -block logical properties (check inline styles as well as CSS)

DOM Cleanup

Did NOT remove any block/element classNames (e.g. euiComponent, euiComponent__child)
SEARCH KIBANA FIRST: Deleted any modifier classNames or maps if not being used in Kibana.

Kibana due diligence

Search Kibana's codebase for {euiComponent}- (case sensitive) to check for usage of modifier classes
- ~~[ ] If usage exists, consider converting to a data attribute so that consumers still have something to hook into~~
Pre-emptively check how your conversion will impact the next Kibana upgrade. This entails searching/grepping through Kibana (excluding **/target, **/*.snap, **/*.storyshot for less noise) for eui{Component} (case sensitive) to find:
~~[ ] Any test/query selectors that will need to be updated~~
~~[ ] Any Sass or CSS that will need to be updated, particularly if a component Sass var was deleted~~
~~[ ] Any direct className usages that will need to be refactored (e.g. someone calling the euiBadge class on a div instead of simply using the EuiBadge component)~~ None, thankfully

Extras/nice-to-have

Reduced specificity where possible (usually by reducing nesting and class name chaining)
~~[ ] Optional component/code cleanup: consider splitting up the component into multiple children if it's overly verbose or difficult to reason about~~
~~[ ] Documentation pass~~
~~[ ] Check for issues in the backlog that could be a quick fix for that component~~

No major Kibana customization/className usages 🤞

- each input can use `controlOnly` to reduce number of layout wrappers rendered + remove direct `euiFieldNumber` className

- allows us to write us less repeated CSS

- delimited is just there for classNames, group styles already takes care of it. disabled/readonly can just inherit form styles

…rapper`

- underline needs to be on the children wrapper and not on the full parent - extend `euiFormControlDefaultShadow` to allow setting either just the box-shadow or the background-gradient

- JS context is generally more reliable than wildcard CSS, and consumers should generally prefer to use EUI components rather than not (or set their own styles if they're using raw HTML inputs) + fix EuiFormControlLayoutDelimited not inheriting fullWidth from form context

- doesn't have theme tokens and can just be a static const

+ Fix `cloneElementWithCss()` throwing an error if used twice without a `key` - remove unnecessary `!important`s - no longer needed with flattened Emotion styles - remove unnecessary `min-width: 0` (no longer an issue on Firefox?)

…FormControlLayoutDelimited` className - move inline styles to a nested obj for greater clarity - keeping the nesting now for laziness / since we might be rewriting date picker - `background-color` has some order shenanigans :T hopefully it's commented enough

+ remove unnecessary group/delimiter styles

…essary specificity - !important and other extra specificity comments can be removed - border radius is now completely handled by form control layouts and is no longer needed

+ remove unnecessary margin unset - not seeing it being used

…ed to always appending - this was otherwise causing a `shouldRenderWithCustomStyles` test failure in EuiDualRange

packages/eui/src/components/form/form_control_layout/form_control_layout_delimited.styles.ts

packages/eui/src/components/date_picker/date_picker.styles.ts

packages/eui/src/services/emotion/clone_element.tsx

mgadewoll · 2024-08-13T08:30:18Z

.../.loki/reference/chrome_desktop_Forms_EuiForm_EuiFormControlLayoutDelimited_Kitchen_Sink.png

[adding here for loose connection]

I notice that borders of append/prepend don't align correctly with the form control wrapper anymore. Is that expected?

Screen.Recording.2024-08-13.at.09.43.45.mov

😭 Thanks for pointing this out!

Should be fixed now in 4216c21
(edit: force pushed, forgot to update VRT)

.../.loki/reference/chrome_desktop_Forms_EuiForm_EuiFormControlLayoutDelimited_Kitchen_Sink.png

…donly states to start/end controls

- Fix `disabled`+`invalid` to hide invalid underline (transparent) - tweak `euiFormControlDefaultShadow` to allow more granular CSS - it's possible I should just rewrite this whole mixin at this point? :| - Update delimited styles to smoothly transition background color for disabled/readonly bgs (to match the inputs)

- have to use border and not box-shadow/padding for `overflow: hidden` to work correctly - I think I missed this/f'd this up in the amsterdam overrides

elasticmachine · 2024-08-13T17:40:18Z

💚 Build Succeeded

Buildkite Build
Commit: 4216c21

History

💚 Build #2569 succeeded 7424d0d
💚 Build #2565 succeeded 4027cc3

mgadewoll

🚢 🐈‍⬛ Thanks for the updates, looks good to me now! 👍

`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 12, 2024 13:22

[docs] Clean up EuiFormControlLayoutDelimited example

39c8497

- each input can use `controlOnly` to reduce number of layout wrappers rendered + remove direct `euiFieldNumber` className

DRY out repeated border/border-radius styles by adding new prop

18d507f

- allows us to write us less repeated CSS

Set up basic delimited styles

5dfc86c

- delimited is just there for classNames, group styles already takes care of it. disabled/readonly can just inherit form styles

Allowing passing styles directly to `.euiFormControlLayout__childrenW…

3eab24a

…rapper`

Set up invalid styles

f41ff68

- underline needs to be on the children wrapper and not on the full parent - extend `euiFormControlDefaultShadow` to allow setting either just the box-shadow or the background-gradient

Convert delimiter styles to Emotion

800ab9b

- doesn't have theme tokens and can just be a static const

Convert .euiFormControlLayoutDelimited__input styles

dfe0d36

+ Fix `cloneElementWithCss()` throwing an error if used twice without a `key` - remove unnecessary `!important`s - no longer needed with flattened Emotion styles - remove unnecessary `min-width: 0` (no longer an issue on Firefox?)

Delete Sass files

2091371

[EuiDatePickerRange] Fix regressed disabled background styles

7ae705a

+ remove unnecessary group/delimiter styles

[EuiSuperDatePicker] Fix quick select only styling + remove now-unnec…

92ff1ef

…essary specificity - !important and other extra specificity comments can be removed - border radius is now completely handled by form control layouts and is no longer needed

[EuiDualRange] Fix broken CSS selector

25e35da

+ remove unnecessary margin unset - not seeing it being used

[VRT] Update screenshots and add new permutations story

4903635

[tests] shouldRenderCustomStyles

fc74c1c

Enhance cloneElementWithCss to allow prepending cloned CSS as oppos…

6bd72d5

…ed to always appending - this was otherwise causing a `shouldRenderWithCustomStyles` test failure in EuiDualRange

cee-chen added the emotion label Aug 12, 2024

changelog

25e2db7

cee-chen force-pushed the emotion/form-layout-2 branch from 4027cc3 to 25e2db7 Compare August 13, 2024 05:03

Tweak EuiFieldNumber's browser stepper UI to follow EUI's color mode

7424d0d

cee-chen marked this pull request as ready for review August 13, 2024 06:02

cee-chen requested a review from a team as a code owner August 13, 2024 06:02

cee-chen changed the title ~~[Emotion] Convert EuiFormLayoutDelimited (2/3)~~ [Emotion] Convert EuiFormControlLayoutDelimited (2/3) Aug 13, 2024

mgadewoll reviewed Aug 13, 2024

View reviewed changes

cee-chen added 3 commits August 13, 2024 09:09

[PR feedback] Tweak delimited playground to pass invalid/disabled/rea…

78c1c29

…donly states to start/end controls

Fix border-radius regression/lapse

4216c21

- have to use border and not box-shadow/padding for `overflow: hidden` to work correctly - I think I missed this/f'd this up in the amsterdam overrides

cee-chen force-pushed the emotion/form-layout-2 branch from 5aef227 to 4216c21 Compare August 13, 2024 17:14

mgadewoll approved these changes Aug 14, 2024

View reviewed changes

cee-chen merged commit bf9eb33 into elastic:main Aug 14, 2024
5 checks passed

cee-chen deleted the emotion/form-layout-2 branch August 14, 2024 16:13

cee-chen mentioned this pull request Aug 20, 2024

Upgrade EUI to v95.9.0 elastic/kibana#190752

Merged

cee-chen mentioned this pull request Sep 12, 2024

Fix various EuiFormControlLayout usages elastic/kibana#192779

Open

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Emotion] Convert EuiFormControlLayoutDelimited (2/3) #7957

[Emotion] Convert EuiFormControlLayoutDelimited (2/3) #7957

cee-chen commented Aug 12, 2024 •

edited by mgadewoll

Loading

mgadewoll Aug 13, 2024

cee-chen Aug 13, 2024

cee-chen Aug 13, 2024 •

edited

Loading

elasticmachine commented Aug 13, 2024

mgadewoll left a comment

[Emotion] Convert EuiFormControlLayoutDelimited (2/3) #7957

[Emotion] Convert EuiFormControlLayoutDelimited (2/3) #7957

Conversation

cee-chen commented Aug 12, 2024 • edited by mgadewoll Loading

Summary

QA

General checklist

mgadewoll Aug 13, 2024

Choose a reason for hiding this comment

cee-chen Aug 13, 2024

Choose a reason for hiding this comment

cee-chen Aug 13, 2024 • edited Loading

Choose a reason for hiding this comment

elasticmachine commented Aug 13, 2024

💚 Build Succeeded

History

mgadewoll left a comment

Choose a reason for hiding this comment

cee-chen commented Aug 12, 2024 •

edited by mgadewoll

Loading

cee-chen Aug 13, 2024 •

edited

Loading