[Emotion] Convert EuiFlexGroup, EuiFlexItem, and EuiFlexGrid by cee-chen · Pull Request #6270 · elastic/eui

cee-chen · 2022-09-27T21:42:24Z

Summary

I strongly recommend following along by commit (due to the high amount of snapshot noise).

Converted EuiFlexGroup, EuiFlexGrid, and EuiFlexItem to Emotion
EuiFlexItem now supports grow={0}
Breaking changes
- EuiFlexGrid no longer supports columns={0} (use 1-4 or an EuiFlexGroup instead)
- EuiFlexGrid now uses modern display: grid CSS
- EuiFlexGroup, EuiFlexGrid, and EuiFlexItem now use modern gap CSS instead of margins and negative margins
- EuiFlexGroup no longer applies responsive styles to column or columnReverse directions

closes #5529
closes #4140

Things to look out for when moving styles

Anything in the backlog that could be a quick fix for that component?
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
Delete any className maps if not being used in Kibana. If used in Kibana, consider converting to a data attribute instead
Can any still existing .js files be converted to TS? (e.g., docs files)

~~- [ ] All animations or transitions are wrapped in euiCanAnimate~~
~~- [ ] Convert global Sass vars to JS version like sizing~~
~~- [ ] Convert component-specific Sass vars to exported JS versions~~
~~- [ ] Delete any Amsterdam override .scss files~~

QA

Does the output CSS match the previous CSS / as expected
Does the rendered class read as expected
shouldRenderCustomStyles() passes with parent component ~~and any nested childProps (e.g. tooltipProps, buttonProps)~~

Checklist

Checked in mobile
Checked in Chrome, Safari, Edge, and Firefox
Props have proper autodocs and playground toggles
Added or updated jest and cypress tests
Checked for breaking changes and labeled appropriately
A changelog entry exists and is marked appropriately

~~- [ ] Checked in both light and dark modes~~
- [ ] Added documentation
~~- [ ] Checked Code Sandbox works for any docs examples~~
~~- [ ] Checked for accessibility including keyboard-only and screenreader modes~~
~~- [ ] Updated the Figma library counterpart~~

+ convert gutter CSS to `gap` + remove className maps + clean up unnecessary `...rest` cast + convert nested EuiFlexItem to flattened styles within flex item styles - realistically, who's going to be using EuiFlexItem outside of EuiFlexGroup?

…column directions - Due to switch to `gap`, no need for margin overrides anymore - the gap will be calculated automatically - Due to flatter Emotion classes, no need for !important anymore (which is probably extremely annoying to override) - see elastic#5529 for column directions decision

- separate out `grow` logic via JS to reduce CSS overrides - explicitly print grow-1 class if grow is a generic `true` + improve props table + remove `@ts-ignore` by switching type to ElementType + add missing `component` prop unit test

src/components/flex/flex_grid.styles.ts

cee-chen

Just wanted to highlight that I have intentionally removed all nesting as well as !important declarations in our EuiFlexGroup CSS. IMO, this will be a significantly less annoying developer experience for our consumers, but may also have unintentional side effects where responsive CSS is now overridden by something else.

Please leave a comment if you feel the !importants or high specificity needs to be preserved for any reason.

cee-chen · 2022-09-27T21:59:27Z

src/components/flex/_flex_grid.scss

-    // sass-lint:disable-block no-important
-    margin-left: 0 !important;
-    margin-right: 0 !important;
-    column-count: 1 !important;


See above parent comment

cee-chen · 2022-09-27T21:59:48Z

src/components/flex/_flex_group.scss

-  .euiFlexItem {
-    flex-grow: 1;
-    flex-basis: 0%;
-  }


See above parent comment

cee-chen · 2022-09-27T22:00:03Z

src/components/flex/_flex_item.scss

-// On mobile we force them to stack and act the same.
-@include euiBreakpoint('xs', 's') {
-  .euiFlexGroup--responsive > .euiFlexItem,
-  .euiFlexGrid--responsive > .euiFlexItem {
-    // sass-lint:disable-block no-important
-    width: 100% !important;
-    flex-basis: 100% !important;
-    margin-left: 0 !important;
-    margin-right: 0 !important;
-    margin-bottom: $euiSize !important;
-  }
-}


See above parent comment

kibanamachine · 2022-09-27T22:10:47Z

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

kibanamachine · 2022-09-28T16:13:50Z

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

elizabetdev · 2022-09-28T18:17:13Z

@constancecchen I tested the changes and it looks good. 🎉

I found some minor issues.

Docs

We can remove the following callout:

