[CSS-in-JS] Global style reset with `emotion` #5122

thompsongl · 2021-09-01T17:15:04Z

Summary

Replacing the Sass global reset file with a 1:1 conversion using Global from @emtion/react
Enabled in the docs; try to find any differences as compared to the production site 🕵️

Diff of the CSS vs. emotion output can be seen in this gist. Note that there are prefixing differences because emotion uses stylis for preprocessing.

This PR also changes the isDefaultTheme utility to isLegacyTheme.

Checklist

Check against all themes for compatibility 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 the any docs examples

- [ ] Added or updated jest tests

Checked for breaking changes and labeled appropriately
Checked for accessibility including keyboard-only and screenreader modes

~~- [ ] A changelog entry exists and is marked appropriately~~

thompsongl · 2021-09-01T17:22:54Z

src/global_styling/mixins/_typography.ts

+
+// Our base fonts
+
+export const useEuiFont = () => {


Thinking that we should also provide non-hook versions of these utilities that do not use useEuiTheme directly, but accept a theme parameter. This style of hook can then just consume that function and layer on context awareness.

I still think this is a good idea, but I'm going to do a follow-up PR because there are several other hooks that could use the same two-step treatment.

kibanamachine · 2021-09-01T17:33:04Z

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

kibanamachine · 2021-09-01T18:24:39Z

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

thompsongl · 2021-09-09T20:44:36Z

src/global_styling/mixins/_typography.ts

+    -webkit-text-size-adjust: 100%;
+    -ms-text-size-adjust: 100%;


Can be safely condensed to text-size-adjust: https://developer.mozilla.org/en-US/docs/Web/CSS/text-size-adjust#browser_compatibility

kibanamachine · 2021-09-09T20:59:58Z

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

thompsongl · 2021-09-09T21:03:39Z

Notable omissions from the diff gist:

-moz-selection: Legacy; no longer required (https://developer.mozilla.org/en-US/docs/Web/CSS/::selection#syntax)
scrollbar-color: Only supported by Firefox, but styles appear correct without it (https://developer.mozilla.org/en-US/docs/Web/CSS/scrollbar-color)
~~Still researching missing font-family values~~

cchaos · 2021-09-09T21:35:32Z

Still researching missing font-family values

Because I removed those 😆 and reduced the font-family stack to a much smaller, necessary stack.

thompsongl · 2021-09-09T21:36:58Z

Because I removed those 😆 and reduced the font-family stack to a much smaller, necessary stack.

Oh wow good thing I haven't actually spent any time looking into yet, then 😌

kibanamachine · 2021-09-09T21:56:16Z

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

thompsongl · 2021-09-10T14:45:57Z

Opening this up for review. @cchaos let's decide what to do with the various variables that don't (yet) exist in the JS theme that the global file relies on.

kibanamachine · 2021-09-10T14:58:16Z

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

cchaos · 2021-09-14T17:34:31Z

Issues

I'm just documenting issues as I'm finding them in the diff (I haven't looked at the new reset file yet), not delegating anyone to fix unless you know how to.

1. The compile order is now backwards. It seems the reset is coming in after the component styles.

This is clear in the example of input focus rings.

Correct

Incorrect (this PR)

2. Shouldn't be converting colors from hex to RGB

This is apparent in the Diff (but I see different results in the Dev Console locally).

I think we'll need to add .hex() to any colors we want in hex. Chroma deals in rgba (from @thompsongl)

We may want to think on this one a bit then...

3. Stripping out quotes from font-family 👍

Outdated

This is also happening in the Diff but not locally. But certain font families require the quotes to ensure it's finding the right one. I don't think this conversion is going to work as expected.

4. Using `calc`s for negative values 👍

Outdated

Not sure if this is truly an issue, but just an oddity to point out. We should maybe look into the performance between straight values and `calc()`.

5. `stylis` seems to be wrong right with its prefixing 🤦‍♀️

Outdated

For example `::-moz-selection` is absolutely necessary. Compare:

Correct

Incorrect (this PR)

Another example is checking for experimental flags like content: none which is apparently not supported by any browser, yet it removed the previous content: '' because it just saw two duplicate properties.

6. Linter vs compiled CSS

This is just an observation, not necessarily an issue, but it looks like it will convert single quotes to double, but our Sass linter requires single quotes, so it just feels odd that we have reverse linter of what gets compiled. The same goes for the appending of px to 0 values.

kibanamachine · 2021-09-14T18:05:58Z

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

thompsongl · 2021-09-14T19:03:28Z

The compile order is now backwards. It seems the reset is coming in after the component styles.

Not entirely sure how to solve this one yet. Now that the global styles are injected after the static css file loads, we may need something like CacheProvider to specify where to inject the globals (i.e., before the static file). Note that this will not be a problem once EUI components are converted; global styles are always injected before non-global styles when done via Emotion.

Default will mean something different when Amsterdam becomes default so I want to head this off now by checking for “Legacy”

…need it

… at the global level

…a provider" This reverts commit 127f12e.

kibanamachine · 2021-09-30T15:28:36Z

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

thompsongl · 2021-09-30T21:33:51Z

though one last think you might want to check is the TS by changing the app_view.js to .tsx

Looking at this but the imported png files cause the build to break

…in-js/reset # Conflicts: # src-docs/src/views/app_view.js # src/global_styling/index.scss # src/themes/eui-amsterdam/global_styling/index.scss

cchaos · 2021-09-30T21:44:16Z

We don't have to actually keep it converted to TS, i just wanted you to check the provider in a TS file. When I converted I was getting a TS error around the Provider. But as I just tried again, I'm not seeing one.

thompsongl · 2021-09-30T21:51:28Z

Oh I see. No TS errors regarding Providers in that file.

…/reset

kibanamachine · 2021-10-01T21:04:12Z

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

thompsongl · 2021-10-05T20:53:25Z

Added docs for EuiProvider, although the location is an afterthought at this point. Will likely refactor after #5227 and after a decision is made on the location of EuiThemeProvider (likely moves from services/ to components/).

kibanamachine · 2021-10-05T21:05:37Z

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

src/global_styling/index.scss

kibanamachine · 2021-10-06T18:58:50Z

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

thompsongl · 2021-10-06T21:37:44Z

The main CI job succeeded but the docs deployment is stuck. Merging without running again.

* emotion as peerDep; babel and typescript * jest config * isDefaultTheme util * CL TODO * update snapshots * [CSS-in-JS] [Breaking] Deprecating EUI theme to Legacy (#5222) * [CSS-in-JS] Global style reset with `emotion` (#5122) * global reset with emotion * amsterdam overrides * text-size-adjust * ams reset * invert ::selection * Renamed `isDefaultTheme` utility to `isLegacyTheme` Default will mean something different when Amsterdam becomes default so I want to head this off now by checking for “Legacy” * Quick cleanup * Fixed up scrollbar function * Removed `useEuiFont` in favor of a single reset on the elements that need it * Added `body` key to global `font` for setting base body font settings at the global level * Some more fixes based on new diff * global reset with emotion * amsterdam overrides * text-size-adjust * ams reset * invert ::selection * Renamed `isDefaultTheme` utility to `isLegacyTheme` Default will mean something different when Amsterdam becomes default so I want to head this off now by checking for “Legacy” * Quick cleanup * Fixed up scrollbar function * Removed `useEuiFont` in favor of a single reset on the elements that need it * Added `body` key to global `font` for setting base body font settings at the global level * Some more fixes based on new diff * Pulling our reset from global styles * SVG `hacks` directly in reset file * Fixed reset import * euiprovider; reset styles * use euiprovider with emotion/cache in docs * Decision clean up * Fixing a few things for legacy dark mode based on not having a provider * Revert "Fixing a few things for legacy dark mode based on not having a provider" This reverts commit 127f12e. * Actually keeping `colorMode` even if `theme = null` * Fixing merge * Moved reset sass folder to `legacy` * fix wiki * euiprovider docs * fix provider docs * remove commented import Co-authored-by: cchaos <caroline.horn@elastic.co> * [CSS-in-JS] Add provider to generated codesandbox examples (#5253) * add provider to generated codesandbox examples * indentation * euithemeprovider -> euiprovider * legacy theme * fix snapshot tests * [CSS-in-JS] Provider for fullscreen examples; Context refactor (#5309) * refactor to extract context component * refactor for fewer updates on route changes * refactor AppView to function component * [CSS-in-JS] [Breaking] Better Getting Started page (#5299) * Moved all color Sass files to their respective theme folders - Duplicates variables, but makes it actually capable of customization - New files for code & vis colors that weren’t theme specific * Remove outdated theming wiki page * Cleanup (in MD files too) * fix heading level * update changelog page regex to fully remove the master section * manual merge * update release script changelog regex * Cleanup some docs * Hide LanguageSelector tour on Getting Started page * Some GS page cleanup * PR feedback * cl * Apply suggestions from code review Co-authored-by: Constance <constancecchen@users.noreply.github.com> * lint fix * make amsterdam default * provider updates * fix cypress styles * PR feedback: - Removed blank mixins file - Added `yarn add` block for dependencies - Added props table to bottom of Provider docs page * move keydown inside useeffect * Reusing correct Sass customization snippet under Customizing * fix cypress path * update export Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com> Co-authored-by: cchaos <caroline.horn@elastic.co> Co-authored-by: Constance <constancecchen@users.noreply.github.com>

thompsongl commented Sep 1, 2021

View reviewed changes

thompsongl force-pushed the css-in-js/reset branch from 32512c8 to 468ebd5 Compare September 1, 2021 18:00

thompsongl mentioned this pull request Sep 1, 2021

[Meta][CSS-in-JS] Switching to emotion #3912

Open

24 tasks

thompsongl commented Sep 9, 2021

View reviewed changes

thompsongl added 2 commits September 9, 2021 16:31

global reset with emotion

e79c771

amsterdam overrides

885b5f1

thompsongl force-pushed the css-in-js/reset branch from 3c5bbf9 to 885b5f1 Compare September 9, 2021 21:31

text-size-adjust

f586690

thompsongl added the skip-changelog label Sep 10, 2021

thompsongl requested a review from cchaos September 10, 2021 14:46

thompsongl marked this pull request as ready for review September 10, 2021 14:46

ams reset

644551f

invert ::selection

10d79d0

cchaos added 5 commits September 14, 2021 15:17

Renamed isDefaultTheme utility to isLegacyTheme

6516e8a

Default will mean something different when Amsterdam becomes default so I want to head this off now by checking for “Legacy”

Quick cleanup

e32f3e0

Fixed up scrollbar function

1adf4a8

Removed useEuiFont in favor of a single reset on the elements that …

72ce95a

…need it

Added body key to global font for setting base body font settings…

ed6b6f0

… at the global level

cchaos and others added 8 commits September 30, 2021 10:01

SVG hacks directly in reset file

0bcbb0a

Fixed reset import

367aaba

euiprovider; reset styles

42751cd

use euiprovider with emotion/cache in docs

4d1b933

Decision clean up

5763a43

Fixing a few things for legacy dark mode based on not having a provider

38b61b7

Revert "Fixing a few things for legacy dark mode based on not having …

5bea5bd

…a provider" This reverts commit 127f12e.

Actually keeping colorMode even if theme = null

b2dcb74

thompsongl force-pushed the css-in-js/reset branch from 89a3bee to b2dcb74 Compare September 30, 2021 15:01

Merge branch 'css-in-js/reset' of github.com:thompsongl/eui into css-…

efc1f9e

…in-js/reset # Conflicts: # src-docs/src/views/app_view.js # src/global_styling/index.scss # src/themes/eui-amsterdam/global_styling/index.scss

cchaos added 4 commits October 1, 2021 15:38

Merge remote-tracking branch origin/css-in-js/breaking into css-in-js…

139c2fe

…/reset

Fixing merge

979193f

Moved reset sass folder to legacy

1d3b4d4

fix wiki

5524d46

euiprovider docs

51f3386

fix provider docs

5055047

thompsongl commented Oct 6, 2021

View reviewed changes

src/global_styling/index.scss Outdated Show resolved Hide resolved

remove commented import

79c0506

thompsongl merged commit 3b60df7 into elastic:css-in-js/breaking Oct 6, 2021

thompsongl mentioned this pull request Oct 18, 2021

[EuiProvider] App configuration provider #4439

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CSS-in-JS] Global style reset with `emotion` #5122

[CSS-in-JS] Global style reset with `emotion` #5122

thompsongl commented Sep 1, 2021 •

edited

Loading

thompsongl Sep 1, 2021

thompsongl Sep 10, 2021

kibanamachine commented Sep 1, 2021

kibanamachine commented Sep 1, 2021

thompsongl Sep 9, 2021 •

edited

Loading

kibanamachine commented Sep 9, 2021

thompsongl commented Sep 9, 2021 •

edited

Loading

cchaos commented Sep 9, 2021 •

edited

Loading

thompsongl commented Sep 9, 2021

kibanamachine commented Sep 9, 2021

thompsongl commented Sep 10, 2021

kibanamachine commented Sep 10, 2021

cchaos commented Sep 14, 2021 •

edited

Loading

kibanamachine commented Sep 14, 2021

thompsongl commented Sep 14, 2021

kibanamachine commented Sep 30, 2021

thompsongl commented Sep 30, 2021

cchaos commented Sep 30, 2021

thompsongl commented Sep 30, 2021

kibanamachine commented Oct 1, 2021

thompsongl commented Oct 5, 2021 •

edited

Loading

kibanamachine commented Oct 5, 2021

kibanamachine commented Oct 6, 2021

thompsongl commented Oct 6, 2021

[CSS-in-JS] Global style reset with emotion #5122

[CSS-in-JS] Global style reset with emotion #5122

Conversation

thompsongl commented Sep 1, 2021 • edited Loading

Summary

Checklist

thompsongl Sep 1, 2021

Choose a reason for hiding this comment

thompsongl Sep 10, 2021

Choose a reason for hiding this comment

kibanamachine commented Sep 1, 2021

kibanamachine commented Sep 1, 2021

thompsongl Sep 9, 2021 • edited Loading

Choose a reason for hiding this comment

kibanamachine commented Sep 9, 2021

thompsongl commented Sep 9, 2021 • edited Loading

cchaos commented Sep 9, 2021 • edited Loading

thompsongl commented Sep 9, 2021

kibanamachine commented Sep 9, 2021

thompsongl commented Sep 10, 2021

kibanamachine commented Sep 10, 2021

cchaos commented Sep 14, 2021 • edited Loading

Issues

1. The compile order is now backwards. It seems the reset is coming in after the component styles.

2. Shouldn't be converting colors from hex to RGB

3. Stripping out quotes from font-family 👍

4. Using calcs for negative values 👍

5. stylis seems to be wrong right with its prefixing 🤦‍♀️

6. Linter vs compiled CSS

kibanamachine commented Sep 14, 2021

thompsongl commented Sep 14, 2021

kibanamachine commented Sep 30, 2021

thompsongl commented Sep 30, 2021

cchaos commented Sep 30, 2021

thompsongl commented Sep 30, 2021

kibanamachine commented Oct 1, 2021

thompsongl commented Oct 5, 2021 • edited Loading

kibanamachine commented Oct 5, 2021

kibanamachine commented Oct 6, 2021

thompsongl commented Oct 6, 2021

[CSS-in-JS] Global style reset with `emotion` #5122

[CSS-in-JS] Global style reset with `emotion` #5122

thompsongl commented Sep 1, 2021 •

edited

Loading

thompsongl Sep 9, 2021 •

edited

Loading

thompsongl commented Sep 9, 2021 •

edited

Loading

cchaos commented Sep 9, 2021 •

edited

Loading

cchaos commented Sep 14, 2021 •

edited

Loading

4. Using `calc`s for negative values 👍

5. `stylis` seems to be wrong right with its prefixing 🤦‍♀️

thompsongl commented Oct 5, 2021 •

edited

Loading