[CSS-in-JS] Convert EuiBreakpoint Mixin to Emotion by breehall · Pull Request #6057 · elastic/eui

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 <constancecchen@users.noreply.github.com>

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 <thompson.glowe@gmail.com>

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 PRs with breaking changes. (Don't delete - used for automation) 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 cee-chen and thompsongl 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 PRs with breaking changes. (Don't delete - used for automation) 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 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 <constancecchen@users.noreply.github.com>

Update upcoming_changelogs/6057.md

161856c

Co-authored-by: Constance <constancecchen@users.noreply.github.com>

Clean up import statements

6a6293e

thompsongl reviewed Jul 20, 2022

View reviewed changes

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

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

30bbd7a

Co-authored-by: Greg Thompson <thompson.glowe@gmail.com>

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

Conversation

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

Uh oh!

Summary

Checklist

Uh oh!

kibanamachine commented Jul 18, 2022

Uh oh!

breehall commented Jul 18, 2022

Uh oh!

kibanamachine commented Jul 18, 2022

Uh oh!

cee-chen Jul 18, 2022

Choose a reason for hiding this comment

Uh oh!

thompsongl commented Jul 18, 2022

Uh oh!

cee-chen commented Jul 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kibanamachine commented Jul 19, 2022

Uh oh!

Uh oh!

kibanamachine commented Jul 19, 2022

Uh oh!

cee-chen commented Jul 19, 2022

Uh oh!

thompsongl commented Jul 19, 2022

Uh oh!

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

Uh oh!

Uh oh!

kibanamachine commented Jul 19, 2022

Uh oh!

thompsongl commented Jul 20, 2022

Uh oh!

kibanamachine commented Jul 20, 2022

Uh oh!

Uh oh!

cee-chen commented Jul 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

kibanamachine commented Jul 20, 2022

Uh oh!

breehall commented Jul 20, 2022

Uh oh!

cee-chen commented Jul 20, 2022

Uh oh!

kibanamachine commented Jul 20, 2022

Uh oh!

thompsongl left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kibanamachine commented Jul 20, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

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