[Emotion] Convert EuiColorPalettePicker and EuiColorPaletteDisplay by cee-chen · Pull Request #7854 · elastic/eui

cee-chen · 2024-06-25T20:11:53Z

Summary

Note

This PR merges into a feature branch.

I recommend code reviewing by commit for this PR!

QA

The below links should look the same as before with no UI/visual regressions:

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~~ No playground

Unit tests

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

Sass/Emotion conversion process

Converted all global Sass vars/mixins to JS (e.g. $euiSize to euiTheme.size.base)
Listed var/mixin removals in changelog
Removed ~~or converted~~ component-specific Sass vars/mixins to exported JS versions
Removed component from src/components/index.scss
~~[ ] 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)~~
~~[ ] Deleted any src/amsterdam/overrides/{component}.scss files (styles within should have been converted to the baseline Emotion styles)~~
~~[ ] Added an @warn deprecation message within the global_styling/mixins/{component}.scss file~~

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.
- Removed .euiColorPaletteDisplay--[size] - no usages

Kibana due diligence

Search Kibana's codebase for {euiComponent}- (case sensitive) to check for usage of modifier classes - None
- ~~[ ] 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. - None
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

There's a single custom CSS euiColorPaletteDisplay override, but it shouldn't be impacted by this PR

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~~

mgadewoll · 2024-06-26T10:00:01Z

packages/eui/src/components/color_picker/color_palette_picker/color_palette_picker.tsx

 import { CommonProps } from '../../common';
+import { EuiSpacer } from '../../spacer';
+import { EuiText } from '../../text';
+import { EuiSuperSelect, type EuiSuperSelectProps } from '../../form';


This is weird: Changing the import like this results in the props not being passed along to the Storybook controls table 🤯 Only the ones manually defined as args are added but they lose their docgenInfo because it's not actually resolved.

Maybe it's the level of abstraction also here. '../../form' adds one more export file that the import goes through to resolve 🤔 But changing the path for it in /form/index to /super_select/super_select does not work. 🤷‍♀️

before

current

I saw this comment and can also reproduce this: The props are properly resolved when declaring components with function instead of const 🙈

Digging a bit further and seeing this issue, I can also confirm that this is also true in this case:

// does not work export const SomeComponent: FunctionComponent<Props> = ({ propA, propB, ...rest }) => {} // does work export const SomeComponent = ({ propA, propB, ...rest }: Props) => {}

whoaaa what the heck. I changed the import path to forms/super_select and it started working again :| NO idea why. What on earth. I left a comment so hopefully that helps at least??

Yeah I think for now that's fine. 👍 Eventually I guess we might need to look at this limitation around resolving types that we keep running into with Storybook, or rather react-docgen-typescript 🫣

Because React is not an ESM package, if using React.memo or React.ReactNode, etc., in the file, the TypeScript compiler used by react-docgen-typescript cannot recognize the type.

An easy solution is to replace the import: import React from 'react' with import * as React from 'react'.

The final solution is to replace the use of react-docgen-typescript with withDefaultConfig(parserOpts) by:

- import { withDefaultConfig } from 'react-docgen-typescript'; + import { withCompilerOptions } from 'react-docgen-typescript'; import { defaultOptions } from 'react-docgen-typescript/lib/parser'; - withDefaultConfig(parserOpts).parseWithProgramProvider + withCompilerOptions( + /* tsconfig compilerOptions */ + { + ...defaultOptions, + /* Either of these two is useful */ + esModuleInterop: true, + allowSyntheticDefaultImports: true, + }, + /* your parserOpts */ + parserOpts + ).parseWithProgramProvider

packages/eui/src/components/color_picker/color_palette_display/color_palette_display.styles.ts

.../eui/src/components/color_picker/color_palette_display/color_palette_display_fixed.styles.ts

- doesn't need CSS at all! whoa!

- convert Enzyme to RTL + snapshot the popover

- remove need for passing a size to by simply using the `inherit` CSS property - remove mixin - ignore light/dark arg, appears to be unused

