[Emotion] Convert `EuiBottomBar` to Emotion by breehall · Pull Request #5823 · elastic/eui

breehall · 2022-04-20T14:17:19Z

Summary

Converts EuiBottomBar to Emotion
Additional changes include converting the original EuiBottomBar function into an internal utility component that is nested inside of a new component that wraps the the bottom bar in an EuiThemeProvider. This was done to lay the ground work for future enhancements.
Future enhancements include resolving #4158 by using colorMode: inverse on the bottom bar. This will ensure that the bottom bar always stands out in light mode and in dark mode because the background color of the bar will always be in contrast to the color mode.
This will also include adding a prop to EuiBottomBar that will consumers to choose light or dark as the colorMode for the bottom bar. If this prop is in use, the colorMode: inverse enhancement (mentioned above) will not take affect.
These enhancements will come at a later time to reduce the amount of disruption for consumers. The colorMode changes will affect the background color of the bottom bar, but until other components have been converted to Emotion, these changes will be out of sync.

Testing Note
With the changes made, the background color of EuiBottomBar should be a dark gray (euiTheme.colors.lightShade) in both light and dark mode. Previous, the background color was white.

Things to look out for when moving styles

Anything in the backlog that could be a quick fix for that component?
- Bottom bar too subtle to get properly noticed #4158 (TBD)
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?~~

QA

Do className, css, and style props work properly when passed by the consumer
Does the rendered class read as expected

Checklist

Merge Changes from EUI Main

Merging in the latest code from eui/main

Merging in the latest code from main

Merging in the latest code from the main branch

Pulling in the latest code from the main branch

…ottom_bar Pulling in code from main

…st tests

…ottom_bar Merging in the latest code from the main branch

…ility

kibanamachine · 2022-04-20T14:45:43Z

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

… that wraps the original inside of EuiThemeProvider to pass the dark theme to the entire component. Removed the shouldRenderCustomStyles test utility

cchaos · 2022-04-26T21:24:31Z

src/components/bottom_bar/bottom_bar.styles.ts

