[EuiButton] Convert to Emotion #5989

cchaos · 2022-06-21T19:54:11Z

1. Moved Amsterdam-specific Sass overrides to default `global_styling/` folder

This just cleans up some of the unnecessary overrides and Sass files. But mostly leaves the variable / mixins in tact for now.

2. New theme-specific (Amsterdam only for now) button styling functions

Removed disabled as a specific BUTTON_COLOR but allow it to be passed into euiButtonColor for coloring
euiButtonColor() now returns both backgroundColor and color for proper text contrast
euiButtonFillColor() creates the fill (solid color, white/black text) styling combo
euiButtonEmptyColor() creates the empty style using text-variants for color and transparent hover backgrounds
useEuiButtonColorCSS() now accepts options such as the display: 'base' | 'fill' | 'empty' and returns both keys for color and display.
useEuiButtonRadiusCSS() returns the border-radius values by size of button
useEuiButtonFocusCSS() is just the translate animation

3. Button component updates

EuiButton

Renamed exported ButtonColor to EuiButtonColor and ButtonSize to EuiButtonSize
Using the new EuiButtonDisplay component as the render and passing on all the rest of the props to handle the button vs anchor logic
Using Emotion for color, fill, and disabled and removing associated classes.
Fixed outline focus color of text color fill buttons
Background colors for base buttons are no longer transparent

The new tinted colors are just slightly different from their old transparent counterpart. The biggest difference being in the text version which will now align better to forms.

Note
This basically means this whole component is now Emotion since EuiButton now only handles superficial styling, everything else is passed through to the already Emotion EuiButtonDisplay.

EuiButtonEmpty

Using Emotion for color, and disabled and removing associated classes.

Note
There's still more we can do here to make use of EuiButtonDisplay but I didn't want to get too far into the weeds.

EuiButtonIcon

Using Emotion for color, disabled, and display removing associated classes.

Note
There's still more we can do here to make use of EuiButtonDisplay but I didn't want to get too far into the weeds.

EuiButtonGroup & Option

[Breaking]: Removed support for ghost. The combo styles between Emotion and CSS were too complex to maintain, but it felt safe to remove support (based an searching for usages - none) and will guide consumers to use colorMode instead once fully converted.
Using Emotion for color, disabled, and isSelected (as display='fill')

Note
This still uses the old/deprecated EuiButtonDisplay for now

EuiButtonDisplay

Since this is the component that renders the actual element (button vs anchor) I have moved all the logic around the associated props of each from EuiButton to this file so all consumers of this can benefit / doesn't have to worry about the actual DOM element.

EuiButtonDisplayContent

I moved the currently applied disabled styles to be handled by EuiButtonDisplay instead while also applying truncation to the text span using the utility class.

Other updates

Added isButtonDisabled() utility for DRY-ing out the logic which compares the disabled, isDisabled, isLoading, and validity of href to determine final disabled state.
[EuiMarkdownEditorToolbar] Added data-test-subj props to empty buttons for tests

Checklist

# Conflicts: # src/components/accessibility/skip_link/__snapshots__/skip_link.test.tsx.snap # src/components/accordion/__snapshots__/accordion.test.tsx.snap # src/components/collapsible_nav/collapsible_nav_group/__snapshots__/collapsible_nav_group.test.tsx.snap # src/components/control_bar/__snapshots__/control_bar.test.tsx.snap

- Uses new EuiButtonDisplay - Updates EuiButtonDisplay with more styles and fixes

kibanamachine · 2022-06-21T20:19:06Z

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

kibanamachine · 2022-06-21T23:43:58Z

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

kibanamachine · 2022-06-24T03:13:11Z

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

cchaos

This PR is ready for review. I know it's large, but it's also a lot of snap updates. There is also still a lot of repetition that will hopefully get updated soon after this PR. A lot of the styling is importing from the theme folder but I'm hoping we'll find a solution to #5888 that will remove these relative imports.

cchaos · 2022-06-21T22:36:36Z

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

+      font-weight: ${euiTheme.font.weight.medium};
+      padding: 0 ${euiTheme.size.m};
+
+      &:hover:not(:disabled),
+      &:focus {
+        text-decoration: underline;
+      }


I'll be addressing theses styles more globally in a follow up.

cchaos · 2022-06-21T22:40:44Z

src/components/button/button.tsx

+// Use defaultProps for simple pass-through props
+EuiButton.defaultProps = {
+  minWidth: 112,
+  size: 'm',
+};


This was a cool little pattern I realized in our Props table. If the component is inheriting props from somewhere else and doesn't actually do anything with the props but pass them through, instead of having to re-declare them outside of the ..rest spread, just using .defaultProps works here and spits out the values in our docs props table. 🙌

kibanamachine · 2022-06-27T16:25:57Z

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

miukimiu

Just found some initial issues but I just tested in Chome for now. I'll test later in other browsers.

