[Emotion] Convert EuiButtonEmpty #6863

cee-chen · 2023-06-21T19:09:00Z

Summary

This PR:

Converts EuiButtonEmpty to Emotion, removing/cleaning up unnecessary CSS due to dogfooding of new internal EuiButtonDisplay components
Removes all .euiButtonEmpty-- modifier classes
Removes EuiButtonContentDeprecated fully - this was the last component using the old/deprecated internal button components
closes [Emotion] EuiButton #6388 - this is the last button component to be converted to Emotion 🎉

As always, for review I recommend following along by commit (due to the number of affected snapshots).

QA

Confirm that production looks similar to staging (note that there is an expected slight font/typography change)
gh pr checkout 6863 && yarn storybook
Confirm that the color, flush, and size permutations work as expected in Storybook

General checklist

Checked in both light and dark modes
Checked in mobile
Checked in Chrome, Safari, Edge, and Firefox
Added or updated jest and cypress tests
A changelog entry exists and is marked appropriately

Emotion checklist

Acceptance criteria

All SCSS files have been removed from the component(s) directory
~~- [ ] All SCSS overrides have been removed from the Amsterdam theme~~ - No overrides for this component
~~- [ ] Any dependent components are identified in a new issue~~ - All dependent usages in EUI are calling the generic .euiButtonEmpty or .euiButtonEmpty__content

Checklists

Kibana usage

Kibana due diligence

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 Sass or CSS that will need to be updated, particularly if a component Sass var was deleted~~ None
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~~ - Usages can be swapped out for non-modifier .euiButtonEmpty
Any test/query selectors that will need to be updated
- Multiple updates needed
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)
- Two usages that will need to be fixed

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 from Enzyme to RTL
~~- [ ] Removed any mounted snapshots~~

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)~~
~~- [ ] Removed or converted component-specific Sass vars/mixins to exported JS versions~~
~~- [ ] Listed var/mixin removals in changelog~~
~~- [ ] 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)~~
~~- [ ] Added an @warn deprecation message within the global_styling/mixins/{component}.scss file~~

CSS tech debt

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

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.

Extras/nice-to-have

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

…omponent

- place the padding on the parent button instead of on the content - remove the text truncation (already handled by the new component - no need to repeat) - retain `className`s, as they're used/targeted in many places in Kibana, but update syntax

- flush modifiers - transition + comment - leave out transform/animation - those aren't defined in the new base CSS-in-JS util and don't need to be unset

+ rewrite enums to new preferred syntax

+ add missing `textProps` test

- TODO: add another story for link vs button? - add another with all permutations visible at once (like in docs?)

kibanamachine · 2023-06-21T20:35:17Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_6863/

kibanamachine · 2023-06-21T21:08:07Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_6863/

kibanamachine · 2023-06-23T00:50:07Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_6863/

breehall

closes #6388 - this is the last button component to be converted to Emotion 🎉

WHOOP! Love to see it! This looks good and it's so nice to have these inside of Sorybook. I compared staging to prod and everything looks the same for the most part. I did notice the very small typographical difference mentioned.

One observation for the current Playground for EuiEmptyButton's with ghost as the color.
In light mode when selecting the buttons in the Playground, it doesn't have a background highlight but in Storybook it does. It works fine in the docs example when switching to dark mode and selecting the buttons, so I don't think it's a problem but I figured I would call it out. I know ghost has its quirks 👻

src/components/button/button_empty/button_empty.styles.ts

breehall · 2023-06-26T18:50:09Z

src/components/inline_edit/inline_edit_form.styles.ts

