[EuiFilterGroup] Fix responsive styles #6983

cee-chen · 2023-07-24T15:06:26Z

Summary

🤦 I messed up the syntax for this during #6982, and now EuiFilterGroup is full width at all breakpoints when it should only be full-width at the xs->s breakpoint.

Above `m` breakpoint (width):

Broken on prod:

After fix:

Below `m` breakpoint (flex wrapping):

Broken on prod:

After fix:

QA

General checklist

Checked in mobile
A changelog entry exists and is marked appropriately

- I messed up the breakpoint interpretation/logic during its Emotion conversion

cee-chen · 2023-07-24T15:19:25Z

src/components/filter_group/filter_group.styles.ts

@@ -51,10 +47,10 @@ export const euiFilterGroupStyles = (euiThemeContext: UseEuiTheme) => {
        }
      }

-      ${euiMaxBreakpoint(euiThemeContext, 's')} {
+      ${euiBreakpoint(euiThemeContext, ['xs', 's'])} {


I always forget how dang unintuitively our breakpoint mixin is written, at least for me - I personally see a "breakpoint" as a single 1D point, the previous EUI team apparently saw a breakpoint as a 2D "from X to Y size screen". So this breakpoint actually translates to a max-width of the m breakpoint, not the s breakpoint.

This leads to me occasionally fucking up converting Sass media queries - hopefully I'll get back into the muscle memory of it after this PR 🤦

cee-chen · 2023-07-24T15:19:46Z

@tkajtoch We'll need to backport this into your current Kibana upgrade (it may or may not be causing failures in your PR; either way I would feel bad if Kibana devs saw this or had to deal with this).

kibanamachine · 2023-07-24T15:29:56Z

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

kibanamachine · 2023-07-24T17:31:02Z

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

1Copenut

👍 LGTM! I tested briefly in Chrome, Edge, and Firefox. Toggled through the mobile emulator and full-width to ensure breakpoint changes.

## Summary `eui@84.0.0` ⏩ `eui@85.0.1` ## [`85.0.1`](https://github.com/elastic/eui/tree/v85.0.1) **Bug fixes** - Fixed `EuiFilterGroup`'s responsive styles ([#6983](elastic/eui#6983)) ## [`85.0.0`](https://github.com/elastic/eui/tree/v85.0.0) - Updated `EuiThemeProvider` to set an Emotion theme context that returns the values of `useEuiTheme()` ([#6913](elastic/eui#6913)) - Added `size` prop to `EuiStepsHorizontal`, defaulting to the previous size of `m` ([#6928](elastic/eui#6928)) - Added new `s` sizing to `EuiStepsHorizontal` ([#6928](elastic/eui#6928)) - Added `at` and `key` icon glyphs. ([#6934](elastic/eui#6934)) - Added a new `cloneElementWithCss` Emotion utility ([#6939](elastic/eui#6939)) - Updated `EuiPopover` to allow consumer control of all `focusTrapProps` ([#6955](elastic/eui#6955)) **Bug fixes** - Fixed `EuiDataGrid` height calculation bug when browser zoom levels are not 100% ([#6895](elastic/eui#6895)) - Fixed `EuiTab` not correctly passing selection color state to `prepend` and `append` children ([#6938](elastic/eui#6938)) - Fixed `EuiInputPopover` to allow consumer control of its focus trap via `focusTrapProps` ([#6955](elastic/eui#6955)) **Breaking changes** - `EuiProvider` will no longer render multiple or duplicate nested instances of itself. If a nested `EuiProvider` is detected, that instance will return early without further processing, and will warn if configured to do so via `setEuiDevProviderWarning`. For nested theming, use `EuiThemeProvider` instead. ([#6949](elastic/eui#6949)) - Removed `onTrapDeactivation` prop from `EuiPopover`. Use `focusTrapProps.onDeactivation` instead ([#6955](elastic/eui#6955)) **CSS-in-JS conversions** - Converted `EuiFilterGroup` and `EuiFilterButton` to Emotion; Removed styles attached to `.euiFilterGroup__popoverPanel` ([#6957](elastic/eui#6957)) --------- Co-authored-by: Cee Chen <constance.chen@elastic.co> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

## Summary `eui@84.0.0` ⏩ `eui@85.0.1` ## [`85.0.1`](https://github.com/elastic/eui/tree/v85.0.1) **Bug fixes** - Fixed `EuiFilterGroup`'s responsive styles ([elastic#6983](elastic/eui#6983)) ## [`85.0.0`](https://github.com/elastic/eui/tree/v85.0.0) - Updated `EuiThemeProvider` to set an Emotion theme context that returns the values of `useEuiTheme()` ([elastic#6913](elastic/eui#6913)) - Added `size` prop to `EuiStepsHorizontal`, defaulting to the previous size of `m` ([elastic#6928](elastic/eui#6928)) - Added new `s` sizing to `EuiStepsHorizontal` ([elastic#6928](elastic/eui#6928)) - Added `at` and `key` icon glyphs. ([elastic#6934](elastic/eui#6934)) - Added a new `cloneElementWithCss` Emotion utility ([elastic#6939](elastic/eui#6939)) - Updated `EuiPopover` to allow consumer control of all `focusTrapProps` ([elastic#6955](elastic/eui#6955)) **Bug fixes** - Fixed `EuiDataGrid` height calculation bug when browser zoom levels are not 100% ([elastic#6895](elastic/eui#6895)) - Fixed `EuiTab` not correctly passing selection color state to `prepend` and `append` children ([elastic#6938](elastic/eui#6938)) - Fixed `EuiInputPopover` to allow consumer control of its focus trap via `focusTrapProps` ([elastic#6955](elastic/eui#6955)) **Breaking changes** - `EuiProvider` will no longer render multiple or duplicate nested instances of itself. If a nested `EuiProvider` is detected, that instance will return early without further processing, and will warn if configured to do so via `setEuiDevProviderWarning`. For nested theming, use `EuiThemeProvider` instead. ([elastic#6949](elastic/eui#6949)) - Removed `onTrapDeactivation` prop from `EuiPopover`. Use `focusTrapProps.onDeactivation` instead ([elastic#6955](elastic/eui#6955)) **CSS-in-JS conversions** - Converted `EuiFilterGroup` and `EuiFilterButton` to Emotion; Removed styles attached to `.euiFilterGroup__popoverPanel` ([elastic#6957](elastic/eui#6957)) --------- Co-authored-by: Cee Chen <constance.chen@elastic.co> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

`85.0.0` ➡️ `85.1.0` ⚠️ The biggest change in this PR by far is the **removal** of several `EuiFilterSelectItem` CSS classes and styles associated with those classes. EUI has previously exported component-specific CSS classes for usage such as `.euiFilterSelect__items`, `.euiFilterGroup__popoverPanel`, or `.euiAccordionForm`. ❌ **As of our move to CSS-in-JS, we are actively moving away from this architecture**. The goal is to either provide either a component or prop directly to you instead of a CSS class. As a reminder, our classNames are not guaranteed APIs and are subject to change without warning - should you need to tweak or customize EUI's styling, we strongly recommend passing in your own `className`. 💬 Moving forward, EUI will attempted to convert any usages of removed CSS classes and their direct usages in Kibana for you. In most cases, we'll hopefully be able to take the correct path of using a component or prop instead. In cases where we cannot or significant/complex changes are required, we may temporarily convert the CSS to a static CSS-in-JS usage instead and add a TODO asking the relevant team to address this in their own time (for example, the deprecation of `EuiFilterSelectItem` and its conversion to `EuiSelectable`). ## [`85.1.0`](https://github.com/elastic/eui/tree/v85.1.0) - Updated `EuiComboBox`'s `options` to accept `option.append` and `option.prepend` props ([#6953](elastic/eui#6953)) - Updated deprecated `.substr()` usages to `.substring()` ([#6954](elastic/eui#6954)) - Updated `EuiInlineEdit`'s read mode button to include a title tooltip, increasing readability of truncated text ([#6966](elastic/eui#6966)) **Bug fixes** - Fixed `EuiFilterGroup`'s responsive styles ([#6983](elastic/eui#6983)) **Deprecations** - Deprecated `EuiFilterSelectItem`; Use `EuiSelectable` instead ([#6982](elastic/eui#6982)) **CSS-in-JS conversions** - Converted `EuiFilterSelectItem` to Emotion ([#6982](elastic/eui#6982)) - Removed `.euiFilterSelect__items` CSS; Use `EuiSelectable` instead ([#6982](elastic/eui#6982)) - Removed `.euiFilterSelect__note` and `.euiFilterSelect__noteContent` CSS; Use `EuiSelectableMessage` instead ([#6982](elastic/eui#6982)) - Added `focus.transparency` and `focus.backgroundColor` theme tokens ([#6984](elastic/eui#6984)) --------- Co-authored-by: Cee Chen <constance.chen@elastic.co> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

cee-chen added 2 commits July 24, 2023 08:03

Fix EuiFilterGroup's responsive styling

6b842c6

- I messed up the breakpoint interpretation/logic during its Emotion conversion

changelog

0ee5a07

cee-chen marked this pull request as ready for review July 24, 2023 15:13

cee-chen requested a review from tkajtoch July 24, 2023 15:13

cee-chen commented Jul 24, 2023

View reviewed changes

Merge branch 'elastic:main' into emotion/filter-group-fix

1108870

1Copenut approved these changes Jul 24, 2023

View reviewed changes

cee-chen merged commit 2121e09 into elastic:main Jul 24, 2023
2 checks passed

cee-chen deleted the emotion/filter-group-fix branch July 24, 2023 18:07

cee-chen mentioned this pull request Jul 24, 2023

Upgrade @elastic/eui to 85.0.1 elastic/kibana#162209

Merged

breehall mentioned this pull request Jul 27, 2023

Upgrade EUI to v85.1.0 elastic/kibana#162660

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[EuiFilterGroup] Fix responsive styles #6983

[EuiFilterGroup] Fix responsive styles #6983

cee-chen commented Jul 24, 2023 •

edited

Loading

cee-chen Jul 24, 2023

cee-chen commented Jul 24, 2023

kibanamachine commented Jul 24, 2023

kibanamachine commented Jul 24, 2023

1Copenut left a comment

[EuiFilterGroup] Fix responsive styles #6983

[EuiFilterGroup] Fix responsive styles #6983

Conversation

cee-chen commented Jul 24, 2023 • edited Loading

Summary

Above m breakpoint (width):

Below m breakpoint (flex wrapping):

QA

General checklist

cee-chen Jul 24, 2023

Choose a reason for hiding this comment

cee-chen commented Jul 24, 2023

kibanamachine commented Jul 24, 2023

kibanamachine commented Jul 24, 2023

1Copenut left a comment

Choose a reason for hiding this comment

cee-chen commented Jul 24, 2023 •

edited

Loading

Above `m` breakpoint (width):

Below `m` breakpoint (flex wrapping):