[CSS-in-JS] Convert EuiBreakpoint Mixin to Emotion #6057

breehall · 2022-07-18T15:52:09Z

Summary

Converted the euiBreakpoint mixin to Emotion and created a hook for use within components. Unlike the Sass mixin, the JS mixin allows creating media queries with just (min-width) and (max-width) via the 0 and Infinity args.
Example:
euiBreakpoint(['m', Infinity]) becomes @media only screen and (min-width: 768px)
euiBreakpoint([0, 'm']) becomes @media only screen and (max-width: 767px)

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]~~(https://github.com/elastic/eui/blob/main/wiki/documentation-guidelines.md#adding-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

… validation functions

…n conditions to allow consumers the flexibility of obtaining a min-width, max-width, or min and max width media query from a variety of different scenarios

kibanamachine · 2022-07-18T16:18:14Z

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

breehall · 2022-07-18T17:48:18Z

👋🏾 Hey Greg and Constance, I can go through and clean up some of my comments after the reviews are good. I just wanted to make sure that my thought process matched our goal here. Also, I know the validation might look like a lot, but the more I added conditions to account for 0, infinity, and undefined, I realized there were a few holes.

kibanamachine · 2022-07-18T18:12:25Z

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

cee-chen · 2022-07-18T20:58:47Z

src/global_styling/mixins/_responsive.ts

+  | '0'
+  | 'infinity'


We should likely be using the numeric version of these for easier type checking:

Suggested change

| '0'

| 'infinity'

| 0

| Infinity

thompsongl · 2022-07-18T21:27:18Z

Breaking Change
The previous version of euiBreakpoint allowed single element arrays to be passed.

I don't think we need to consider this a breaking change. The previous mixin was written in Sass and this is more of a port than a refactor. Any one choosing to "upgrade" to the JS version will be working with new APIs outside of Sass. Probably worth noting the difference(s) in a code comment in responsive.ts

- Prefer types from theme instead of the breakpoints service (which likely needs to be refactored to use the theme) - simplify output to use array logic to output/concatenate a string - Remove undefined type - improve unit tests: - catch console warn conditions - don't snapshot invalid/uncommon size combos

cee-chen · 2022-07-19T03:11:58Z

@breehall I realize I was a little unspecific with what I wanted in the euiBreakpoint 0/Infinity API change, for which I apologize (I also totally did not realize xs was already 0 or that Infinity has Typescript shenanigans 🙈). FWIW, I don't think we need undefined as an arg, just 0/Infinity, which helps simplify checks/testing some.

I've created a local spike that simplifies the checks/warnings as well as adds more specific unit tests. Let me know what you think! If you think it's an improvement, feel free to cherry-pick it in.

FYI, we'll almost certainly want to document this new function in https://elastic.github.io/eui/#/theming/breakpoints/values. There might need to be more updates down the road as src/services/breakpoint.ts currently does not use euiTheme.breakpoints when it likely should, but for now, we should at minimum add useEuiBreakpoints() in there with example usage.

[PR feedback] Simplify branching logic and typing

kibanamachine · 2022-07-19T15:16:22Z

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

src/global_styling/mixins/_responsive.test.ts

kibanamachine · 2022-07-19T16:58:36Z

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

cee-chen · 2022-07-19T20:57:26Z

@thompsongl Any feedback on the revised function/logic?

@breehall I think this is good to move forward with removing the breaking change changelog line and adding documentation to the Breakpoints CSS-in-JS utils!

thompsongl · 2022-07-19T21:24:51Z

Nope, let's start adding documentation 👍

…into emotion/breakpoint Merging in latest code from eui/breakpoint

breehall · 2022-07-19T23:30:47Z

I think this is good to move forward with removing the breaking change changelog line and adding documentation to the Breakpoints CSS-in-JS utils!

@constancecchen Gotcha! I've added documentation to the Breakpoints page in the docs. I only added an entry for the hook because there's no variation between the hook and the function. Part of me would like to see two examples here, but I believe that would break a pattern as no other examples have two code snippets.

kibanamachine · 2022-07-19T23:53:58Z

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

thompsongl · 2022-07-20T14:17:40Z

Part of me would like to see two examples here, but I believe that would break a pattern as no other examples have two code snippets.

Let's add a new entry for the function version. Not just another snippet, but another ThemeExample section and specify that they accept the same arguments.

…or the euiBreakpoing function

kibanamachine · 2022-07-20T15:30:01Z

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

upcoming_changelogs/6057.md

cee-chen · 2022-07-20T16:27:10Z

Let's add a new entry for the function version. Not just another snippet, but another ThemeExample section and specify that they accept the same arguments.

@thompsongl I don't think we do this anywhere else, actually. It looks like if a non-hook and a hook util both exist, we document the hook version only (the most likely version for consumers to use, the non-hook version is most likely to only be used internally by EUI). For example, our color utilities: https://eui.elastic.co/pr_6057/#/theming/colors/utilities

src-docs/src/views/theme/breakpoints/_breakpoints_js.tsx

Co-authored-by: Constance <[email protected]>

kibanamachine · 2022-07-20T17:51:44Z

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

breehall · 2022-07-20T17:59:20Z

@constancecchen I'm not sure why I can't respond to the comment about the imports, but as you commented, I just pushed a commit cleaning them up.

cee-chen · 2022-07-20T17:59:43Z

Haha sike! Nobody saw that last comment

@thompsongl I think this is good to merge on my end - any objections/change requests to the docs?

kibanamachine · 2022-07-20T18:24:08Z

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

thompsongl

Might as well round out the example. Otherwise this all LGTM!

src-docs/src/views/theme/breakpoints/_breakpoints_js.tsx

Co-authored-by: Greg Thompson <[email protected]>

kibanamachine · 2022-07-20T21:32:52Z

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

breehall added 2 commits July 14, 2022 16:21

Converted euiBreakpoint mixin. Added additional parameter options and…

d8ba2c7

… validation functions

Retooled euiBreakpoint mixin by adding additional logic and validatio…

3f7b919

…n conditions to allow consumers the flexibility of obtaining a min-width, max-width, or min and max width media query from a variety of different scenarios

breehall changed the title ~~[Emotion} Convert EuiBreakpoint Mixin to Emotion~~ [Emotion] Convert EuiBreakpoint Mixin to Emotion Jul 18, 2022

breehall added the breaking change label Jul 18, 2022

breehall added 3 commits July 18, 2022 13:38

CHANGELOG

bf7588a

Clean up a rogue curly brace

2101005

CHANGELOG

3d0a70f

breehall requested review from thompsongl and cee-chen July 18, 2022 17:46

breehall marked this pull request as ready for review July 18, 2022 17:46

breehall changed the title ~~[Emotion] Convert EuiBreakpoint Mixin to Emotion~~ [CSS-in-JS] Convert EuiBreakpoint Mixin to Emotion Jul 18, 2022

cee-chen reviewed Jul 18, 2022

View reviewed changes

Merge pull request #2 from constancecchen/emotion/breakpoint

9e77bd6

[PR feedback] Simplify branching logic and typing

cee-chen reviewed Jul 19, 2022

View reviewed changes

src/global_styling/mixins/_responsive.test.ts Outdated Show resolved Hide resolved

Fix commented out assertion

6589f3a

breehall added 2 commits July 19, 2022 19:23

Added documentation for euiBreakpoint mixin. Updated changelog

8ca2c76

Merge branch 'emotion/breakpoint' of https://github.com/breehall/eui …

c6a958a

…into emotion/breakpoint Merging in latest code from eui/breakpoint

breehall removed the breaking change label Jul 19, 2022

Update Breakpoint Theming documentation page with an addition entry f…

65e7ef3

…or the euiBreakpoing function

cee-chen reviewed Jul 20, 2022

View reviewed changes

upcoming_changelogs/6057.md Outdated Show resolved Hide resolved

cee-chen reviewed Jul 20, 2022

View reviewed changes

src-docs/src/views/theme/breakpoints/_breakpoints_js.tsx Outdated Show resolved Hide resolved

breehall and others added 2 commits July 20, 2022 13:23

Update src-docs/src/views/theme/breakpoints/_breakpoints_js.tsx

4c18bc0

Co-authored-by: Constance <[email protected]>

Update upcoming_changelogs/6057.md

161856c

Co-authored-by: Constance <[email protected]>

Clean up import statements

6a6293e

thompsongl reviewed Jul 20, 2022

View reviewed changes

src-docs/src/views/theme/breakpoints/_breakpoints_js.tsx Outdated Show resolved Hide resolved

Update src-docs/src/views/theme/breakpoints/_breakpoints_js.tsx

30bbd7a

Co-authored-by: Greg Thompson <[email protected]>

thompsongl approved these changes Jul 20, 2022

View reviewed changes

breehall merged commit 56e7f6f into elastic:main Jul 20, 2022

breehall mentioned this pull request Jul 21, 2022

[Emotion] Convert EuiDescriptionList #5971

Merged

11 tasks

cee-chen mentioned this pull request Jul 27, 2022

Upgrade EUI to v62.0.3 elastic/kibana#136865

Closed

cee-chen mentioned this pull request Aug 9, 2022

Upgrade EUI to v62.0.3 elastic/kibana#138351

Merged

walterra mentioned this pull request Jan 3, 2023

[ML] Migrate SCSS use of euiBreakpoint to emotion. elastic/kibana#148277

Closed

5 tasks

breehall deleted the emotion/breakpoint branch September 18, 2023 19:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CSS-in-JS] Convert EuiBreakpoint Mixin to Emotion #6057

