[Emotion] Convert `EuiFacetButton` and `EuiFacetGroup` #5878

miukimiu · 2022-05-05T20:21:40Z

Summary

Converted both EuiFacetButton and EuiFacetGroup to Emotion. The horizontal EuiFacetGroup is no longer using negative margins. It was converted to use flex and gap.

Facet in Emotion

With the use of gap the sizes of the components changes a little. Because we no longer have the margin top and bottom for each button.

Facet layout example

I deleted one of the examples in Facet Layout section. It was repeated:

Deprecated `EuiButtonDisplay` in favor of a new `EuiButtonDisplay`

The EuiFacetButton is a button so it makes sense to use the EuiButtonDisplay instead of a custom button with some CSS styles. But to use the EuiButtonDisplay I found some issues:

It required a baseClassName.
If no color is passed it creates a ${baseClassName}${colorToClassNameMap.primary}.
The component uses Sass
The children was being wrapped with a span and expected to receive text.
The component only expects to have one icon.

So I decided to set this component to be deprecated. To do this I renamed the component toEuiButtonDisplayDeprecated.

The new component is meant to internal use only and for now it lives in src/components/button/button_display/_button_display.tsx:

This component is basically a conversion from the old component to use Emotion
The goal of the component is to be less strict and more reusable so for example we can pass custom children. If a children is not a string we assume is not text and the content is not wrapped in a span. This way it's easier to account for button components that don't follow the normal button structure (1 icon and text).
The color prop was omitted because the old logic to apply colors to button components has to be rethought. Buttons that don't have a background color should also use this component. This logic should be thought of when we start converting other buttons that require backgrounds.
All the EuiButtonProps types were copied to this new file and renamed as EuiButtonDisplayProps. I believe this component should have the common button props.

Deprecated `EuiButtonContent` in favour of `EuiButtonDisplayContent`

The idea of this component is to replace the EuiButtonContent. I called it EuiButtonDisplayContent because the way I see it in the future is this component to be an inner component of the EuiButtonDisplay. So we would always use the EuiButtonDisplay.

For example, the EuiButtonEmpty uses the EuiButtonContent but in the future, it could use the EuiButtonDisplay.

eui/src/components/button/button_empty/button_empty.tsx

Lines 197 to 208 in 776e95a

    
           return ( 
        
             <button 
        
               disabled={buttonIsDisabled} 
        
               className={classes} 
        
               type={type} 
        
               ref={buttonRef as Ref<HTMLButtonElement>} 
        
               aria-pressed={isSelected} 
        
               {...(rest as EuiButtonEmptyPropsForButton)} 
        
             > 
        
               {innerNode} 
        
             </button> 
        
           );

As we can see the EuiButtonEmpty could use the EuiButtonDisplay. No need to create a custom button and pass the the innerNode that is a EuiButtonContent.

EuiFacetButton is now using the new EuiButtonDisplay

With the new EuiButtonDisplay the goal is for all EUI button components to use the EuiButtonDisplay as a building block. So the new EuiFacetButton is now using this component.

EuiLoadingSpinner accepts new `color` prop

It's now possible to configure the EuiLoadingSpinner color. The new EuiButtonDisplayContent uses a EuiLoadingSpinner with this new color prop.

Checklist

Checked in both light and dark modes
[ ] Checked in mobile
Checked in Chrome, Safari, Edge, and Firefox
Props have proper autodocs and playground toggles
[ ] Added documentation
~~[ ] Checked Code Sandbox works for any docs examples~~
Added or updated jest and cypress tests
~~[ ] Checked for breaking changes and labeled appropriately~~
~~[ ] Checked for accessibility including keyboard-only and screenreader modes~~
~~[ ] Updated the Figma library counterpart~~
A changelog entry exists and is marked appropriately

Things to look out for when moving styles

~~[ ] Anything in the backlog that could be a quick fix for that component?~~
Convert global Sass vars to JS version like sizing
~~[ ] Convert component-specific Sass vars to exported JS versions~~
Convert side specific padding, margin, and position to -inline and -block (Logical properties)
~~[ ] Reduce specificity where possible (usually by reducing class name chaining)~~
Use gap property to add margin between items if using flex
~~[ ] Can any still existing .js files be converted to TS?~~
All animations or transitions are wrapped in euiCanAnimate

QA

Does the output CSS match the previous CSS / as expected
Does the rendered class read as expected

kibanamachine · 2022-05-05T20:51:01Z

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

…rsion

kibanamachine · 2022-05-06T13:06:25Z

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

miukimiu · 2022-05-06T15:05:23Z

src/components/facet/facet_button.tsx