+  // Base
+  euiBottomBar: css`
+    ${euiShadowFlat(euiTheme, undefined, colorMode)};
+    background: ${euiTheme.colors.lightShade};


We still need this background color to render as it did previously. The sass variable is:

$euiHeaderDarkBackgroundColor: lightOrDarkTheme(shade($euiColorDarkestShade, 28%), shade($euiColorLightestShade, 50%)) !default;

But since we're only ever in dark theme, we just need to apply the second parameter as the color:

Suggested change

background: ${euiTheme.colors.lightShade};

background: ${shade(euiTheme.colors.lightestShade, .5)};

src/components/bottom_bar/bottom_bar.styles.ts

cchaos · 2022-04-26T21:28:00Z

src/components/bottom_bar/bottom_bar.styles.ts

+    color: ${euiTheme.colors.text};
+    animation: ${euiBottomBarAppear} ${euiTheme.animation.slow}
+      ${euiTheme.animation.resistance};
+    z-index: ${_euiBottomBarZHeader - 2};


I just pushed the z-index layering global theme vars to main.

Suggested change

z-index: ${_euiBottomBarZHeader - 2};

z-index: ${euiTheme.levels.header - 2};

Hm, it looks like this z-index should also only apply if the position is fixed or sticky.

cchaos · 2022-04-26T21:28:20Z

src/components/bottom_bar/bottom_bar.styles.ts

+//const _euiBottomBarColor = '#131317';
+const _euiBottomBarZHeader = 1000;


So neither of these should be needed anymore.

Suggested change

//const _euiBottomBarColor = '#131317';

const _euiBottomBarZHeader = 1000;

cchaos · 2022-04-26T21:28:43Z

src/components/bottom_bar/bottom_bar.styles.ts

+  }
+
+  100% {
+    transform: translateY(00%);


Suggested change

transform: translateY(00%);

transform: translateY(0%);

src/components/bottom_bar/bottom_bar.styles.ts

src/components/index.scss

src/components/bottom_bar/bottom_bar.styles.ts

breehall · 2022-04-27T20:05:21Z

src/components/bottom_bar/bottom_bar.tsx

      className
    );

+    const cssStyles = [


I added styles[position] back to the cssStyles array because I need to access the position to determine if the z-index should be added to the bottom bar. Since I already need it for that purpose, should we just attach the position styling in those style blocks instead of passing it in directly here?

cchaos · 2022-04-28T04:27:28Z

Jenkins, test this

kibanamachine · 2022-04-28T13:35:49Z

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

cchaos

🙌 Our first dark-theme only component!

Just to compare using already converted components (Avatar, HR, LoadingChart) rendered inside the bottom bar:

Global light mode

Global dark mode

LGTM. I tested in the 3 browsers and mobile. Gonna tag @thompsongl in on this one to ensure the wrapping component stuff is all what he expected.

cchaos · 2022-04-28T17:54:22Z

src/components/bottom_bar/bottom_bar.tsx

+export const EuiBottomBar = forwardRef<HTMLElement, EuiBottomBarProps>(
+  (props, ref) => {
+    const BottomBar = _EuiBottomBar;
+    return (
+      <EuiThemeProvider colorMode={'dark'}>
+        <BottomBar ref={ref} {...props} />
+      </EuiThemeProvider>
+    );
+  }
+);


Can you document this as a pattern for color-mode specific components in the Emotion styling wiki???

thompsongl

Just a couple nits. Nice work!

thompsongl · 2022-04-28T20:17:47Z

src/components/bottom_bar/bottom_bar.styles.ts

+
+export const euiBottomBarStyles = ({ euiTheme, colorMode }: UseEuiTheme) => ({
+  // Base
+  //Text color needs to be reapplied to properly scope the forced `colorMode`


Suggested change

//Text color needs to be reapplied to properly scope the forced `colorMode`

// Text color needs to be reapplied to properly scope the forced `colorMode`

thompsongl · 2022-04-28T20:17:59Z

src/components/bottom_bar/bottom_bar.styles.ts

+import { UseEuiTheme } from '../../services';
+import { shade } from '../../services/color';


Can these be combined?

kibanamachine · 2022-04-29T14:55:39Z

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

kibanamachine · 2022-04-29T15:59:10Z

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

kibanamachine · 2022-04-29T16:52:24Z

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

wiki/writing-styles-with-emotion.md

Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com>

kibanamachine · 2022-04-29T19:01:14Z

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

breehall and others added 9 commits February 28, 2022 10:57

Merge pull request #1 from elastic/main

4e1db78

Merge Changes from EUI Main

Merge branch 'main' of https://github.com/breehall/eui

0671ea5

Merging in the latest code from eui/main

Merge branch 'main' of https://github.com/breehall/eui

b2acbfe

Merging in the latest code from main

Merge branch 'main' of https://github.com/breehall/eui

9ccab1b

Merging in the latest code from the main branch

Merge branch 'main' of https://github.com/breehall/eui

91bfff4

Pulling in the latest code from the main branch

Merge branch 'main' of https://github.com/breehall/eui into emotion-b…

475bc12

…ottom_bar Pulling in code from main

Converted EuiBottomBar to Emotion and updated related EuiBottomBar je…

9b35152

…st tests

Merge branch 'main' of https://github.com/breehall/eui into emotion-b…

d294eef

…ottom_bar Merging in the latest code from the main branch

Updated EuiBottomBar tests to include the shouldRenderCustomStyles ut…

06fcd68

…ility

breehall mentioned this pull request Apr 20, 2022

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

Closed

49 tasks

breehall changed the title ~~Emotion bottom bar~~ [Emotion] Convert EuiBottomBar to Emotion Apr 20, 2022

breehall added 3 commits April 21, 2022 16:12

Updated the EuiBottomBar component by making creating a new component…

639b5cb

… that wraps the original inside of EuiThemeProvider to pass the dark theme to the entire component. Removed the shouldRenderCustomStyles test utility

Updated the EuiBottomBar props. Updated BottomBar snapshots

28a74d1

Resolved merge conflicts in index.scss. Changelog

fd7aff7

breehall marked this pull request as ready for review April 26, 2022 20:50

cchaos suggested changes Apr 26, 2022

View reviewed changes

Handle PR comments and resolve a merge conflict in index.scss

0c84ad1

breehall commented Apr 27, 2022

View reviewed changes

breehall requested a review from cchaos April 27, 2022 20:06

Re-adding index.scss file chanes

518690d

cchaos approved these changes Apr 28, 2022

View reviewed changes

cchaos requested a review from thompsongl April 28, 2022 17:56

thompsongl approved these changes Apr 28, 2022

View reviewed changes

Small PR change requests

6b5722e

Added documentation for colorMode specific component pattern creation

6b08296

Resolved merge conflict in Writing Styles with Emotion wiki page

99b57a1

cchaos reviewed Apr 29, 2022

View reviewed changes

wiki/writing-styles-with-emotion.md Show resolved Hide resolved

cchaos reviewed Apr 29, 2022

View reviewed changes

wiki/writing-styles-with-emotion.md Outdated Show resolved Hide resolved

breehall and others added 2 commits April 29, 2022 14:27

Update wiki/writing-styles-with-emotion.md

73d8e3b

Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com>

Update wiki/writing-styles-with-emotion.md

3ab6514

Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com>

breehall merged commit e07ee75 into elastic:main Apr 29, 2022

breehall mentioned this pull request May 16, 2022

Upgrade EUI to v58.0.0 elastic/kibana#132257

Merged

elizabetdev mentioned this pull request Jul 20, 2022

Bug with context menu in bottom bar on light mode #6063

Closed

breehall deleted the emotion-bottom_bar branch September 18, 2023 19:36

cee-chen mentioned this pull request Oct 23, 2023

[Canvas] Fix sharable runtime styles and remove color="ghost" where unnecessary elastic/kibana#169313

Merged

1 task

	background: ${euiTheme.colors.lightShade};
	background: ${shade(euiTheme.colors.lightestShade, .5)};

	z-index: ${_euiBottomBarZHeader - 2};
	z-index: ${euiTheme.levels.header - 2};

		//const _euiBottomBarColor = '#131317';
		const _euiBottomBarZHeader = 1000;

	//Text color needs to be reapplied to properly scope the forced `colorMode`
	// Text color needs to be reapplied to properly scope the forced `colorMode`

		import { UseEuiTheme } from '../../services';
		import { shade } from '../../services/color';

Conversation

breehall commented Apr 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Things to look out for when moving styles

QA

Checklist

Uh oh!

kibanamachine commented Apr 20, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cchaos commented Apr 28, 2022

Uh oh!

kibanamachine commented Apr 28, 2022

Uh oh!

cchaos left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thompsongl left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Apr 29, 2022

Uh oh!

kibanamachine commented Apr 29, 2022

Uh oh!

kibanamachine commented Apr 29, 2022

Uh oh!

Uh oh!

Uh oh!

kibanamachine commented Apr 29, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

breehall commented Apr 20, 2022 •

edited

Loading