-
Notifications
You must be signed in to change notification settings - Fork 841
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Emotion] Convert EuiFormLabel and EuiFormLegend #7967
Conversation
- do something new/slightly clever with the returned title style obj
- reduce specificity of `[for]` CSS, which lets us remove an `!important` elsewhere, and remove need to set/override it at all for disabled labels
- remove mounted Emotion snapshots from EuiFormRow in favor of specific assertion
- likely just minor typographic changes, but might as well update these while we're here
aab3eab
to
dc2ede9
Compare
💚 Build Succeeded
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm approving this because the changes look good to me and I want to keep this moving.
I did take a look at some usage in Kibana, just raising them here in advance just to check in to see if any of it looks concerning.
@@ -1,6 +1,4 @@ | |||
.euiFormLegend { | |||
@include euiFormLabel; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I only see this being used once in Kibana, so shouldn't be a big change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for looking into it! I'll likely change their usage to the <EuiFormLabel />
component directly.
* 1. Focused state overrides invalid state. | ||
* 2. Disabled state overrides pointer. | ||
*/ | ||
.euiFormLabel { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When we remove this from our sass files ... is it still available globally for use like this? Or will that cause an issue: https://github.com/elastic/kibana/blob/main/x-pack/plugins/cases/public/components/configure_cases/field_mapping.tsx#L35
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, it will cause an issue! I'll change their usage to the EuiFormLabel
component directly
`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>
Summary
Part of #6400
This PR is mostly downstream screenshot/snapshot updates, so I recommend reviewing by commit. Otherwise it's a fairly straightforward conversion
QA
The below links/docs should look the same as production:
General checklist
Added orupdated jestand cypresstests- [ ] If applicable, added the breaking change issue label (and filled out the breaking change checklist)