+ // Override the cursor and allow users to highlight text when in the read only state
 isReadOnly: css`
- &.euiButtonEmpty:disabled {
+ &:disabled {


Thank you for including the TODO in this PR!

Co-authored-by: Bree Hall <40739624+breehall@users.noreply.github.com>

cee-chen · 2023-06-26T19:33:53Z

so I don't think it's a problem but I figured I would call it out. I know ghost has its quirks 👻

Yep, good eye! It's basically a quirk of ghost. It'll probably make more sense to simply deprecate the prop in the future 🤞

kibanamachine · 2023-06-26T19:54:02Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_6863/

kibanamachine · 2023-06-27T00:12:25Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_6863/

`eui@82.1.0` ⏩ `83.0.0` ⚠️ The biggest change in this PR by far is the `EuiButtonEmpty` Emotion conversion, which changes the DOM structure of the button slightly as well as several CSS classes around it. EUI has attempted to convert any custom EuiButtonEmpty CSS overrides where possible, but would super appreciate it if CODEOWNERS checked their touched files. If anything other than a snapshot or test was touched, please double check the display of your button(s) and confirm everything still looks shipshape. Feel free to ping us for advice if not. --- ## [`83.0.0`](https://github.com/elastic/eui/tree/v83.0.0) **Bug fixes** - Fixed `EuiPaginationButton` styling affected by `EuiButtonEmpty`'s Emotion conversion ([#6893](elastic/eui#6893)) **Breaking changes** - Removed `isPlaceholder` prop from `EuiPaginationButton` ([#6893](elastic/eui#6893)) ## [`82.2.1`](https://github.com/elastic/eui/tree/v82.2.1) - Updated supported Node engine versions to allow Node 16, 18 and >=20 ([#6884](elastic/eui#6884)) ## [`82.2.0`](https://github.com/elastic/eui/tree/v82.2.0) - Updated EUI's SVG icons library to use latest SVGO v3 optimization ([#6843](elastic/eui#6843)) - Added success color `EuiNotificationBadge` ([#6864](elastic/eui#6864)) - Added `badgeColor` prop to `EuiFilterButton` ([#6864](elastic/eui#6864)) - Updated `EuiBadge` to use CSS-in-JS for named colors instead of inline styles. Custom colors will still use inline styles. ([#6864](elastic/eui#6864)) **CSS-in-JS conversions** - Converted `EuiButtonGroup` and `EuiButtonGroupButton` to Emotion ([#6841](elastic/eui#6841)) - Converted `EuiButtonIcon` to Emotion ([#6844](elastic/eui#6844)) - Converted `EuiButtonEmpty` to Emotion ([#6863](elastic/eui#6863)) - Converted `EuiCollapsibleNav` and `EuiCollapsibleNavGroup` to Emotion ([#6865](elastic/eui#6865)) - Removed Sass variables `$euiCollapsibleNavGroupLightBackgroundColor`, `$euiCollapsibleNavGroupDarkBackgroundColor`, and `$euiCollapsibleNavGroupDarkHighContrastColor` ([#6865](elastic/eui#6865)) --------- Co-authored-by: Cee Chen <constance.chen@elastic.co> Co-authored-by: Jeramy Soucy <jeramy.soucy@elastic.co> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

[EuiButtonEmpty] Convert to use new non-deprecated internal content c…

46a2251

…omponent

cee-chen force-pushed the emotion/button-empty branch from fd9b536 to a18a507 Compare June 21, 2023 19:10

cee-chen added 2 commits June 21, 2023 12:59

[EuiButtonEmpty] Convert styles shared with EuiButtonDisplay

710565e

cee-chen force-pushed the emotion/button-empty branch 2 times, most recently from 18dd77f to 64e25ae Compare June 21, 2023 20:11

cee-chen and others added 7 commits June 21, 2023 13:22

[EuiButtonEmpty] Convert remaining CSS specific to empty buttons

430eb86

- flush modifiers - transition + comment - leave out transform/animation - those aren't defined in the new base CSS-in-JS util and don't need to be unset

[EuiButtonEmpty] Remove modifier classes

1698c7a

+ rewrite enums to new preferred syntax

[EuiButtonEmpty] Delete Sass files

77b7172

Delete EuiButtonContentDeprecated and all button Sass files/imports

865fb7b

[EuiButtonEmpty] Convert tests from Enzyme to RTL

380190b

+ add missing `textProps` test

[EuiButtonEmpty] Add Storybook

9ba156a

- TODO: add another story for link vs button? - add another with all permutations visible at once (like in docs?)

changelog

b27f118

Update downstream snapshots

03afc3f

cee-chen force-pushed the emotion/button-empty branch from 64e25ae to 03afc3f Compare June 21, 2023 20:40

cee-chen requested a review from breehall June 21, 2023 21:58

cee-chen marked this pull request as ready for review June 21, 2023 22:05

cee-chen mentioned this pull request Jun 22, 2023

[Emotion] EuiButton #6388

Closed

Address specificity TODO

c49ba5b

breehall approved these changes Jun 26, 2023

View reviewed changes

cee-chen and others added 2 commits June 26, 2023 12:30

[PR feedback] comment typo

02dae69

Co-authored-by: Bree Hall <40739624+breehall@users.noreply.github.com>

Merge branch 'main' into emotion/button-empty

0259dc8

cee-chen enabled auto-merge (squash) June 26, 2023 19:34

snapshot

921b1a7

cee-chen merged commit 38ec5ab into elastic:main Jun 27, 2023
1 check passed

cee-chen deleted the emotion/button-empty branch June 27, 2023 00:29

breehall mentioned this pull request Jun 28, 2023

Upgrade EUI to 83.0.0 elastic/kibana#160813

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Emotion] Convert EuiButtonEmpty #6863

[Emotion] Convert EuiButtonEmpty #6863

cee-chen commented Jun 21, 2023 •

edited

Loading

kibanamachine commented Jun 21, 2023

kibanamachine commented Jun 21, 2023

kibanamachine commented Jun 23, 2023

breehall left a comment

breehall Jun 26, 2023

cee-chen commented Jun 26, 2023

kibanamachine commented Jun 26, 2023

kibanamachine commented Jun 27, 2023

[Emotion] Convert EuiButtonEmpty #6863

[Emotion] Convert EuiButtonEmpty #6863

Conversation

cee-chen commented Jun 21, 2023 • edited Loading

Summary

QA

General checklist

Emotion checklist

Acceptance criteria

Checklists

kibanamachine commented Jun 21, 2023

kibanamachine commented Jun 21, 2023

kibanamachine commented Jun 23, 2023

breehall left a comment

Choose a reason for hiding this comment

breehall Jun 26, 2023

Choose a reason for hiding this comment

cee-chen commented Jun 26, 2023

kibanamachine commented Jun 26, 2023

kibanamachine commented Jun 27, 2023

cee-chen commented Jun 21, 2023 •

edited

Loading