- remove modifier maps, no Kibana usages - remove Sass variables - remove unnecessary nested bleed area height CSS by inheriting `height: 100%` from the parent

- can be a static const, no EUI variables being used - switch to position inset of height/width for simplicity - it's unclear if the 1px offset/workaround is due to an actual EUI var so I'm leaving it as a static 1px

kibanamachine · 2024-06-26T21:57:48Z

Preview staging links for this PR:

Docs site: https://eui.elastic.co/pr_7854/
Storybook: https://eui.elastic.co/pr_7854/storybook

elasticmachine · 2024-06-26T21:57:51Z

💚 Build Succeeded

Buildkite Build
Commit: 29a4697

History

💚 Build #2250 succeeded e1feba7
💚 Build #2229 succeeded 342721f
💚 Build #2228 succeeded a5ff8f6

mgadewoll

🚢 🐈‍⬛ Looking good to me!

cee-chen · 2024-06-27T15:51:03Z

Thanks Lene!!

`v95.2.0`⏩`v95.3.0` _[Questions? Please see our Kibana upgrade FAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams)_ --- ## [`v95.3.0`](https://github.com/elastic/eui/releases/v95.3.0) - Updated `EuiThemeProvider`s to allow modifying/setting custom `breakpoint`s in nested usage (as opposed to only at the top `EuiProvider` level) ([#7862](elastic/eui#7862)) **Bug fixes** - Fixed a Chrome/Edge CSS `mask-image` bug that was affecting scroll overflow shadow utilties ([#7855](elastic/eui#7855)) **CSS-in-JS conversions** - Converted `EuiColorPicker` to Emotion; Removed `$euiColorPickerWidth` ([#7845](elastic/eui#7845)) - Converted `EuiColorPickerSwatch` to Emotion ([#7853](elastic/eui#7853)) - Converted `EuiColorPalettePicker` and `EuiColorPaletteDisplay` to Emotion ([#7854](elastic/eui#7854)) - Removed `$euiColorPaletteDisplaySizes` - Removed `@mixin euiColorPaletteInnerBorder` - Removed `$euiColorPickerValueRange0`, `$euiColorPickerValueRange1`, `$euiColorPickerSaturationRange0`, `$euiColorPickerSaturationRange1`, and `$euiColorPickerIndicatorSize` ([#7859](elastic/eui#7859)) **Accessibility** - Updated the `aria-label` attribute for the `EuiFilePicker` remove file button ([#7860](elastic/eui#7860)) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

