Upgrade EUI to v54.0.0 #129653

cee-chen · 2022-04-06T19:23:22Z

Summary

eui@53.0.1 ⏩ eui@54.0.0

`54.0.0`

EuiDataGrid now allows limiting the number of visible cell actions with a new columns.visibleCellActions prop (defaults to 2). All additional actions will be shown in the cell expansion popover. (#5675)
Added a new gridStyle.rowClasses API to EuiDataGrid, which allows adding custom classes/styling to specific row indices, primarily for the purpose of highlighting rows (#5732)
Added alert icon indicator and aria-invalid when the following form controls are isInvalid: EuiFieldNumber, EuiFieldPassword, EuiFieldText, EuiSelect, EuiSuperSelect, EuiFieldSearch, EuiColorPicker (#5738)
Added isInvalid prop to EuiFormControlLayout to render the alert icon (#5738)
Added isDropdown prop to EuiFormControlLayout to create and control an arrowDown icon (#5738)
Added color as to EuiFormControlLayout's icon object (#5738)

Bug fixes

Fixed EuiSuperSelect border-radius with append or prepend (#5738)
Fixed EuiSuperSelect not respecting readOnly (#5738)
Fixed EuiDataGrid cell focus sometimes not being restored for keyboard users when cell expansion popovers were closed (#5761)

Breaking changes

Removed the closePopover() callback passed to EuiDataGrid's cellActions render functions. Use closeCellPopover() passed by EuiDataGrid's ref prop instead. (#5734)

CSS-in-JS conversions

Converted EuiAvatar to CSS-in-JS styling (#5670)

cee-chen · 2022-04-06T19:26:02Z

x-pack/plugins/security_solution/public/common/components/events_viewer/index.tsx

@@ -212,6 +212,7 @@ const StatefulEventsViewerComponent: React.FC<Props> = ({
              dataProviders,
              dataViewId,
              defaultCellActions,
+              visibleCellActions: 3,


@MikePaquette (pinging you because you posted in elastic/eui#5675 (comment))

I'm not overly familiar with Security's usage of EuiDataGrid - can you confirm that this component (Events Viewer?) is the correct/only component that needs 3 visible cell actions instead of the default 2?

From a slack thread:

imo would be safer to just pass a hard coded 3 here https://github.com/elastic/kibana/blob/main/x-pack/plugins/timelines/public/components/t_grid/body/index.tsx#L819-#L839 instead of as a prop from the different places it’s used

@MikePaquette (pinging you because you posted in elastic/eui#5675 (comment))

I'm not overly familiar with Security's usage of EuiDataGrid - can you confirm that this component (Events Viewer?) is the correct/only component that needs 3 visible cell actions instead of the default 2?

@constancecchen yes, this is the only component that needs 3 visible cell actions instead of the default 2

- for closeCellPopover ref API

…ellActions` prop + update Security to show 3 visible cell actions

…` change

- which, due to mount() causes .first() to no longer work as expected - targeting .last() instead gets the actual div element which works

- EUI added 2 `.euiPopoverFooter`s for overflowing cell actions, and Security's CSS to hide the first 2 cell actions (replaced by their own custom cell actions) was unintentionally affecting other actions

sabarasaba

@elastic/platform-deployment-management changes lgtm

elasticmachine · 2022-04-11T08:54:05Z

Pinging @elastic/apm-ui (Team:apm)

kqualters-elastic · 2022-04-11T15:11:01Z

x-pack/plugins/timelines/public/components/t_grid/body/index.tsx

@@ -720,6 +728,7 @@ export const BodyComponent = React.memo<StatefulBodyProps>(
        columnHeaders,
        data,
        defaultCellActions,
+        visibleCellActions,


instead of passing this down as a prop from all the places it's used, wouldn't it be safer to just hard code the 3 in this component?

https://github.com/elastic/kibana/blob/main/x-pack/plugins/timelines/public/components/t_grid/body/index.tsx#L819-#L839

Yeah that would be fine! I wasn't sure what all plugins used this TGrid component and if future usages might want to customize visibleCellActions, so was trying to play it safe. If you're confident all future usages will want a constant max of 3 visible cell actions I'm good with hard-coding it.

fc93474 @kqualters-elastic @andrew-goldstein. Here's the relevant line where 3 is hard-coded:

kibana/x-pack/plugins/timelines/public/components/t_grid/body/index.tsx

Line 719 in fc93474

visibleCellActions: 3,

jportner · 2022-04-11T15:50:35Z

...ck/plugins/spaces/public/management/spaces_grid/__snapshots__/spaces_grid_page.test.tsx.snap

+        <Insertion
+          cache={
+            Object {


😬 these Spaces snapshot test changes aren't great --
The tests themselves are ancient and I found that I could make a few small changes to avoid these gigantic snapshots.
If you're OK with it, I can push these changes to your branch. Let me know!

We spoke offline and I pushed the changes here: 3ddd8bd

Heenawter

Code review - primarily just adding new classes to Storybook components, plus some other minor restructuring of Color Manager Storybook. Presentation changes LGTM 👍

andrew-goldstein

Thanks @constancecchen for upgrading the Security Solution to use EuiDataGrid's new visibleCellActions option, such that there's no change in the behavior of the app 🙏

✅ Desk tested locally
++ to @kqualters-elastic's suggestion that configurability of the 2 -> 3 override via props isn't required
While desk testing, I found what appears to be an unreleated issue with EuiDataGrid popovers owning focus in Safari: [EuiContextMenu] tab key doesn't cycle through context menu items in Safari eui#5778

LGTM

kertal

DataDiscover.team code owned code LGTM, tested in Cloud, works as expected, I'm exited about that

More than 10kb less in Discover's page load bundle 🥳 ... just wondering, how comes?

cee-chen · 2022-04-12T15:56:32Z

More than 10kb less in Discover's page load bundle 🥳 ... just wondering, how comes?

Honestly I'm not sure 🙈 I'm not seeing anything in the diffs that would significantly account for that, so my best guess is maybe Emotion dependency upgrades?

kevinlog

I see that @andrew-goldstein and @kqualters-elastic have also reviewed with checked out code in Security solution. Onboarding-Lifecycle just owns a snapshot update used for testing, which LGTM! (tests would fail otherwise).

mbondyra

LGTM 👌🏼

kertal · 2022-04-12T17:11:19Z

More than 10kb less in Discover's page load bundle 🥳 ... just wondering, how comes?

Honestly I'm not sure 🙈 I'm not seeing anything in the diffs that would significantly account for that, so my best guess is maybe Emotion dependency upgrades?

You're right, styles.js is no longer in the bundle twice!

tsullivan

app services changes LGTM

kibana-ci · 2022-04-12T17:34:26Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`dataViewEditor`	176	177	+1
`discover`	447	445	-2
`maps`	886	887	+1
`spaces`	287	288	+1
total			+1

Async chunks

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

id	before	after	diff
`apm`	2.8MB	2.8MB	-18.0B
`dataViewEditor`	175.8KB	176.3KB	+534.0B
`discover`	407.0KB	407.0KB	-6.0B
`lens`	1.0MB	1.0MB	+79.0B
`maps`	2.7MB	2.7MB	+533.0B
`securitySolution`	4.8MB	4.8MB	+4.0B
`spaces`	290.4KB	290.9KB	+533.0B
`visTypeTable`	15.7KB	15.8KB	+69.0B
`visTypeTimeseries`	470.6KB	470.7KB	+92.0B
total			+1.8KB

Page load bundle

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

id	before	after	diff
`core`	319.1KB	320.0KB	+943.0B
`discover`	50.7KB	40.5KB	-10.2KB
`kbnUiSharedDeps-css`	595.5KB	594.5KB	-1.0KB
`kbnUiSharedDeps-npmDll`	4.8MB	4.8MB	+4.0KB
`timelines`	286.8KB	286.9KB	+146.0B
total			-6.2KB

History

💚 Build #37347 succeeded 84457df
💚 Build #36932 succeeded 562bd03
💔 Build #36672 failed aa8966ca909069722b59ca315f524a43e6d54501
💔 Build #36637 failed c73c40e6de521bd59b272e3ff2dcde35ff59c6c1

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

jportner

🎉

cee-chen · 2022-04-12T17:59:52Z

Thanks everyone!!

…disable-server-side * 'main' of github.com:elastic/kibana: (35 commits) [Uptime] remove latency limit warnings when using monitor management (elastic#129597) [Security Solution] [ReponseOps] Executes Cases Cypress test when there is a change on cases plugin (elastic#129992) Paramaterized Discover tests (elastic#129684) [Security Solution][Investigations] - Minor bug fixes (elastic#130054) [DOCS} Adds technical preview to Lens annotations (elastic#130058) [Security solution] [Endpoint] Revisit blocklist wrong labels (elastic#128773) [Security Solutions] Adds API docs for value lists (elastic#129962) [CI] Move jest tests to spot instances, and fix spot retries in PRs (elastic#130045) chore(NA): upgrades rules_node_js to v5.4.0 (elastic#130051) [SecuritySolution] Remove the cell hovers actions for agent status (elastic#130042) Upgrade RxJS to 7 (elastic#129087) [SecuritySolution] Clean up CaseContext (elastic#130036) Revert "chore(NA): upgrades rules_node_js to v5.4.0 (elastic#130021)" Use RuleDataReader to query for threshold signal history (elastic#129763) Remove securityRulesCancelEnabled setting and set shorter default timeouts (elastic#129769) Upgrade EUI to v54.0.0 (elastic#129653) [Security Solution] More Ransomware exceptionable fields (elastic#130039) Add e2e for the apm integration policy form (elastic#129860) chore(NA): upgrades rules_node_js to v5.4.0 (elastic#130021) [ML] Fix Single Metric Viewer chart failing to load if no points during calendar event (elastic#130000) ... # Conflicts: # x-pack/plugins/screenshotting/server/screenshots/index.test.ts

…rint-media-attempt-2 * 'main' of github.com:elastic/kibana: (75 commits) [Lens] Hide disabled toolbar entries (elastic#129994) Fix explore tables don't display data when a global filter is applied (elastic#130024) [Console] Add option to disable keyboard shortcuts (elastic#128887) [Discover] Update refreshOnClick flaky test (elastic#130001) [Uptime] remove latency limit warnings when using monitor management (elastic#129597) [Security Solution] [ReponseOps] Executes Cases Cypress test when there is a change on cases plugin (elastic#129992) Paramaterized Discover tests (elastic#129684) [Security Solution][Investigations] - Minor bug fixes (elastic#130054) [DOCS} Adds technical preview to Lens annotations (elastic#130058) [Security solution] [Endpoint] Revisit blocklist wrong labels (elastic#128773) [Security Solutions] Adds API docs for value lists (elastic#129962) [CI] Move jest tests to spot instances, and fix spot retries in PRs (elastic#130045) chore(NA): upgrades rules_node_js to v5.4.0 (elastic#130051) [SecuritySolution] Remove the cell hovers actions for agent status (elastic#130042) Upgrade RxJS to 7 (elastic#129087) [SecuritySolution] Clean up CaseContext (elastic#130036) Revert "chore(NA): upgrades rules_node_js to v5.4.0 (elastic#130021)" Use RuleDataReader to query for threshold signal history (elastic#129763) Remove securityRulesCancelEnabled setting and set shorter default timeouts (elastic#129769) Upgrade EUI to v54.0.0 (elastic#129653) ... # Conflicts: # x-pack/plugins/screenshotting/server/formats/pdf/index.ts

cee-chen added release_note:skip Skip the PR/issue when compiling release notes EUI backport:skip This commit does not require backporting v8.3.0 labels Apr 6, 2022

cee-chen commented Apr 6, 2022

View reviewed changes

cee-chen and others added 13 commits April 7, 2022 08:48

Upgrade EUI to v54.0.0

6acef3e

[Discover] Remove deprecated closePopover call

d6cdec1

- for closeCellPopover ref API

[Lens] Remove deprecated closePopover call

4984200

- for closeCellPopover ref API

[Security/Timelines] Remove deprecated closePopover call

86b4a0f

- for closeCellPopover ref API

[Security/Timeline] Update Timeline datagrid to accept/pass `visibleC…

793d208

…ellActions` prop + update Security to show 3 visible cell actions

[APM] Account for removed EUI theme avatar sizes

64651f1

Update emotion dependencies to latest

615dc4b

Remove styles from being rendered in emotion serializer

effc21e

Update snapshots affected by emotion serializer `includeStyles: false…

597aba1

…` change

Update snapshot changes caused by EuiFormControlLayout changes

9da3f58

Update snapshot changes caused by EuiAvatar CSS-in-JS conversion

fdfa694

consolidate yarn.lock

97bff51

[Spaces] Fix failing test due to new EuiAvatar emotion wrapper

0331f09

- which, due to mount() causes .first() to no longer work as expected - targeting .last() instead gets the actual div element which works

cee-chen force-pushed the eui-54.0.0 branch from aa8966c to 0331f09 Compare April 7, 2022 16:07

[Security] Fix cell expansion popover actions

562bd03

- EUI added 2 `.euiPopoverFooter`s for overflowing cell actions, and Security's CSS to hide the first 2 cell actions (replaced by their own custom cell actions) was unintentionally affecting other actions

cee-chen marked this pull request as ready for review April 7, 2022 18:26

cee-chen requested review from a team as code owners April 7, 2022 18:26

cee-chen requested a review from paul-tavares April 7, 2022 18:26

cee-chen added the ci:deploy-cloud label Apr 7, 2022

sabarasaba approved these changes Apr 11, 2022

View reviewed changes

botelastic bot added the Team:APM All issues that need APM UI Team support label Apr 11, 2022

jbudz approved these changes Apr 11, 2022

View reviewed changes

dgieselaar approved these changes Apr 11, 2022

View reviewed changes

kqualters-elastic reviewed Apr 11, 2022

View reviewed changes

jportner reviewed Apr 11, 2022

View reviewed changes

Heenawter approved these changes Apr 11, 2022

View reviewed changes

Joe Portner and others added 2 commits April 11, 2022 12:25

Clean up spaces test snapshots

3ddd8bd

Merge branch 'main' into eui-54.0.0

84457df

andrew-goldstein approved these changes Apr 11, 2022

View reviewed changes

kertal approved these changes Apr 12, 2022

View reviewed changes

cee-chen added 2 commits April 12, 2022 08:46

Merge remote-tracking branch 'upstream/main' into eui-54.0.0

f00164a

[Security feedback] Revert 793d208 and hard-code visibleCellActions

fc93474

kevinlog approved these changes Apr 12, 2022

View reviewed changes

mbondyra approved these changes Apr 12, 2022

View reviewed changes

tsullivan approved these changes Apr 12, 2022

View reviewed changes

jportner approved these changes Apr 12, 2022

View reviewed changes

cee-chen merged commit 0955953 into elastic:main Apr 12, 2022

cee-chen deleted the eui-54.0.0 branch April 12, 2022 18:00

tylersmalley added ci:cloud-deploy Create or update a Cloud deployment and removed ci:deploy-cloud labels Aug 17, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade EUI to v54.0.0 #129653

Upgrade EUI to v54.0.0 #129653

cee-chen commented Apr 6, 2022

cee-chen Apr 6, 2022

chandlerprall Apr 11, 2022

andrew-goldstein Apr 11, 2022

sabarasaba left a comment

elasticmachine commented Apr 11, 2022

kqualters-elastic Apr 11, 2022

kqualters-elastic Apr 11, 2022

cee-chen Apr 11, 2022

cee-chen Apr 12, 2022

jportner Apr 11, 2022

jportner Apr 11, 2022

Heenawter left a comment

andrew-goldstein left a comment

kertal left a comment

cee-chen commented Apr 12, 2022

kevinlog left a comment

mbondyra left a comment

kertal commented Apr 12, 2022

tsullivan left a comment

kibana-ci commented Apr 12, 2022

jportner left a comment

cee-chen commented Apr 12, 2022

Upgrade EUI to v54.0.0 #129653

Upgrade EUI to v54.0.0 #129653

Conversation

cee-chen commented Apr 6, 2022

Summary

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sabarasaba left a comment

Choose a reason for hiding this comment

elasticmachine commented Apr 11, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Heenawter left a comment

Choose a reason for hiding this comment

andrew-goldstein left a comment

Choose a reason for hiding this comment

kertal left a comment

Choose a reason for hiding this comment

cee-chen commented Apr 12, 2022

kevinlog left a comment

Choose a reason for hiding this comment

mbondyra left a comment

Choose a reason for hiding this comment

kertal commented Apr 12, 2022

tsullivan left a comment

Choose a reason for hiding this comment

kibana-ci commented Apr 12, 2022

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

History

jportner left a comment

Choose a reason for hiding this comment

cee-chen commented Apr 12, 2022