[CSS-in-JS] Convert EuiBreakpoint Mixin to Emotion #6057

breehall commented Jul 18, 2022 •

edited

Loading

kibanamachine commented Jul 18, 2022

breehall commented Jul 18, 2022

kibanamachine commented Jul 18, 2022

cee-chen Jul 18, 2022

thompsongl commented Jul 18, 2022

cee-chen commented Jul 19, 2022 •

edited

Loading

kibanamachine commented Jul 19, 2022

kibanamachine commented Jul 19, 2022

cee-chen commented Jul 19, 2022

thompsongl commented Jul 19, 2022

breehall commented Jul 19, 2022 •

edited

Loading

kibanamachine commented Jul 19, 2022

thompsongl commented Jul 20, 2022

kibanamachine commented Jul 20, 2022

cee-chen commented Jul 20, 2022 •

edited

Loading

kibanamachine commented Jul 20, 2022

breehall commented Jul 20, 2022

cee-chen commented Jul 20, 2022

kibanamachine commented Jul 20, 2022

thompsongl left a comment •

edited

Loading

kibanamachine commented Jul 20, 2022

[CSS-in-JS] Convert EuiBreakpoint Mixin to Emotion #6057

[CSS-in-JS] Convert EuiBreakpoint Mixin to Emotion #6057

