Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CSS-in-JS] Context and foundation #4440

Merged
merged 29 commits into from
Feb 17, 2021

Conversation

thompsongl
Copy link
Contributor

@thompsongl thompsongl commented Jan 21, 2021

Summary

Current status: Resolve open questions and decide on timeline


Targeting feature/css-in-js feature branch.

Foundational work for #3912, establishing dependencies, contexts, and initial patterns for

  • Style generation via CSS-in-JS (Emotion)
  • Theme construction via a Proxy-based system with cascading and computed values
    • Proxy-based: allows for the theme system to reference its own values (for reuse or for computational manipulation)
    • Cascading: conceptually similar to Sass, where variable location and order are important
    • Extendable: allows for appending style variables to the EUI theme structure, scoped to a React context provider
    • Override-able: all theme tokens/variables can be altered by consumers
  • Theme consumption via React hook and HOC methods
  • Color mode support as first-class consideration
    • "Light" and "dark" mode (or defined through extensions/overrides) accounting
    • Theme consumption is scoped to the current color mode (set in the context provider)
**Read about the layers of the system**

Unbuilt theme

See euiThemeDefault
An unbuilt theme is a composed object of style values or computed functions.

Style values

Think design tokens or CSS property values. Ready to be consumed as-is in an application environment, using some JavaScript method of applying styles (i.e., emotion is not a requirement).

computed functions

These properties specify that the value depends upon some other value in the theme, in the shape of:

computed(
  ['sizes.euiSize'], // dependency array, referencing other properties in the theme object
  ([size]) => size * 2 // predicate. What to do with the dependency values
)

Theme system (built theme)

See EuiThemeDefault
A built theme by way of buildTheme, which transforms the object containing static style values and computed functions into a JavaScript Proxy object with handler traps. In this state, the theme is essentially inaccessible and immutable, that is, it requires getComputed to correctly order and access values and dependencies, and set() is disabled.

Computed theme

See EuiThemeContext
A consumable theme object in which all computed function values have been computed; all values are accessible and usable in an application environment.
Returned from getComputed, in the shape of:

getComputed(
  EuiThemeDefault, // Theme system (Proxy)
  {}, // Overrides object
  'light' // Color mode
)

Overrides

Compute-time value overrides for theme property values. Because a theme system is unchangeable, this mechanism allows for changing values at certain points during consumption.
The overrides object must match the partial shape of the theme system:

{ 
  sizes: {
    euiSize: 4
  }
}

Color mode

Think light and dark mode. A theme has built-in color mode support, using the colors property as a marker:

colors: {
  light: {...}
  dark : {...}
}

getComputed will only compute and return values in the specified current color mode.

**Read about the React-specific context**

EuiThemeProvider

Umbrella provider component that holds the various top-level theme configuration option providers: theme system, color mode, overrides; as well as the primary output provider: computed theme.
The actual computation for computed theme values takes place at this level, where the three inputs are known (theme system, color mode, overrides) and the output (computed theme) can be cached for consumption. Input changes are captured and the output is recomputed.

<EuiThemeProvider
  theme={DefaultEuiTheme}
  colorMode="light
  overrides={{}}
  />

All three props are optional. The default values for EUI will be used in the event that no configuration is provided. Note, however that colorMode switching will require consumers to maintain that app state.

useEuiTheme

A custom React hook that is returns the computed theme. This hook it little more than a wrapper around the useContext hook, accessing three of the top-level providers: computed theme, color mode, and overrides.

const [theme, colorMode, overrides] = useEuiTheme();

WithEuiTheme

A higher-order-component that wraps useEuiTheme for React class components.

Notable components:

  • EuiThemeProvider: React context provider for theme values and color mode selection
  • useEuiTheme: theme consumer in React hook form
  • withEuiTheme: theme consumer in React HOC form (for class components)

Notable modifications:
* EuiTable*: converted color usage to CSS-in-JS


Miscellaneous:

  • Snapshot testing (as currently configured) will result in generic emotion-${n} class names with the generated style object as part of the snapshot.
    • This seems good for EUI, but it also affects consumers
      • Consumers will need to use the @emotion/jest snapshot serializer to avoid class name churn.
      • Not ideal; unsure of any other solutions
    • During the conversion process, the snapshot diffs will look less than ideal when using shallow (a single wrapper element; DOM itself is unchanged):
-                         <div
-                           className="euiTableRowCell__mobileHeader euiTableRowCell--hideForDesktop"
+                         <EmotionCssPropInternal
+                           __EMOTION_LABEL_PLEASE_DO_NOT_USE__="EuiTableRowCell"
+                           __EMOTION_TYPE_PLEASE_DO_NOT_USE__="td"
+                           className="euiTableRowCell"
+                           style={
+                             Object {
+                               "width": undefined,
+                             }
+                           }
                              >
  • TypeScript support resulting in IDE autocomplete availability when consuming with useEuiTheme
    • 🎉
    • Extended theme variables can be also be accessed through generic type extensions