-      className: classNames(icon.props.className, 'euiFacetButton__icon'),
+    buttonIcon = cloneElementWithCss(icon, {
+      css: cssIconStyles,
+      className: 'euiFacetButton__icon',


In use in Kibana.

miukimiu · 2022-05-06T15:05:36Z

src/components/facet/facet_button.tsx

            <span
+              css={cssTextStyles}
              className="euiFacetButton__text"


In use in Kibana.

kibanamachine · 2022-05-06T15:12:50Z

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

…rsion

kibanamachine · 2022-05-09T14:32:55Z

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

kibanamachine · 2022-05-09T15:31:56Z

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

…rsion

kibanamachine · 2022-05-12T16:17:06Z

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

src/components/button/button.tsx

kibanamachine · 2022-06-01T15:31:55Z

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

cchaos

Getting close, I didn't realize how involved this component is. But we should also make sure we're cleaning up any unused exports or unnecessary mixins. I'll leave the deprecation naming question to the engineers.

src/components/button/button_display/_button_display.styles.ts

src/components/button/button_display/_button_display.tsx

src/components/button/button_display/_button_display_content.styles.ts

src/components/facet/facet_button.styles.ts

src/components/loading/loading_spinner.styles.ts

…rsion

kibanamachine · 2022-06-03T16:06:25Z

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

kibanamachine · 2022-06-03T17:47:57Z

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

miukimiu · 2022-06-03T18:07:39Z

@cchaos I went through your review:

Deleted all the types that were not being used in the new EuiButtonDisplay.
Added a color prop to EuiLoadingSpinner

@thompsongl I renamed both deprecated components to EuiButtonDisplayDeprecated and EuiButtonContentDeprecated. No more _Deprecated on both names.

I updated the PR description to add the new changes.

kibanamachine · 2022-06-03T19:06:19Z

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

cchaos

LGTM, I just had a few last nits. But overall I think it's good to go.

src/components/loading/loading_spinner.tsx

upcoming_changelogs/5878.md

src/components/button/button_display/_button_display.tsx

Co-authored-by: Caroline Horn <[email protected]>

kibanamachine · 2022-06-08T13:28:19Z

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

…rsion

kibanamachine · 2022-06-08T14:53:35Z

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

This reverts commit 2e836f6 to help release a patch release.

#5878)"" This reverts commit 469cee1, re-adding #5878 to the main branch.

kibanamachine · 2022-06-29T13:25:10Z

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

miukimiu added 3 commits May 5, 2022 20:56

Convert EuiFacetButton and EuiFacetGroup to emotion

3aaf346

Deleting .scss files

8be0805

Moving buttons mixins to functions

265fc72

miukimiu added the skip-changelog label May 5, 2022

miukimiu added 3 commits May 6, 2022 13:22

Removed unused mixin. Adding CL. Improving gutter size func.

322793f

Merge remote-tracking branch 'upstream/main' into emotion-facet-conve…

f4784a1

…rsion

Improving example

b8f9133

miukimiu added 5 commits May 6, 2022 14:42

Better code for calling euiTheme

1ddd2cd

Renaming contentElementStyles to contentElementsStyles

cd2f2ec

Fix typo. CL.

e29df48

Better gap styles

54172b4

Adding euiCanAnimate for transitions

a8a4eec

miukimiu added emotion and removed skip-changelog labels May 6, 2022

miukimiu commented May 6, 2022

View reviewed changes

miukimiu marked this pull request as ready for review May 6, 2022 15:06

miukimiu added 2 commits May 9, 2022 14:29

Merge remote-tracking branch 'upstream/main' into emotion-facet-conve…

9db464c

…rsion

Moving button functions and using euiLinkFocusCSS

781f93b

Typo

80a149b

miukimiu mentioned this pull request May 9, 2022

[Meta][CSS-in-JS] Component conversions #5685

Closed

49 tasks

miukimiu added 2 commits May 12, 2022 16:46

Adding logicalCSS

2452e51

Merge remote-tracking branch 'upstream/main' into emotion-facet-conve…

bbfff1c

…rsion

miukimiu requested a review from cchaos May 16, 2022 11:32

miukimiu commented Jun 1, 2022

View reviewed changes

src/components/button/button.tsx Outdated Show resolved Hide resolved

cchaos reviewed Jun 2, 2022

View reviewed changes

miukimiu added 4 commits June 3, 2022 12:42

Renaming deprecated components

3f51ca0

Addressing PR review

e5a1d1c

Added new color prop to EuiLoadingSpinner

a522250

Merge remote-tracking branch 'upstream/main' into emotion-facet-conve…

51eec20

…rsion

miukimiu added 2 commits June 3, 2022 17:57

Removing unnecessary prop comment

fe6cbc9

Making EuiLoadingSpinnerProps['color'] show in props table

533fa71

Better props table for EuiLoadingSpinnerColor

be920ca

miukimiu requested review from thompsongl and cchaos June 3, 2022 18:38

cchaos approved these changes Jun 7, 2022

View reviewed changes

miukimiu and others added 5 commits June 8, 2022 13:12

Update src/components/loading/loading_spinner.tsx

1555212

Co-authored-by: Caroline Horn <[email protected]>

Update src/components/loading/loading_spinner.tsx

8668a50

Co-authored-by: Caroline Horn <[email protected]>

Update upcoming_changelogs/5878.md

344a8f0

Co-authored-by: Caroline Horn <[email protected]>

Update src/components/button/button_display/_button_display.tsx

9327062

Co-authored-by: Caroline Horn <[email protected]>

Moving CL entry out of CSS-in-JS

2bc4cb9

Merge remote-tracking branch 'upstream/main' into emotion-facet-conve…

daf148d

…rsion

miukimiu merged commit 2e836f6 into elastic:main Jun 8, 2022

chandlerprall added a commit that referenced this pull request Jun 13, 2022

Revert "[Emotion] Convert EuiFacetButton and EuiFacetGroup (#5878)"

469cee1

This reverts commit 2e836f6 to help release a patch release.

chandlerprall added a commit that referenced this pull request Jun 13, 2022

Revert "Revert "[Emotion] Convert EuiFacetButton and EuiFacetGroup (

42c82b6

#5878)"" This reverts commit 469cee1, re-adding #5878 to the main branch.

cee-chen mentioned this pull request Jun 22, 2022

Upgrade EUI to v60.0.0 elastic/kibana#134919

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Emotion] Convert `EuiFacetButton` and `EuiFacetGroup` #5878

[Emotion] Convert `EuiFacetButton` and `EuiFacetGroup` #5878

miukimiu commented May 5, 2022 •

edited

Loading

kibanamachine commented May 5, 2022

kibanamachine commented May 6, 2022

miukimiu May 6, 2022

miukimiu May 6, 2022

kibanamachine commented May 6, 2022

kibanamachine commented May 9, 2022

kibanamachine commented May 9, 2022

kibanamachine commented May 12, 2022

kibanamachine commented Jun 1, 2022

cchaos left a comment

kibanamachine commented Jun 3, 2022

kibanamachine commented Jun 3, 2022

miukimiu commented Jun 3, 2022 •

edited

Loading

kibanamachine commented Jun 3, 2022

cchaos left a comment

kibanamachine commented Jun 8, 2022

kibanamachine commented Jun 8, 2022

kibanamachine commented Jun 29, 2022

	return (
	<button
	disabled={buttonIsDisabled}
	className={classes}
	type={type}
	ref={buttonRef as Ref<HTMLButtonElement>}
	aria-pressed={isSelected}
	{...(rest as EuiButtonEmptyPropsForButton)}
	>
	{innerNode}
	</button>
	);

[Emotion] Convert EuiFacetButton and EuiFacetGroup #5878

[Emotion] Convert EuiFacetButton and EuiFacetGroup #5878

Conversation

miukimiu commented May 5, 2022 • edited Loading

Summary

Facet in Emotion

Facet layout example

Deprecated EuiButtonDisplay in favor of a new EuiButtonDisplay

Deprecated EuiButtonContent in favour of EuiButtonDisplayContent

EuiFacetButton is now using the new EuiButtonDisplay

EuiLoadingSpinner accepts new color prop

Checklist

Things to look out for when moving styles

QA

kibanamachine commented May 5, 2022

kibanamachine commented May 6, 2022

miukimiu May 6, 2022

Choose a reason for hiding this comment

miukimiu May 6, 2022

Choose a reason for hiding this comment

kibanamachine commented May 6, 2022

kibanamachine commented May 9, 2022

kibanamachine commented May 9, 2022

kibanamachine commented May 12, 2022

kibanamachine commented Jun 1, 2022

cchaos left a comment

Choose a reason for hiding this comment

kibanamachine commented Jun 3, 2022

kibanamachine commented Jun 3, 2022

miukimiu commented Jun 3, 2022 • edited Loading

kibanamachine commented Jun 3, 2022

cchaos left a comment

Choose a reason for hiding this comment

kibanamachine commented Jun 8, 2022

kibanamachine commented Jun 8, 2022

kibanamachine commented Jun 29, 2022

[Emotion] Convert `EuiFacetButton` and `EuiFacetGroup` #5878

[Emotion] Convert `EuiFacetButton` and `EuiFacetGroup` #5878

miukimiu commented May 5, 2022 •

edited

Loading

Deprecated `EuiButtonDisplay` in favor of a new `EuiButtonDisplay`

Deprecated `EuiButtonContent` in favour of `EuiButtonDisplayContent`

EuiLoadingSpinner accepts new `color` prop

miukimiu commented Jun 3, 2022 •

edited

Loading