`v95.2.0`⏩`v95.3.0` _[Questions? Please see our Kibana upgrade FAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams)_ --- - Updated `EuiThemeProvider`s to allow modifying/setting custom `breakpoint`s in nested usage (as opposed to only at the top `EuiProvider` level) ([elastic#7862](elastic/eui#7862)) **Bug fixes** - Fixed a Chrome/Edge CSS `mask-image` bug that was affecting scroll overflow shadow utilties ([elastic#7855](elastic/eui#7855)) **CSS-in-JS conversions** - Converted `EuiColorPicker` to Emotion; Removed `$euiColorPickerWidth` ([elastic#7845](elastic/eui#7845)) - Converted `EuiColorPickerSwatch` to Emotion ([elastic#7853](elastic/eui#7853)) - Converted `EuiColorPalettePicker` and `EuiColorPaletteDisplay` to Emotion ([elastic#7854](elastic/eui#7854)) - Removed `$euiColorPaletteDisplaySizes` - Removed `@mixin euiColorPaletteInnerBorder` - Removed `$euiColorPickerValueRange0`, `$euiColorPickerValueRange1`, `$euiColorPickerSaturationRange0`, `$euiColorPickerSaturationRange1`, and `$euiColorPickerIndicatorSize` ([elastic#7859](elastic/eui#7859)) **Accessibility** - Updated the `aria-label` attribute for the `EuiFilePicker` remove file button ([elastic#7860](elastic/eui#7860)) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

cee-chen added the emotion label Jun 25, 2024

cee-chen force-pushed the emotion/color-picker-3 branch from aa4f13a to a5ff8f6 Compare June 25, 2024 20:29

cee-chen marked this pull request as ready for review June 25, 2024 22:06

cee-chen requested a review from a team as a code owner June 25, 2024 22:06

mgadewoll reviewed Jun 26, 2024

View reviewed changes

cee-chen force-pushed the emotion/color-pickers branch from 38b92ac to 2e46b04 Compare June 26, 2024 20:40

cee-chen added 10 commits June 26, 2024 13:43

[EuiColorPalettePicker] Convert Sass to components

e8256d5

- doesn't need CSS at all! whoa!

[EuiColorPalettePicker] Optional performance/memoization

cf4298b

[EuiColorPalettePicker] Update tests/snapshots

3bd8992

- convert Enzyme to RTL + snapshot the popover

[EuiColorPaletteDisplay] Set up Emotion file

ef960b6

[EuiColorPaletteDisplay] Convert pseudo border CSS

bc4d244

- remove need for passing a size to by simply using the `inherit` CSS property - remove mixin - ignore light/dark arg, appears to be unused

[EuiColorPaletteDisplay] Convert sizes

67175a3

- remove modifier maps, no Kibana usages - remove Sass variables - remove unnecessary nested bleed area height CSS by inheriting `height: 100%` from the parent

[EuiColorPaletteDisplayFixed] Convert to Emotion

118c3c1

- can be a static const, no EUI variables being used - switch to position inset of height/width for simplicity - it's unclear if the 1px offset/workaround is due to an actual EUI var so I'm leaving it as a static 1px

[EuiColorPaletteDisplay] Delete Sass files + update tests/snapshots

0f8a62c

changelog

54f69dc

[PR feedback] Make Storybook props table work again?

29a4697

cee-chen force-pushed the emotion/color-picker-3 branch from e1feba7 to 29a4697 Compare June 26, 2024 21:32

mgadewoll approved these changes Jun 27, 2024

View reviewed changes

cee-chen merged commit 608f7b3 into elastic:emotion/color-pickers Jun 27, 2024

cee-chen deleted the emotion/color-picker-3 branch June 27, 2024 15:51

cee-chen mentioned this pull request Jun 27, 2024

[Emotion] Convert all color picker components #7863

Merged

22 tasks

tkajtoch mentioned this pull request Jul 2, 2024

Upgrade EUI to v95.3.0 elastic/kibana#187342

Merged

cee-chen mentioned this pull request Aug 13, 2024

[Emotion] EuiColorPicker #6390

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Emotion] Convert EuiColorPalettePicker and EuiColorPaletteDisplay#7854

[Emotion] Convert EuiColorPalettePicker and EuiColorPaletteDisplay#7854
cee-chen merged 10 commits intoelastic:emotion/color-pickersfrom
cee-chen:emotion/color-picker-3

cee-chen commented Jun 25, 2024 •

edited by mgadewoll

Loading

Uh oh!

mgadewoll Jun 26, 2024

Uh oh!

cee-chen Jun 26, 2024

Uh oh!

mgadewoll Jun 27, 2024

Uh oh!

zjfresh Jul 25, 2024 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

kibanamachine commented Jun 26, 2024

Uh oh!

elasticmachine commented Jun 26, 2024

Uh oh!

mgadewoll left a comment

Uh oh!

cee-chen commented Jun 27, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

cee-chen commented Jun 25, 2024 • edited by mgadewoll Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

QA

General checklist

Uh oh!

mgadewoll Jun 26, 2024

Choose a reason for hiding this comment

Uh oh!

cee-chen Jun 26, 2024

Choose a reason for hiding this comment

Uh oh!

mgadewoll Jun 27, 2024

Choose a reason for hiding this comment

Uh oh!

zjfresh Jul 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kibanamachine commented Jun 26, 2024

Uh oh!

elasticmachine commented Jun 26, 2024

💚 Build Succeeded

History

Uh oh!

mgadewoll left a comment

Choose a reason for hiding this comment

Uh oh!

cee-chen commented Jun 27, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

cee-chen commented Jun 25, 2024 •

edited by mgadewoll

Loading

zjfresh Jul 25, 2024 •

edited

Loading