Open questions

  • Do we want to allow EUI styling apart from css prop (e.g. in Kibana)
  • What works in the css object passed from a project without the babel plugin, e.g.:
  • object? css={{ textAlign: 'center' }}
  • import css from eui? css={css`text-align: center`}
  • EUI does not always apply className to the outermost element, does a css prop on an object prop apply as expected
  • What do we do with the handful of createElement usages, replace with emotion’s createElement equivalent?
  • When css is applied on a custom component which element gets the style? What if there's a fragment at the top level?
  • What component(s) do we want to convert to using Emotion right now, if any? (tables, EuiMark)

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
  • 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

@kibanamachine
Copy link

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

@kibanamachine
Copy link

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

@kibanamachine
Copy link

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

@kibanamachine
Copy link

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

@kibanamachine
Copy link

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

@kibanamachine
Copy link

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

@kibanamachine
Copy link

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

@kibanamachine
Copy link

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

@thompsongl thompsongl marked this pull request as ready for review February 8, 2021 18:34

// TODO: `calc()` probably not the right solution
const sizes = {
euiSize: '16px',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on https://emotion.sh/docs/object-styles#numbers, it's probably best to go unitless.

Copy link
Contributor Author

@thompsongl thompsongl Feb 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, if we move to unitless values:

const styles = css`padding: ${sizes.euiSize}`

div css={styles} />

will not automatically add px.

But using div css={{padding: sizes.euiSize}} /> will automatically add px

🤔

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To add to that discrepancy, I noticed a funny way the calcs are rendered based on using the css or style tag.

When using the <div css={{}}> syntax it spits out the calc() with the formula intact

Screen Shot 2021-02-09 at 17 05 46 PM

When passing as a style tag <div style={{}}> it does the computation and spits out a calc() of the final value.

Screen Shot 2021-02-09 at 17 06 13 PM

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah the discrepancy is not great. In the latest commit I changed sizes to be unitless, which I like better than using calc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although it seems like most folks use strings and just deal with the awkward math because it eliminates any confusion about whether px needs to be added or not.

@thompsongl
Copy link
Contributor Author

thompsongl commented Feb 8, 2021

What works in the css object passed from a project without the babel plugin, e.g.:

  • object? css={{ textAlign: 'center' }}
  • import css from eui? css={csstext-align: center}

Depends on the element type:

  • EUI components will work with either, regardless of babel plugin/pragma usage
  • Other elements (native DOM included) will not work with either without babel plugin/pragma usage

(Will push some code in a test project that shows this) See https://github.com/thompsongl/cra-eui/tree/ref/emotion

import css from eui?

Haven't really explored this yet, but my first look shows that it might require some extra babel config, which raises questions for CRA/zero-config users.


// Status
euiColorSuccess: computed(
['colors.euiColorSecondary'],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we'll be able to get autocomplete support for these dependency strings when we upgrade to TS 4.1

Copy link
Contributor

@chandlerprall chandlerprall Feb 9, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be a good place for the accessor types. We can take another pass at the strings parsing/completing, but I was unable to get accurate results with that pattern. Maybe instead of computed(...) it could have the shape computed.colors.euiColorSecondary with the proxy accessor, and originating from a custom computed variable would allow detection that it is a computation.

EDIT: still need the computation function, so perhaps computed(euitheme.colors.euiColorSecondary, ...) and not overloading the purpose of euitheme or whatever we call the head of that path.

@kibanamachine
Copy link

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

@kibanamachine
Copy link

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

@thompsongl
Copy link
Contributor Author

☝️ Cleaner starting point. Set up for TS, testing, linting, etc., but no existing components (or src/ code) have been altered.

@kibanamachine
Copy link

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

@thompsongl
Copy link
Contributor Author

jenkins test this

Error: Timed out waiting for: http-get://localhost:9999

@kibanamachine
Copy link

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

@thompsongl
Copy link
Contributor Author

jenkins test this

flaky EuiContextMenu

@kibanamachine
Copy link

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

Copy link
Contributor

@chandlerprall chandlerprall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantastic foundation to build on!

Copy link
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🥇 Great first merge into feature branch. We'll tackle the checklist items as future PR's against the feature branch.

@kibanamachine
Copy link

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

@thompsongl
Copy link
Contributor Author

CI has been pretty unreliable today; merging this as-is into the feature branch. Previous build passed and only change since was a new .md file.

@thompsongl thompsongl merged commit 156f7fe into elastic:feature/css-in-js Feb 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants