Upgrade EUI to v88.2.0 by cee-chen · Pull Request #165790 · elastic/kibana

cee-chen · 2023-09-05T23:02:08Z

Major changes in this PR:

Removal of `.euiAccordionForm` classNames

EUI is moving away from providing global classNames styles for components - where possible, we want to provide props as opposed to styles. As part of our ongoing Emotion conversion, we have removed the following EuiAccordion-specific classes:

.euiAccordionForm (replaced with borders="horizontal")
.euiAccordionForm__button (replaced with buttonProps={{ paddingSize: 'm' }})
.euiAccordionForm__title styles - this was only removing text underlines on hover. If still desired, re-add this behavior with custom CSS.
.euiAccordionForm__extraAction - there was 1 usage of this in Kibana in Watcher, which was converted to one-off custom inline Emotion CSS instead.

This change accounts for the first 3-4 commits in the PR. ⚠️ If your team was one of the 4-5 teams affected by this change, we would greatly appreciate your help QAing the UI and ensuring it looks as desired/the same as before.

Fixed `EuiHeader` affordance

The Sass euiHeaderAffordForFixed mixin has been deprecated and replaced by a global --euiFixedHeadersOffset CSS variable. This variable updates independently and dynamically based on the number of fixed headers on the page, and is usable by both Sass and Emotion. All underlying EUI components that need to account for fixed headers (such as flyouts and page sidebars/templates) have been updated to consume this new variable.

This change cleans up a great deal of Sass code, and is incidentally extremely timely with serverless efforts (as serverless has only one fixed header and the classic Kibana chrome has two).

This change constitutes a majority of the commits in this PR, which involve removing various fixed Sass header variables and replacing them with the new CSS variable. I strongly recommend reviewing changes by commit if possible to see any associated changes that make sense together with any of your touched file(s). ⚠️ If your team was affected by this change (primarily due to custom header layouts), your help would be greatly appreciated QAing your app to ensure no UI regressions in that regard have occurred.

v88.1.0⏩ v88.2.0

`88.2.0`

Added a new EuiTextTruncate component, which provides custom truncation options beyond native CSS (#7116)
Fixed-positioned EuiHeaders now set a global CSS --euiFixedHeadersOffset variable, which updates dynamically based on the number of fixed headers on the page. (#7144)
EuiFlyouts now dynamically set their position, height, and mask based on the number of fixed headers on the page. (#7144)
Sticky-positioned EuiPageSidebars now dynamically set their position and height based on the number of fixed headers on the page. This can still be overridden via the sticky.offset prop if needed. (#7144)
EuiPageTemplate now dynamically offsets content from any fixed headers on the page. This can still be overridden via the offset prop if needed. (#7144)
Updated EuiAccordion with a new borders prop (#7154)
Updated EuiAccordion with a new buttonProps.paddingSize prop (#7154)

Deprecations

Deprecated the Sass euiHeaderAffordForFixed mixin. Use the new global CSS var(--euiFixedHeadersOffset) variable instead. (#7144)

CSS-in-JS conversions

Except for generic CSS utilities, EUI is moving away from providing global classNames that are component-specific. As part of this effort, we have removed the following EuiAccordion-specific classes: (#7154)
- Removed .euiAccordionForm styles. Use the borders="horizontal" prop instead
- Removed .euiAccordionForm__button styles. Use the buttonProps={{ paddingSize: 'm' }} prop instead
- Removed .euiAccordionForm__extraAction styles. Convert this to your own custom CSS if necessary.
- Removed .euiAccordionForm__title styles. Convert this to your own custom CSS if necessary.

…extraAction` to example custom CSS

- likely not important to preserve the text-decoration CSS here

- include global banner if present, account for chrome visible or not + flatten conditional modifiers, to make overriding by other usages easier + remove offsets logic in _banner.scss, easier to handle it all in one place note: all flyouts/collapsible navs should inherit automatically from the `--euiFixedHeadersOffset` var

+ delete unused `kibanaFullBodyMinHeight` mixin

- by obtaining the height value from the DOM/CSS var

- Remove deprecated EUI mixin (now handled by CSS below) - Remove selector-specific overrides; update their usages to use the global CSS variable instead

- remove manual offset calculation and fall back to EuiPageSidebar's default inline styles instead, which automatically uses `--euiFixedHeadersOffset` + bonus: fix broken `:hover` effect on solution side navs by restoring missing className + continue dismantling `kbnAffordForHeader` mixin

+ remove `kbnStickyMenu` entirely - it looks like old code and it's not clear to me exactly how that behavior is being used, or if it should be using EuiPageSidebar intead - will ping Fleet team to ask

- # of fixed EuiHeaders should now be automatically detected and handled by EUI without needing any custom body logic

… var

+ some snapshot updates around the new header CSS var

- was relying on the old selectors to set the correct height - use the new EUI var instead and remove direct className reference

- className no longer has any direct styles applied, and as far as I can tell doesn't need any associated header-related flyout logic

sphilipse

ent-search changes LGTM

maximpn

Rules Management changes LGTM

thomheymann

security changes LGTM

nreese · 2023-09-08T16:35:22Z

x-pack/test/functional/apps/maps/group1/sample_data.js

          'web_logs_map',
          updateBaselines
        );
-        expect(percentDifference).to.be.lessThan(0.02);


Can you add some context for increasing this threshold. Maybe some screen shots from each or a diff screen shot. A 50% increase is pretty big.

Maybe the baselines need to be updated instead?

To be honest, I've struggled with Kibana's visual snapshot tests in the past. When I've previously tried to update the baselines locally, the new images appeared to be using a totally different set of DPI/dimensions than CI and it ended up failing completely on CI.

I'd appreciate either being able to pair with someone on your team for that, or alternatively having your team update baselines in a follow-up PR after this one. Please let me know what you prefer!

or alternatively having your team update baselines in a follow-up PR after this one. Please let me know what you prefer!

We can just make a PR to update the snapshots afterwards. That will be easier for you

Thanks a million @nreese!

nreese

kibana-gis changes LGTM
code review only

ThomThomson

Presentation team changes LGTM! Ran this locally and checked that Canvas worked with the new header height

vadimkibana

Shared UX code changes LGTM!

stratoula

Visualizations team changes LGTM!

ashokaditya

Thanks for the upgrade @cee-chen This is working great. I tested it out and I found a few places within security_solution and fleet where the var() function's fallback values need to be adjusted to account for when euiFixedHeadersOffset is not available.

I've not tested the other plugins but they may need this adjustment as well.

packages/core/chrome/core-chrome-browser-internal/src/ui/project/app_menu.tsx

...olicy/create_package_policy_page/components/steps/components/package_policy_input_stream.tsx

x-pack/plugins/security_solution/public/management/components/page_overlay/page_overlay.tsx

x-pack/plugins/security_solution/public/timelines/components/flyout/pane/index.tsx

ashokaditya · 2023-09-11T09:25:23Z

x-pack/plugins/fleet/public/layouts/without_header.tsx

-    100vh - ${(props) => parseFloat(props.theme.eui.euiHeaderHeightCompensation) * 2}px
-  );
+  // Set the min height to the viewport size minus the height of any global Kibana headers
+  min-height: calc(100vh - var(--euiFixedHeadersOffset));


This breaks the header spacing if euiFixedHeadersOffset is not available. Also I've not been able to figure this out yet. It doesn't work with a fallback of 96px. You might want to restore the min-height value that uses eui props for now. @juliaElastic ☝🏼

Thanks for checking, I'll restore it. At some point the old styled-components props will be deprecated/going away, but right now they're frozen during the Emotion conversion and aren't at risk for removal for another 3+ months.

Actually, @ashokaditya do you mind explaining how exactly you're getting the state of "euiFixedHeadersOffset is not available"?

The only scenario that should happen in production is when no fixed EuiHeaders are on the page, in which case falling back to 0 should be correct for all var() usages. If you're testing by unchecking the --euiFixedHeadersOffset variable definition in browser devtools, that would not be a realistic scenario end-users face.

Just to close the loop on this comment and the above comments - Ash was unchecking the definition in devtools, which isn't something that will happen in production. I've marked the above code suggestions as resolved and fixed all var() usages like this one to have 0 fallbacks in b530f22 🎉

kibana-ci · 2023-09-12T15:01:17Z

💔 Build Failed

Failed CI Steps

Test Failures

[job] [logs] Defend Workflows Endpoint Cypress Tests #2 / Automated Response Actions From alerts should have generated endpoint and rule should have generated endpoint and rule
[job] [logs] Defend Workflows Endpoint Cypress Tests #2 / Automated Response Actions From alerts should have generated endpoint and rule should have generated endpoint and rule
[job] [logs] Investigations - Security Solution Cypress Tests #4 / Discover Datagrid Cell Actions Filter for Filter for
[job] [logs] Investigations - Security Solution Cypress Tests #4 / Discover Datagrid Cell Actions Filter for Filter for
[job] [logs] Investigations - Security Solution Cypress Tests #4 / Discover Datagrid Cell Actions Filter out Filter out
[job] [logs] Investigations - Security Solution Cypress Tests #4 / Discover Datagrid Cell Actions Filter out Filter out

Metrics [docs]

Async chunks

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

id	before	after	diff
`apm`	3.7MB	3.7MB	-1.0B
`canvas`	1.0MB	1.0MB	-59.0B
`dashboard`	369.9KB	369.1KB	-836.0B
`discover`	556.6KB	555.2KB	-1.3KB
`enterpriseSearch`	2.6MB	2.6MB	-263.0B
`eventAnnotation`	177.0KB	176.7KB	-254.0B
`exploratoryView`	200.9KB	200.9KB	-1.0B
`filesManagement`	89.7KB	89.5KB	-254.0B
`fileUpload`	958.1KB	958.1KB	-1.0B
`fleet`	1.2MB	1.2MB	-122.0B
`graph`	409.3KB	409.0KB	-254.0B
`home`	164.1KB	163.9KB	-206.0B
`infra`	2.0MB	2.0MB	+697.0B
`kibanaOverview`	45.1KB	44.9KB	-254.0B
`lens`	1.4MB	1.4MB	+16.0B
`management`	42.6KB	42.4KB	-254.0B
`maps`	2.8MB	2.8MB	-614.0B
`ml`	3.5MB	3.5MB	-254.0B
`monitoring`	462.4KB	462.4KB	-1.0B
`observabilityShared`	36.1KB	35.9KB	-254.0B
`security`	571.6KB	571.4KB	-158.0B
`securitySolution`	12.6MB	12.6MB	-1.8KB
`securitySolutionEss`	42.6KB	42.4KB	-254.0B
`securitySolutionServerless`	270.2KB	270.0KB	-254.0B
`spaces`	174.0KB	173.8KB	-206.0B
`synthetics`	913.7KB	913.7KB	-22.0B
`uptime`	501.9KB	501.9KB	-1.0B
`visTypeVega`	1.8MB	1.8MB	+952.0B
`visualizations`	264.7KB	264.5KB	-254.0B
`watcher`	163.1KB	163.3KB	+174.0B
total			-6.2KB

Page load bundle

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

id	before	after	diff
`canvas`	13.4KB	13.3KB	-39.0B
`core`	381.4KB	365.3KB	-16.1KB
`esUiShared`	156.5KB	156.2KB	-254.0B
`interactiveSetup`	61.1KB	61.2KB	+48.0B
`kbnUiSharedDeps-css`	279.4KB	279.0KB	-427.0B
`kbnUiSharedDeps-npmDll`	6.2MB	6.2MB	+11.0KB
`logsShared`	219.4KB	219.4KB	-1.0B
`observability`	99.8KB	99.8KB	-1.0B
`painlessLab`	10.3KB	9.9KB	-444.0B
`searchprofiler`	19.8KB	19.5KB	-340.0B
total			-6.5KB

History

💔 Build #158200 failed b7684cc
💔 Build #158098 failed 2e64389
💛 Build #157874 was flaky a8f074e
💔 Build #157731 failed 42a4c01

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

## Summary [Latest upgrade of EUI](elastic#165790) removed the `isSticky` property that we were using in Integrations sidebar. This PR is adding it back with some css properties. https://github.com/elastic/kibana/assets/16084106/ffe0e1ed-cf70-45b7-8d8d-cb0c5f53e843

cee-chen force-pushed the eui-88.2.x branch 4 times, most recently from 755f4ab to c92d139 Compare September 6, 2023 16:32

cee-chen added 19 commits September 6, 2023 15:34

Upgrade EUI to v88.2.0

b9a1247

Convert deprecated euiAccordionForm styles to new prop usage

3bcc203

[Watcher] Convert .euiAccordionForm__title and `.euiAccordionForm__…

9295db7

…extraAction` to example custom CSS

[Security] Remove deprecated .euiAccordionForm__title

fe30ee8

- likely not important to preserve the text-decoration CSS here

[EuiAccordion] snapshot updates

132f105

Replace global header Sass variables with CSS variables

f42bbed

Update Sass mixins to use CSS variables

da3b2e5

+ delete unused `kibanaFullBodyMinHeight` mixin

Remove repeated constant declaration

9447496

- by obtaining the height value from the DOM/CSS var

Start dismantling kbnAffordForHeader mixin

d929acb

- Remove deprecated EUI mixin (now handled by CSS below) - Remove selector-specific overrides; update their usages to use the global CSS variable instead

Completely remove kbnAffordForHeader mixin

82af6db

+ remove `kbnStickyMenu` entirely - it looks like old code and it's not clear to me exactly how that behavior is being used, or if it should be using EuiPageSidebar intead - will ping Fleet team to ask

Remove need for kbnBody--projectLayout body class

c5d1e26

- # of fixed EuiHeaders should now be automatically detected and handled by EUI without needing any custom body logic

[Dashboard] Clean up sticky nav to use --euiFixedHeadersOffset CSS var

b5a8d27

Update other misc usages/affordances for fixed headers to use new CSS…

695e246

… var

[EuiHeader] Snapshot updates

ebcd1fe

+ some snapshot updates around the new header CSS var

Fix minute test percentage diff

6617059

[Security] Fix timeline "flyout"

daa279e

- was relying on the old selectors to set the correct height - use the new EUI var instead and remove direct className reference

[Lens] Remove unused euiFlyout className

f7c3aac

- className no longer has any direct styles applied, and as far as I can tell doesn't need any associated header-related flyout logic

cee-chen force-pushed the eui-88.2.x branch from d821e00 to f7c3aac Compare September 6, 2023 22:34

cee-chen marked this pull request as ready for review September 7, 2023 01:11

cee-chen requested review from a team as code owners September 7, 2023 01:11

jbudz approved these changes Sep 7, 2023

View reviewed changes

sphilipse approved these changes Sep 7, 2023

View reviewed changes

sabarasaba approved these changes Sep 7, 2023

View reviewed changes

Merge branch 'main' into eui-88.2.x

3e4611b

maximpn approved these changes Sep 8, 2023

View reviewed changes

thomheymann approved these changes Sep 8, 2023

View reviewed changes

nreese reviewed Sep 8, 2023

View reviewed changes

Merge branch 'main' into eui-88.2.x

42a4c01

nreese approved these changes Sep 8, 2023

View reviewed changes

ThomThomson approved these changes Sep 8, 2023

View reviewed changes

vadimkibana approved these changes Sep 10, 2023

View reviewed changes

Merge branch 'main' into eui-88.2.x

a8f074e

stratoula approved these changes Sep 11, 2023

View reviewed changes

juliaElastic approved these changes Sep 11, 2023

View reviewed changes

shahzad31 approved these changes Sep 11, 2023

View reviewed changes

ashokaditya reviewed Sep 11, 2023

View reviewed changes

cee-chen and others added 3 commits September 11, 2023 10:24

Fix --euiFixedHeadersOffset CSS variables with missing fallbacks

b530f22

Merge remote-tracking branch 'upstream/main' into eui-88.2.x

2e64389

Merge branch 'main' into eui-88.2.x

b7684cc

ashokaditya approved these changes Sep 12, 2023

View reviewed changes

vitaliidm approved these changes Sep 12, 2023

View reviewed changes

semd approved these changes Sep 12, 2023

View reviewed changes

Merge branch 'main' into eui-88.2.x

9704de3

Ikuni17 merged commit 51569b4 into elastic:main Sep 12, 2023

kibanamachine added the backport:skip This PR does not require backporting label Sep 12, 2023

cee-chen deleted the eui-88.2.x branch September 13, 2023 19:12

criamico mentioned this pull request Sep 19, 2023

[Fleet] Make integrations sidebar sticky again #166702

Merged

criamico mentioned this pull request Oct 17, 2023

[Fleet]: LHS category list scroller not available under Integrations tab. #168915

Closed

Conversation

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

Uh oh!

Removal of .euiAccordionForm classNames

Fixed EuiHeader affordance

88.2.0

Uh oh!

sphilipse left a comment

Choose a reason for hiding this comment

Uh oh!

maximpn 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!

thomheymann left a comment

Choose a reason for hiding this comment

Uh oh!

nreese Sep 8, 2023 • 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!

cee-chen Sep 8, 2023 • 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!

nreese Sep 8, 2023

Choose a reason for hiding this comment

Uh oh!

cee-chen Sep 8, 2023

Choose a reason for hiding this comment

Uh oh!

nreese left a comment

Choose a reason for hiding this comment

Uh oh!

ThomThomson left a comment

Choose a reason for hiding this comment

Uh oh!

vadimkibana left a comment

Choose a reason for hiding this comment

Uh oh!

stratoula left a comment

Choose a reason for hiding this comment

Uh oh!

ashokaditya left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ashokaditya Sep 11, 2023

Choose a reason for hiding this comment

Uh oh!

cee-chen Sep 11, 2023

Choose a reason for hiding this comment

Uh oh!

cee-chen Sep 11, 2023 • 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!

cee-chen Sep 11, 2023 • 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!

kibana-ci commented Sep 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💔 Build Failed

Failed CI Steps

Test Failures

Metrics [docs]

Async chunks

Page load bundle

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

cee-chen commented Sep 5, 2023 •

edited

Loading

Removal of `.euiAccordionForm` classNames

Fixed `EuiHeader` affordance

`88.2.0`

maximpn left a comment •

edited

Loading

nreese Sep 8, 2023 •

edited

Loading

cee-chen Sep 8, 2023 •

edited

Loading

cee-chen Sep 11, 2023 •

edited

Loading

cee-chen Sep 11, 2023 •

edited

Loading

kibana-ci commented Sep 12, 2023 •

edited

Loading