src-docs/src/views/button/button_example.js

miukimiu · 2022-06-29T12:26:19Z

src/components/button/button.tsx

+
+  const buttonColorStyles = useEuiButtonColorCSS({
+    display: fill ? 'fill' : 'base',
+  })[color === 'ghost' ? 'text' : color];


The ghost color in light mode doesn't have enough contrast when on top of dark background colors:

Not yet, mainly becuase the dark background components haven't been converted to Emotion to use the dark-mode emptyShade instead of this custom color. Check out the first bottom bar example which is a fully dark-mode component: https://eui.elastic.co/pr_5989/#/layout/bottom-bar

miukimiu · 2022-06-29T12:59:15Z

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

@@ -46,18 +46,29 @@ export const euiButtonDisplayStyles = (
    euiButtonDisplay: css`
      ${euiButtonBaseCSS()};
      ${minWidth && logicalCSS('min-width', minWidth)};


I was the one adding this minWidth here. But I wonder if is it better to handle this in the style tag.

I think you're definitely right if the minWidth is provided by the consumer. But I wonder if we should put the default in the style tag becuase that could be a breaking change if consumers are currently adjusting this property via a custom class. By moving the default to the style tag, their specificity would be overridden.

…`text`

kibanamachine · 2022-06-29T15:34:05Z

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

kibanamachine · 2022-07-26T20:21:42Z

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

morksinaanab · 2022-08-03T10:07:41Z

I downloaded and trying EUI yesterday for the first time. When setting the primary color via the modify prop of EuiProvider, button colors do not change. Is this PR necessary for this to work?

thompsongl · 2022-08-08T18:47:12Z

When setting the primary color via the modify prop of EuiProvider, button colors do not change. Is this PR necessary for this to work?

Yes. Only components converted to Emotion styling will react to changes introduced with the modify prop.

thompsongl · 2022-08-09T15:55:36Z

@miukimiu Would you mind doing another design pass on this before I take a look from the engineering side?

miukimiu

LGTM! 🎉

Tested in Chrome, Safari, Edge, and Firefox.

Just one minor suggestion. What do you think of changing the last example to have a EuiPanel with color "subdued"? I think the contrast gets a little bit better. Let me know your thoughts.

Also the EuiText is not working inside the <EuiThemeProvider colorMode="dark">, I'll open an issue for that.

cchaos · 2022-08-10T17:00:04Z

I am just going to hand off this PR. I'd worry I'm rushing to push it through based on some incidental issues we had with forcing colorMode.

miukimiu · 2022-08-10T17:55:28Z

I am just going to hand off this PR. I'd worry I'm rushing to push it through based on some incidental issues we had with forcing colorMode.

Sounds good. 👍🏽

I opened the PR for the color mode issue with EuiText: #6123

@mixin

* Convert text globals/utilities to Emotion + fix missing CSS logical property on text truncation mixin + convert textLeft/Right to logical CSS - note that we can't use the logicalTextAlign util for this as !important can't be passed to that util * Pass euiTheme to globalStyles and convert euiNumberFormat * Convert display utils to Emotion + convert euiFullHeight Sass mixin to Emotion + move @mixin euiFullHeight to `mixins/_helpers` instead of living in `_utility` * Document euiFullHeight under scroll utils - since that's where the className was documented * Convert scroll globals to Emotion (+ add logical CSS properties) - this requires adding a new `logicalCSSWithFallback` util, since overflow-inline/block is not yet supported on non-FF browsers + fix incorrect overflow mapping - inline should be x and block should be y * Convert .eui-isFocusable global to Emotion + convert mixin to accept the entire euiThemeContext instead of just the euiTheme to match all other mixins * Convert responsive globals to Emotion * Add snapshot test for quick checking of global styles output * Changelog * Remove `.eui-isFocusable` - not being used anywhere + already being handled by global reset

cla-checker-service · 2022-08-16T17:32:55Z

❌ Author of the following commits did not sign a Contributor Agreement:
fb72b85, 3ae838c, 9d7da0d, 7ef4061, 85669ec, 4f7a811, 0dd3a51, c7c06f2, 7060b3f, c12dd0e, ba8d863, ee7c0a7, cbeb27a

Please, read and sign the above mentioned agreement if you want to contribute to this project

miukimiu · 2022-08-22T12:22:50Z

Closing in favor of #6150

cchaos added 9 commits June 4, 2022 16:00

WIP: keys are working

fb72b85

WIP: All colors working for base and empty buttons

3ae838c

WIP: Updating icon button and removing more amsterdam overrides

9d7da0d

WIP: Cleanup and some snaps

7ef4061

WIP: Button groups mostly updated and overrides removed

85669ec

Fixing snaps

4f7a811

[EuiButton] Fully converted to Emotion

0dd3a51

- Uses new EuiButtonDisplay - Updates EuiButtonDisplay with more styles and fixes

Added some navigation cards to button example page

c7c06f2

cchaos added 2 commits June 21, 2022 19:03

Cleanup

7060b3f

Moar snaps

c12dd0e

cchaos mentioned this pull request Jun 24, 2022

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

Closed

49 tasks

cchaos added 2 commits June 23, 2022 22:40

Clean up styles functions and us EuiThemeProvider around ghost buttons

ba8d863

Merge remote-tracking branch 'origin/main' into emotion/button_colors

f9c17b3

cchaos added 2 commits June 27, 2022 11:39

Merge remote-tracking branch origin/main into emotion/button_colors

ad78752

Cleanup and CL

ee7c0a7

cchaos added the breaking change label Jun 27, 2022

cchaos changed the title ~~WIP [EuiButton] Convert to Emotion~~ [EuiButton] Convert to Emotion Jun 27, 2022

cchaos commented Jun 27, 2022

View reviewed changes

cchaos marked this pull request as ready for review June 27, 2022 16:05

miukimiu self-requested a review June 28, 2022 14:26

miukimiu reviewed Jun 29, 2022

View reviewed changes

cchaos added 2 commits June 29, 2022 10:41

Merge remote-tracking branch 'origin/main' into emotion/button_colors

f425cf5

Fixed button panel links and updated bottom bar usages of ghost to …

cbeb27a

…`text`

thompsongl self-requested a review July 25, 2022 15:24

Merge branch 'elastic:main' into emotion/button_colors

36531ee

miukimiu approved these changes Aug 10, 2022

View reviewed changes

Constance and others added 2 commits August 16, 2022 12:57

Merge remote-tracking branch 'upstream/main' into emotion/button_colors

4326a56

miukimiu mentioned this pull request Aug 22, 2022

[Emotion] Convert EuiButton #6150

Merged

11 tasks

miukimiu closed this Aug 22, 2022

miukimiu mentioned this pull request Sep 6, 2022

Navbar theme="dark" has not background (ghost) while in full dark theme has #6203

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[EuiButton] Convert to Emotion #5989

[EuiButton] Convert to Emotion #5989

cchaos commented Jun 21, 2022 •

edited

Loading

kibanamachine commented Jun 21, 2022

kibanamachine commented Jun 21, 2022

kibanamachine commented Jun 24, 2022

cchaos left a comment

cchaos Jun 21, 2022

cchaos Jun 21, 2022

kibanamachine commented Jun 27, 2022

miukimiu left a comment

miukimiu Jun 29, 2022

cchaos Jun 29, 2022

miukimiu Jun 29, 2022

cchaos Jun 29, 2022

kibanamachine commented Jun 29, 2022

kibanamachine commented Jul 26, 2022

morksinaanab commented Aug 3, 2022

thompsongl commented Aug 8, 2022

thompsongl commented Aug 9, 2022

miukimiu left a comment

cchaos commented Aug 10, 2022

miukimiu commented Aug 10, 2022

cla-checker-service bot commented Aug 16, 2022

miukimiu commented Aug 22, 2022

[EuiButton] Convert to Emotion #5989

[EuiButton] Convert to Emotion #5989

Conversation

cchaos commented Jun 21, 2022 • edited Loading

1. Moved Amsterdam-specific Sass overrides to default global_styling/ folder

2. New theme-specific (Amsterdam only for now) button styling functions

3. Button component updates

EuiButton

EuiButtonEmpty

EuiButtonIcon

EuiButtonGroup & Option

EuiButtonDisplay

EuiButtonDisplayContent

Other updates

Checklist

kibanamachine commented Jun 21, 2022

kibanamachine commented Jun 21, 2022

kibanamachine commented Jun 24, 2022

cchaos left a comment

Choose a reason for hiding this comment

cchaos Jun 21, 2022

Choose a reason for hiding this comment

cchaos Jun 21, 2022

Choose a reason for hiding this comment

kibanamachine commented Jun 27, 2022

miukimiu left a comment

Choose a reason for hiding this comment

miukimiu Jun 29, 2022

Choose a reason for hiding this comment

cchaos Jun 29, 2022

Choose a reason for hiding this comment

miukimiu Jun 29, 2022

Choose a reason for hiding this comment

cchaos Jun 29, 2022

Choose a reason for hiding this comment

kibanamachine commented Jun 29, 2022

kibanamachine commented Jul 26, 2022

morksinaanab commented Aug 3, 2022

thompsongl commented Aug 8, 2022

thompsongl commented Aug 9, 2022

miukimiu left a comment

Choose a reason for hiding this comment

cchaos commented Aug 10, 2022

miukimiu commented Aug 10, 2022

cla-checker-service bot commented Aug 16, 2022

miukimiu commented Aug 22, 2022

cchaos commented Jun 21, 2022 •

edited

Loading

1. Moved Amsterdam-specific Sass overrides to default `global_styling/` folder