Conversation

breehall commented Jul 18, 2022 • edited Loading

Summary

Checklist

kibanamachine commented Jul 18, 2022

breehall commented Jul 18, 2022

kibanamachine commented Jul 18, 2022

cee-chen Jul 18, 2022

Choose a reason for hiding this comment

thompsongl commented Jul 18, 2022

cee-chen commented Jul 19, 2022 • edited Loading

kibanamachine commented Jul 19, 2022

kibanamachine commented Jul 19, 2022

cee-chen commented Jul 19, 2022

thompsongl commented Jul 19, 2022

breehall commented Jul 19, 2022 • edited Loading

kibanamachine commented Jul 19, 2022

thompsongl commented Jul 20, 2022

kibanamachine commented Jul 20, 2022

cee-chen commented Jul 20, 2022 • edited Loading

kibanamachine commented Jul 20, 2022

breehall commented Jul 20, 2022

cee-chen commented Jul 20, 2022

kibanamachine commented Jul 20, 2022

thompsongl left a comment • edited Loading

Choose a reason for hiding this comment

kibanamachine commented Jul 20, 2022

breehall commented Jul 18, 2022 •

edited

Loading

cee-chen commented Jul 19, 2022 •

edited

Loading

breehall commented Jul 19, 2022 •

edited

Loading

cee-chen commented Jul 20, 2022 •

edited

Loading

thompsongl left a comment •

edited

Loading