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

Upgrade @elastic/eui to 85.0.1 #162209

Merged
merged 12 commits into from
Jul 27, 2023
Merged

Upgrade @elastic/eui to 85.0.1 #162209

merged 12 commits into from
Jul 27, 2023

Conversation

tkajtoch
Copy link
Member

@tkajtoch tkajtoch commented Jul 19, 2023

Summary

eui@84.0.0eui@85.0.1

85.0.1

Bug fixes

  • Fixed EuiFilterGroup's responsive styles (#6983)

85.0.0

  • Updated EuiThemeProvider to set an Emotion theme context that returns the values of useEuiTheme() (#6913)
  • Added size prop to EuiStepsHorizontal, defaulting to the previous size of m (#6928)
  • Added new s sizing to EuiStepsHorizontal (#6928)
  • Added at and key icon glyphs. (#6934)
  • Added a new cloneElementWithCss Emotion utility (#6939)
  • Updated EuiPopover to allow consumer control of all focusTrapProps (#6955)

Bug fixes

  • Fixed EuiDataGrid height calculation bug when browser zoom levels are not 100% (#6895)
  • Fixed EuiTab not correctly passing selection color state to prepend and append children (#6938)
  • Fixed EuiInputPopover to allow consumer control of its focus trap via focusTrapProps (#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)
  • Removed onTrapDeactivation prop from EuiPopover. Use focusTrapProps.onDeactivation instead (#6955)

CSS-in-JS conversions

  • Converted EuiFilterGroup and EuiFilterButton to Emotion; Removed styles attached to .euiFilterGroup__popoverPanel (#6957)

@tkajtoch tkajtoch added backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes EUI labels Jul 19, 2023
@tkajtoch tkajtoch self-assigned this Jul 19, 2023
@tkajtoch tkajtoch force-pushed the eui-v85.0.0 branch 3 times, most recently from 9fa8a55 to b2022cd Compare July 24, 2023 09:46
tkajtoch and others added 4 commits July 24, 2023 16:05
- `.presFilterByType__panel` already has its own width Sass, so move the width CSS there instead
…overPanel`

- After Emotion conversion, no longer has any styles attached to it

- use Emotion CSS instead
@tkajtoch tkajtoch marked this pull request as ready for review July 24, 2023 15:45
@tkajtoch tkajtoch requested review from a team as code owners July 24, 2023 15:45
@tkajtoch tkajtoch requested a review from jpdjere July 24, 2023 15:45
@elasticmachine
Copy link
Contributor

Pinging @elastic/eui-team (EUI)

Copy link
Contributor

@Heenawter Heenawter left a comment

Choose a reason for hiding this comment

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

Presentation changes LGTM 👍

@cee-chen cee-chen changed the title Upgrade @elastic/eui to 85.0.0 Upgrade @elastic/eui to 85.0.1 Jul 24, 2023
@banderror banderror requested review from spong and removed request for jpdjere July 25, 2023 09:45
@@ -149,7 +149,7 @@ export function FieldTypeFilter<T extends FieldListItem = DataViewField>({
return (
<EuiPopover
id="unifiedFieldTypeFilter"
panelClassName="euiFilterGroup__popoverPanel"
panelProps={{ css: { width: euiTheme.base * 18 } }}
Copy link
Contributor

Choose a reason for hiding this comment

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

In Discover, the filter button width and the filter item height look larger than it was in 8.9:

Before:
Screenshot 2023-07-25 at 12 08 48

Now:
Screenshot 2023-07-25 at 12 08 58

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @jughosta I'll look into that!

Copy link
Contributor

Choose a reason for hiding this comment

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

The filter button width should be fixed as with 14e7469 - I hadn't realized folks in Kibana were using EuiFilterButton outside of EuiFilterGroup, so that's my bad :)

Regarding the filter item height - this looks like it was affected by #161592 (Sass styling order). I pushed up a specificity override for now (f5a55e0) which we can remove once more of EUI is on Emotion 🤞

Thanks for your eagle eye on these things @jughosta, you're amazing!!

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the fixes, @cee-chen! You are amazing! 😉

cee-chen added 2 commits July 25, 2023 08:49
- `min-width` is now set on `EuiFilterButtons` even outside of `EuiFilterGroup`s, so CSS needs to update to take that into account
@@ -160,7 +160,7 @@ export function TableSortSelect({ tableSort, hasUpdatedAtMetadata, onChange }: P
closePopover={closePopover}
panelPaddingSize="none"
anchorPosition="downCenter"
panelClassName="euiFilterGroup__popoverPanel"
panelProps={{ css: { width: euiTheme.base * 18 } }}
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: would prefer 18 to be a constant. Not sure what that number is supposed to correspond to.

@ryankeairns, you like to call this out often, what do you think?

Copy link
Contributor

@cee-chen cee-chen Jul 25, 2023

Choose a reason for hiding this comment

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

Y'all are super welcome to set your own constant! The number is what .euiFilterGroup__popoverPanel was setting as a width in CSS/Sass (link), it's just not hidden behind an EUI CSS class anymore, as we are no longer providing component-specific CSS utilities to consumers.

In any case, width is fairly content-specific so we would prefer consumers figure out what width they want to display based on their content and not on a generic.

@cee-chen
Copy link
Contributor

@elasticmachine merge upstream

Copy link
Member

@spong spong left a comment

Choose a reason for hiding this comment

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

Security Detection Rule Mgmt changes (just two test updates) LGTM! Thanks @tkajtoch! 🙂

Copy link
Contributor

@jughosta jughosta left a comment

Choose a reason for hiding this comment

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

Data Discovery changes LGTM 👍

@tkajtoch
Copy link
Member Author

@elasticmachine merge upstream

Copy link
Contributor

@ryankeairns ryankeairns left a comment

Choose a reason for hiding this comment

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

@cee-chen
Copy link
Contributor

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Security Solution Tests #3 / Alert details expandable flyout right panel overview tab insights section should display threat intelligence section should display threat intelligence section

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
infra 1397 1401 +4
visTypeVega 305 309 +4
total +8

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
cases 421.6KB 421.6KB +2.0B
dashboard 358.2KB 358.2KB -22.0B
discover 547.9KB 547.9KB +15.0B
eventAnnotation 176.2KB 176.2KB -22.0B
filesManagement 89.2KB 89.2KB -22.0B
graph 407.4KB 407.4KB -22.0B
infra 2.0MB 2.0MB +2.4KB
lens 1.4MB 1.4MB +15.0B
maps 2.8MB 2.8MB -22.0B
presentationUtil 129.8KB 129.8KB -5.0B
visTypeVega 1.8MB 1.8MB +2.4KB
visualizations 262.5KB 262.4KB -22.0B
total +4.8KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
kbnUiSharedDeps-css 296.4KB 294.1KB -2.4KB
kbnUiSharedDeps-npmDll 6.0MB 6.0MB +6.3KB
total +4.0KB

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @tkajtoch

@cee-chen
Copy link
Contributor

@elastic/response-ops As a heads up, we'll be asking @elastic/kibana-operations for an admin merge today as this PR has been open for over a week. If you encounter an issues that you think may be related to the upgrade after the fact (knock on wood), please feel free to ping us.

@Ikuni17 Ikuni17 merged commit 1c42ee9 into elastic:main Jul 27, 2023
ThomThomson pushed a commit to ThomThomson/kibana that referenced this pull request Aug 1, 2023
## 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting EUI release_note:skip Skip the PR/issue when compiling release notes v8.10.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.