Related issues

[EuiFlexGrid] Allow number of columns based on viewport size #2609

What do you think of documenting the suggested pattern (#2609 (comment)) in our docs:

const isMobile = useIsWithinBreakpoints(['xs', 's']);

<EuiFlexGrid columns={isMobile ? 2 : 3} responsive={false}>
  <EuiFlexItem>
    <div>One</div>
  </EuiFlexItem>
  <EuiFlexItem>
    <div>Two</div>
  </EuiFlexItem>
  <EuiFlexItem>
    <div>Three</div>
  </EuiFlexItem>
</EuiFlexGrid>

[EuiFlexGrid] Add justifyContent and alignItems like EuiFlexGroup #4878

Is this issue still valid? Or should we close it?

[EuiFlexGroup] Responsive mode doesn't respect gutterSize #4140

I think this one can be closed. Right?

+ improve props docs

cee-chen · 2022-09-28T18:38:59Z

Awesome catches!! I totally forgot to do my normal docs pass, thanks Elizabet - will push fixes up here shortly.

Callout

✅ 92e4142

Mobile example

Looking into this, there's currently a responsive example for EuiFlexGroup so I think I'll need to do some shuffling around so the two examples aren't confused for each other.

✅ 3163f33

#4878

This is not closed by this issue. alignItems makes sense as a nice shortcut to add. justifyContent IMO really does not make sense given the grid's rigid column/percentage-width structure - I think alignContent makes more sense personally.

❌ I think it would be better to have that be a separate/follow up PR, so we can more closely analyze the options that display: grid gives us.

#4140

Yes, this PR fixes/closes the above issue - great catch. I've added that in the PR description and added a changelog

✅ 1163f84

+ move responsive flex group example up above grid examples

- combine src/components imports - remove unnecessary div wrappers

kibanamachine · 2022-09-28T19:12:25Z

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

kibanamachine · 2022-09-28T19:45:20Z

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

- I probably should not have tried to tackle removing doc example Sass in this PR, sorry Elizabet 🙈

kibanamachine · 2022-09-28T20:32:39Z

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

elizabetdev

Thanks, @constancecchen! LGTM! 🎉

I tested locally in Chrome, Safari, Edge, and Firefox. I also tested using the playground.

cee-chen · 2022-09-30T16:02:46Z

Thanks! I'll open up a follow-up PR to address #4878 after this!

## Summary `eui@67.1.8` ⏩ `eui@70.2.2` ⚠️ Note: This upgrade contains breaking changes to `EuiFlexGroup` and `EuiFlexGrid`, primarily around switching margins and negative margins to `gap`. Please do a quick QA pass of your app to scan for any issues. We're happy to help resolve minor fixes, or potentially follow up after PR merges. You can find us over in #eui! ## [`70.2.4`](https://github.com/elastic/eui/tree/v70.2.4) **Bug fixes** - Fixed visual bug in nested `EuiFlexGroup`s, where the parent `EuiFlexGroup` is responsive but a child `EuiFlexGroup` is not ([#6381](elastic/eui#6381)) ## [`70.2.3`](https://github.com/elastic/eui/tree/v70.2.3) **Bug fixes** - Fixed incorrect margins in `EuiSuperDatePicker` caused by `EuiFlex` CSS gap change ([#6380](elastic/eui#6380)) ## [`70.2.2`](https://github.com/elastic/eui/tree/v70.2.2) - `EuiButton` now accepts `minWidth={false}` ([#6373](elastic/eui#6373)) **Bug fixes** - `EuiButton` no longer outputs unnecessary inline styles for `minWidth={0}` or `minWidth={false}` ([#6373](elastic/eui#6373)) - `EuiFacetButton` no longer reports type issues when passing props accepted by `EuiButton` ([#6373](elastic/eui#6373)) - Fixed the shadow sizes of `.eui-yScrollWithShadows` and `.eui-xScrollWithShadows` ([#6374](elastic/eui#6374)) ## [`70.2.1`](https://github.com/elastic/eui/tree/v70.2.1) **Bug fixes** - Re-fixed `EuiPageSection` not correctly merging `contentProps.css` ([#6365](elastic/eui#6365)) - Fixed `EuiTab` not defaulting to size `m` ([#6366](elastic/eui#6366)) ## [`70.2.0`](https://github.com/elastic/eui/tree/v70.2.0) - Added a keyboard shortcuts popover to `EuiDataGrid`'s toolbar. This can be visually hidden via `toolbarVisibility.showKeyboardShortcuts`, but will always remain accessible to keyboard and screen reader users. ([#6036](elastic/eui#6036)) - `EuiScreenReaderOnly`'s `showOnFocus` prop now also shows on focus within its children ([#6036](elastic/eui#6036)) - Added `onFocus` prop callback to `EuiSuperDatePicker` ([#6320](elastic/eui#6320)) **Bug fixes** - Fixed `EuiSelectable` to ensure the full options list is re-displayed when the search bar is controlled and cleared using `searchProps.value` ([#6317](elastic/eui#6317)) - Fixed incorrect padding on `xl`-sized `EuiTabs` ([#6336](elastic/eui#6336)) - Fixed `EuiCard` not correctly merging `css` on its child `icon`s ([#6341](elastic/eui#6341)) - Fixed `EuiCheckableCard` not setting `css` on the correct DOM node ([#6341](elastic/eui#6341)) - Fixed a webkit rendering issue with `EuiModal`s containing `EuiBasicTable`s tall enough to scroll ([#6343](elastic/eui#6343)) - Fixed bug in `to_initials` that truncates custom initials ([#6346](elastic/eui#6346)) - Fix bug in `EuiCard` where layout breaks when `horizontal` and `selectable` are both passed ([#6348](elastic/eui#6348)) ## [`70.1.0`](https://github.com/elastic/eui/tree/v70.1.0) - Added the `hint` prop to the `<EuiSearchBar />`. This prop lets the consumer render a hint below the search bar that will be displayed on focus. ([#6319](elastic/eui#6319)) - Added the `hasDragDrop` prop to `EuiPopover`. Use this prop if your popover contains `EuiDragDropContext`. ([#6329](elastic/eui#6329)) **Bug fixes** - Fixed `EuiButton`'s cursor style when the button is disabled ([#6323](elastic/eui#6323)) - Fixed `EuiPageTemplate` not recognizing child `EuiPageSidebar`s/`EuiPageTemplate.Sidebar`s with `css` props ([#6324](elastic/eui#6324)) - Fixed `EuiBetaBadge` to always respect its `anchorProps` values, including when there is no tooltip content ([#6326](elastic/eui#6326)) - Temporarily patched `EuiModal` to not cause scroll-jumping issues on modal open ([#6327](elastic/eui#6327)) - Fixed buggy drag & drop behavior within `EuiDataGrid`'s columns & sorting toolbar popovers ([#6329](elastic/eui#6329)) - Fixed `EuiButton` not correctly passing `textProps` for children inside fragments or i18n components ([#6332](elastic/eui#6332)) - Fixed `EuiButton` not correctly respecting `minWidth={0}` ([#6332](elastic/eui#6332)) **CSS-in-JS conversions** - Converted `EuiTabs` to Emotion ([#6311](elastic/eui#6311)) ## [`70.0.0`](https://github.com/elastic/eui/tree/v70.0.0) - Added the `enabled` option to the `<EuiInMemoryTable />` `executeQueryOptions` prop. This option prevents the Query from being executed when controlled by the consumer. ([#6284](elastic/eui#6284)) **Bug fixes** - Fixed `EuiOverlayMask` to set a `[data-relative-to-header=above|below]` attribute to replace the `--aboveHeader` and `--belowHeader` classNames removed in its Emotion conversion ([#6289](elastic/eui#6289)) - Fixed `EuiHeader` CSS using removed `EuiOverlayMask` class modifiers ([#6293](elastic/eui#6293)) - Fixed `EuiToolTip` not respecting reduced motion preferences ([#6295](elastic/eui#6295)) - Fixed a bug with `EuiTour` where passing any `panelProps` would cause the beacon to disappear ([#6298](elastic/eui#6298)) **Breaking changes** - `@emotion/css` is now a required peer dependency, alongside `@emotion/react` ([#6288](elastic/eui#6288)) - `@emotion/cache` is no longer required peer dependency, although your project must still use it if setting custom cache/injection locations ([#6288](elastic/eui#6288)) **CSS-in-JS conversions** - Converted `EuiCode` and `EuiCodeBlock` to Emotion; Removed `euiCodeSyntaxTokens` Sass mixin and `$euiCodeBlockPaddingModifiers`; ([#6263](elastic/eui#6263)) - Converted `EuiResizableContainer` and `EuiResizablePanel` to Emotion ([#6287](elastic/eui#6287)) ## [`69.0.0`](https://github.com/elastic/eui/tree/v69.0.0) - Added support for `fullWidth` prop on EuiForm, which will be the default for all rows/controls within ([#6229](elastic/eui#6229)) - Added support for `onResizeStart` and `onResizeEnd` callbacks to `EuiResizableContainer` ([#6236](elastic/eui#6236)) - Added optional case sensitive option matching to `EuiComboBox` with the `isCaseSensitive` prop ([#6268](elastic/eui#6268)) - `EuiFlexItem` now supports `grow={0}` ([#6270](elastic/eui#6270)) - Added the `alignItems` prop to `EuiFlexGrid` ([#6281](elastic/eui#6281)) - Added `filter`, `filterExclude`, `filterIgnore`, `filterInclude`, `indexTemporary`, `infinity`, `sortAscending`, and `sortDescending` glyphs to `EuiIcon` ([#6282](elastic/eui#6282)) **Bug fixes** - Fixed `EuiTextProps` to show the `color` type option `inherit` as default ([#6267](elastic/eui#6267)) - `EuiFlexGroup` now correctly respects `gutterSize` when responsive ([#6270](elastic/eui#6270)) - Fixed the last breadcrumb in `EuiBreadcrumbs`'s `breadcrumbs` array not respecting `truncate` overrides ([#6280](elastic/eui#6280)) **Breaking changes** - `EuiFlexGrid` no longer supports `columns={0}`. Use `EuiFlexGroup` instead for normal flex display ([#6270](elastic/eui#6270)) - `EuiFlexGrid` now uses modern `display: grid` CSS ([#6270](elastic/eui#6270)) - `EuiFlexGroup`, `EuiFlexGrid`, and `EuiFlexItem` now use modern `gap` CSS instead of margins and negative margins ([#6270](elastic/eui#6270)) - `EuiFlexGroup` no longer applies responsive styles to `column` or `columnReverse` directions ([#6270](elastic/eui#6270)) **CSS-in-JS conversions** - Converted `EuiFlexGroup`, `EuiFlexGrid`, and `EuiFlexItem` to Emotion ([#6270](elastic/eui#6270)) ## [`68.0.0`](https://github.com/elastic/eui/tree/v68.0.0) - Added `beta` glyph to `EuiIcon` ([#6250](elastic/eui#6250)) - Added `launch` and `spaces` glyphs to `EuiIcon` ([#6260](elastic/eui#6260)) - Added the `fallbackDestination` prop to `EuiSkipLink`, which accepts a string of query selectors to fall back to if the `destinationId` does not have a valid target. Defaults to `main` ([#6261](elastic/eui#6261)) - `EuiSkipLink` is now always an `a` tag to ensure that it is always placed within screen reader link menus. ([#6261](elastic/eui#6261)) **Bug fixes** - Fixed `EuiSuperDatePicker` not correctly merging passed `className`s ([#6253](elastic/eui#6253)) - Fixed `EuiColorStops` not correctly merging in passed `data-test-subj`s, `style`s, or `...rest` ([#6255](elastic/eui#6255)) - Fixed `EuiResizablePanel` incorrectly passing `style` to the wrapper instead of the panel. Use `wrapperProps.style` to pass styles to the wrapper. ([#6255](elastic/eui#6255)) - Fixed custom `onClick`s passed to `EuiSkipLink` overriding `overrideLinkBehavior` ([#6261](elastic/eui#6261)) **Breaking changes** - Removed `inherit` and `ghost` color from `EuiListGroupItem` ([#6207](elastic/eui#6207)) - Changed default color to `text` instead of `inherit` ([#6207](elastic/eui#6207)) **CSS-in-JS conversions** - Converted `EuiListGroup` and `EuiListGroupItem` to Emotion; Removed `$euiListGroupGutterTypes`, `$euiListGroupItemColorTypes` and `$euiListGroupItemSizeTypes`; ([#6207](elastic/eui#6207)) - Converted `EuiBadgeGroup` to Emotion ([#6258](elastic/eui#6258)) - Converted `EuiBetaBadge` to Emotion ([#6258](elastic/eui#6258)) - Converted `EuiNotificationBadge` to Emotion ([#6258](elastic/eui#6258)) Co-authored-by: Elizabet Oliveira <elizabet.oliveira@elastic.co> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

cee-chen added 4 commits September 27, 2022 12:08

Convert EuiFlexGroup to Emotion

43b733a

+ convert gutter CSS to `gap` + remove className maps + clean up unnecessary `...rest` cast + convert nested EuiFlexItem to flattened styles within flex item styles - realistically, who's going to be using EuiFlexItem outside of EuiFlexGroup?

Convert EuiFlexItem to Emotion + support grow={0}

187b0f8

- separate out `grow` logic via JS to reduce CSS overrides - explicitly print grow-1 class if grow is a generic `true` + improve props table + remove `@ts-ignore` by switching type to ElementType + add missing `component` prop unit test

Update all downstream snapshots

86e46c9

cee-chen added the emotion label Sep 27, 2022

cee-chen requested a review from elizabetdev September 27, 2022 21:42

Changelog

f269f26

cee-chen force-pushed the emotion/flex branch from 33e2587 to f269f26 Compare September 27, 2022 21:43

cee-chen marked this pull request as ready for review September 27, 2022 21:43

cee-chen mentioned this pull request Sep 27, 2022

[EuiFlexGroup] Switch gutter CSS from negative margins to gap #5575

Closed

6 tasks

cee-chen commented Sep 27, 2022

View reviewed changes

src/components/flex/flex_grid.styles.ts Show resolved Hide resolved

cee-chen commented Sep 27, 2022

View reviewed changes

src/components/flex/flex_grid.styles.ts Outdated Show resolved Hide resolved

cee-chen commented Sep 27, 2022

View reviewed changes

cee-chen mentioned this pull request Sep 28, 2022

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

Closed

49 tasks

Merge branch 'main' into emotion/flex

3e92cd6

Merge branch 'main' into emotion/flex

b01653e

cee-chen added 3 commits September 28, 2022 11:29

[EuiFlexGrid] Remove columns={0} as an option

bc4ce2e

+ improve props docs

[Docs] convert remaining .js examples to .tsx

b2eb6fe

[PR feedback][docs] Remove negative margin callout

92e4142

cee-chen force-pushed the emotion/flex branch from 80c6e60 to 92e4142 Compare September 28, 2022 18:31

cee-chen added 3 commits September 28, 2022 11:41

changelog for elastic#4140 bugfix

1163f84

[PR feedback] Add responsive flex grid example

3163f33

+ move responsive flex group example up above grid examples

[Docs] Misc code cleanup

be73335

- combine src/components imports - remove unnecessary div wrappers

cee-chen added 2 commits September 28, 2022 12:14

[Docs] Remove _flex.scss in favor of an Emotion wrapper

72ac3b2

[Docs] move responsive flex grid example to better match flow of page

c18538e

Fix playgrounds to accept custom Emotion CSS / fix flex playgrounds

7b600b6

- I probably should not have tried to tackle removing doc example Sass in this PR, sorry Elizabet 🙈

elizabetdev approved these changes Sep 30, 2022

View reviewed changes

cee-chen merged commit cd59d7b into elastic:main Sep 30, 2022

cee-chen deleted the emotion/flex branch September 30, 2022 16:04

cee-chen mentioned this pull request Sep 30, 2022

[EuiFlexGrid] Add alignItems prop #6281

Merged

5 tasks

cee-chen mentioned this pull request Nov 8, 2022

Upgrade EUI to v70.2.4 elastic/kibana#144845

Merged

cee-chen mentioned this pull request Mar 6, 2023

[EuiFlexGroup] Column layout has "weird" responsive behavior #3680

Closed

cee-chen mentioned this pull request Jul 5, 2023

[EuiAccordion] Children class cleanup #6906

Merged

1 task

cee-chen mentioned this pull request Oct 4, 2023

[EuiFlexGroup][EuiFlexGrid] Fix m gutter size spacing #7251

Merged

3 tasks

cee-chen mentioned this pull request Mar 27, 2024

[EuiFlexGroup][EuiFlexItem] Add polimorphism support for ReactHTML elements #7612

Closed

Conversation

cee-chen commented Sep 27, 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!

Uh oh!

Uh oh!

cee-chen left a comment

Choose a reason for hiding this comment

Uh oh!

cee-chen Sep 27, 2022

Choose a reason for hiding this comment

Uh oh!

cee-chen Sep 27, 2022

Choose a reason for hiding this comment

Uh oh!

cee-chen Sep 27, 2022

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Sep 27, 2022

Uh oh!

kibanamachine commented Sep 28, 2022

Uh oh!

elizabetdev commented Sep 28, 2022

Docs

Related issues

Uh oh!

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

Uh oh!

Uh oh!

kibanamachine commented Sep 28, 2022

Uh oh!

kibanamachine commented Sep 28, 2022

Uh oh!

kibanamachine commented Sep 28, 2022

Uh oh!

elizabetdev left a comment

Choose a reason for hiding this comment

Uh oh!

cee-chen commented Sep 30, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cee-chen commented Sep 27, 2022 •

edited

Loading

cee-chen commented Sep 28, 2022 •

edited

Loading