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 EUI to v91.0.0 (with backports) #170716

Merged
merged 100 commits into from
Dec 18, 2023
Merged

Upgrade EUI to v91.0.0 (with backports) #170716

merged 100 commits into from
Dec 18, 2023

Conversation

tkajtoch
Copy link
Member

@tkajtoch tkajtoch commented Nov 7, 2023

v90.0.0v91.0.0-backport.0

⚠️ While this upgrade pings many teams and has a large code diff, the majority of the changes are snapshots or tests-related and do not touch source code, so should theoretically only need a code review and not dedicated QA.

The changes in EUI that required a large swathe of these updates are:

  • EuiPopover removed an extra unnecessary <div> wrapper on its anchors, which affected many snapshots and a few CSS overrides, which should have been updated
  • EuiButtonGroup now renders <button> elements instead of <input type="radio"> elements for single selection, which affected both snapshots and E2E tests
  • EuiSuperDatePicker's absolute date input now requires an Enter keypress when parsing dates (affected E2E tests)
  • EuiComboBox, when rendered with singleSelection={{ plainText: 'true' }}, no longer renders a pill (i.e. text). This combobox type now behaves more like an EuiFieldText, where the selection is rendered via input value instead. This affected a high amount of E2E tests (both FTR and Cypress), both in terms of updating assertions and changing selections, but should not significantly affect user experience - see [EuiComboBox] Refactor singleSelection.asPlainText to make greater use of the underlying input element eui#7332 for more.

v91.0.0-backport.0

This is a backport release only intended for use by Kibana.

  • Added esqlVis, pipeBreaks, and pipeNoBreaks icon glyphs.
  • EuiSelectable now allows configurable text truncation via listProps.truncationProps (#7388)
  • EuiTextTruncate now supports a new calculationDelayMs prop for working around font loading or layout shifting scenarios (#7388)

Bug fixes

  • Fixed a bug with EuiSelectables with custom truncationProps, where scrollbar widths were not being accounted for (#7392)

91.0.0

  • Updated the background color of EuiPopovers in dark mode to increase visibility & contrast against other page/panel backgrounds (#7310)
  • Memoized EuiDataGrid to prevent unneeded re-renders (#7324)
  • Added a configurable role prop to EuiAccordion (#7326)
  • Added a configurable role prop to EuiGlobalToastList (#7328)
  • For greater flexibility, EuiSuperDatePicker now allows users to paste ISO 8601, RFC 2822, and Unix timestamps in the Absolute tab input, in addition to timestamps in the dateFormat prop (#7331)
  • Plain text EuiComboBoxes now behave more like a normal text field/input. Backspacing will no longer delete the entire value, and selected values can now be double clicked and copied. (#7332)
  • EuiDataGrid's display settings popover now allows users to clear the "Lines per row" input before typing in a new number (#7338)
  • Improved the UX of EuiSuperDatePicker's Absolute tab for users manually typing in timestamps (#7341)
  • Updated EuiI18ns with multiple tokens to accept dynamic values (#7341)

Bug fixes

  • Fixed EuiComboBox's onSearchChange callback to pass the correct hasMatchingOptions value (#7334)
  • Fixed an EuiSelectableTemplateSitewide bug where the popoverButton behavior would break if passed a non-DOM React wrapper (#7339)

Deprecations

  • EuiPopover: deprecated anchorClassName. Use className instead (#7311)
  • EuiPopover: deprecated buttonRef. Use popoverRef instead (#7311)
  • EuiPopover: removed extra .euiPopover__anchor div wrapper. Target .euiPopover instead if necessary (#7311)
  • Deprecated EuiButtonGroup's name prop. This can safely be removed. (#7325)

Breaking changes

  • Removed deprecated euiPaletteComplimentary - use euiPaletteComplementary Instead (#7333)

Accessibility

  • Updated type="single" EuiButtonGroups to render standard buttons instead of radio buttons under the hood, per recent a11y recommendations (#7325)
  • EuiAccordion now defaults to a less screenreader-noisy group role instead of region. If your accordion contains significant enough content to be a document landmark role, you may re-configure it back to region. (#7326)
  • Reduced screen reader noisiness when sorting EuiDataGrid columns via toolbar (#7327)
  • EuiGlobalToastList now defaults to a log role. If your toasts will always require immediate user action, consider (with caution) using the alert role instead. (#7328)

CSS-in-JS conversions

  • Updated $euiFontFamily and $euiCodeFontFamily to match Emotion fonts (#7332)

@tkajtoch tkajtoch added release_note:skip Skip the PR/issue when compiling release notes EUI backport:skip This commit does not require backporting labels Nov 7, 2023
@tkajtoch tkajtoch self-assigned this Nov 7, 2023
@tkajtoch tkajtoch changed the title Upgrade @elastic/eui to v91.0.0 Upgrade EUI to v91.0.0 Nov 7, 2023
@tkajtoch tkajtoch force-pushed the eui-v91.x branch 15 times, most recently from 8a7a7d9 to 0e3ea13 Compare November 14, 2023 11:47
cee-chen added a commit to cee-chen/kibana that referenced this pull request Nov 16, 2023
- can be reverted once elastic#170716 merges
cee-chen added a commit to cee-chen/kibana that referenced this pull request Nov 16, 2023
- can be reverted once elastic#170716 merges
@tkajtoch tkajtoch force-pushed the eui-v91.x branch 2 times, most recently from 2c67880 to 977a4b4 Compare November 20, 2023 15:47
@tkajtoch tkajtoch force-pushed the eui-v91.x branch 3 times, most recently from 3876821 to 53e6a31 Compare November 22, 2023 13:25
@cee-chen cee-chen added the ci:skip-cypress-osquery Skips osquery cypress checks label Nov 27, 2023
Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

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

Changes to ML tests LGTM (code review only).

Copy link
Member

@azasypkin azasypkin left a comment

Choose a reason for hiding this comment

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

Changes in Security/Spaces tests LGTM.

Copy link
Member

@pheyos pheyos left a comment

Choose a reason for hiding this comment

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

web_element_wrapper test service change LGTM

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.

Tested to ensure that the removal of anchorClassName didn't impact anything visually + looked over test changes. LGTM 👍

@botelastic botelastic bot added the Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team label Dec 13, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-ux-infra_services-team (Team:obs-ux-infra_services)

Copy link
Contributor

@justinkambic justinkambic left a comment

Choose a reason for hiding this comment

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

Code LGTM, no smoke testing on my end

@cee-chen
Copy link
Member

@elastic/security-detection-rule-management @elastic/security-threat-hunting-investigations @elastic/security-threat-hunting-explore @elastic/kibana-visualizations @elastic/kibana-data-discovery @elastic/response-ops @elastic/fleet @elastic/kibana-design - last ping for reviews. We'll be asking KibanaOps to admin merge on Monday.

Copy link
Contributor

@banderror banderror left a comment

Choose a reason for hiding this comment

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

Changes for the Rule Management page (x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/components/rules_table/rules_table_filters/rule_search_field.tsx) LGTM -- tested this locally.

As long as CI is happy with the rest of the changes in tests, I'm happy too 👍

Copy link
Contributor

@mattkime mattkime left a comment

Choose a reason for hiding this comment

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

changes lgtm

@stratoula
Copy link
Contributor

/ci

@stratoula
Copy link
Contributor

Wow I see some bundle sizes decrease 👏

Copy link
Contributor

@stratoula stratoula left a comment

Choose a reason for hiding this comment

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

Visualizations team changes LGTM

Copy link
Contributor

@stephmilovic stephmilovic left a comment

Choose a reason for hiding this comment

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

LGTM for explore team. thanks!

@stephmilovic
Copy link
Contributor

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

kibana-ci commented Dec 18, 2023

💔 Build Failed

Failed CI Steps

Test Failures

  • [job] [logs] Defend Workflows Cypress Tests #11 / Response console From Alerts "after all" hook for "should open responder from alert details flyout" "after all" hook for "should open responder from alert details flyout"
  • [job] [logs] Defend Workflows Cypress Tests #11 / Response console From Alerts "after all" hook for "should open responder from alert details flyout" "after all" hook for "should open responder from alert details flyout"
  • [job] [logs] Defend Workflows Cypress Tests #11 / Response console From Alerts "before all" hook for "should open responder from alert details flyout" "before all" hook for "should open responder from alert details flyout"
  • [job] [logs] Defend Workflows Cypress Tests #11 / Response console From Alerts "before all" hook for "should open responder from alert details flyout" "before all" hook for "should open responder from alert details flyout"
  • [job] [logs] Defend Workflows Cypress Tests #9 / Unenroll agent from fleet changing when agent tamper protection is enabled but then is switched to a policy with it disabled "after all" hook for "should unenroll from fleet without issues" "after all" hook for "should unenroll from fleet without issues"
  • [job] [logs] Defend Workflows Cypress Tests #9 / Unenroll agent from fleet changing when agent tamper protection is enabled but then is switched to a policy with it disabled "after all" hook for "should unenroll from fleet without issues" "after all" hook for "should unenroll from fleet without issues"
  • [job] [logs] Defend Workflows Cypress Tests #9 / Unenroll agent from fleet changing when agent tamper protection is enabled but then is switched to a policy with it disabled "before each" hook for "should unenroll from fleet without issues" "before each" hook for "should unenroll from fleet without issues"
  • [job] [logs] Defend Workflows Cypress Tests #9 / Unenroll agent from fleet changing when agent tamper protection is enabled but then is switched to a policy with it disabled "before each" hook for "should unenroll from fleet without issues" "before each" hook for "should unenroll from fleet without issues"
  • [job] [logs] Defend Workflows Cypress Tests #8 / Unenroll agent from fleet when agent tamper protection is disabled but then is switched to a policy with it enabled "after all" hook for "should unenroll from fleet without issues" "after all" hook for "should unenroll from fleet without issues"
  • [job] [logs] Defend Workflows Cypress Tests #8 / Unenroll agent from fleet when agent tamper protection is disabled but then is switched to a policy with it enabled "after all" hook for "should unenroll from fleet without issues" "after all" hook for "should unenroll from fleet without issues"
  • [job] [logs] Defend Workflows Cypress Tests #8 / Unenroll agent from fleet when agent tamper protection is disabled but then is switched to a policy with it enabled "before each" hook for "should unenroll from fleet without issues" "before each" hook for "should unenroll from fleet without issues"
  • [job] [logs] Defend Workflows Cypress Tests #8 / Unenroll agent from fleet when agent tamper protection is disabled but then is switched to a policy with it enabled "before each" hook for "should unenroll from fleet without issues" "before each" hook for "should unenroll from fleet without issues"
  • [job] [logs] Defend Workflows Cypress Tests #10 / Uninstall agent from host changing agent policy when agent tamper protection is disabled but then is switched to a policy with it enabled "after all" hook for "should uninstall from host without issues" "after all" hook for "should uninstall from host without issues"
  • [job] [logs] Defend Workflows Cypress Tests #10 / Uninstall agent from host changing agent policy when agent tamper protection is disabled but then is switched to a policy with it enabled "after all" hook for "should uninstall from host without issues" "after all" hook for "should uninstall from host without issues"
  • [job] [logs] Defend Workflows Cypress Tests #10 / Uninstall agent from host changing agent policy when agent tamper protection is disabled but then is switched to a policy with it enabled "before each" hook for "should uninstall from host without issues" "before each" hook for "should uninstall from host without issues"
  • [job] [logs] Defend Workflows Cypress Tests #10 / Uninstall agent from host changing agent policy when agent tamper protection is disabled but then is switched to a policy with it enabled "before each" hook for "should uninstall from host without issues" "before each" hook for "should uninstall from host without issues"

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
aiops 424 425 +1
apm 1541 1543 +2
infra 1330 1328 -2
maps 1097 1095 -2
ml 1870 1872 +2
securitySolution 4832 4833 +1
securitySolutionServerless 500 501 +1
visTypeVega 310 308 -2
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
aiops 390.1KB 390.2KB +96.0B
apm 3.7MB 3.7MB +138.0B
canvas 1013.9KB 1013.9KB -18.0B
cloudSecurityPosture 451.7KB 451.7KB -36.0B
controls 198.4KB 198.2KB -167.0B
dashboard 377.3KB 377.2KB -150.0B
discover 593.9KB 593.8KB -54.0B
enterpriseSearch 2.7MB 2.7MB -108.0B
eventAnnotationListing 196.9KB 196.9KB -16.0B
exploratoryView 203.7KB 203.6KB -120.0B
fleet 1.2MB 1.2MB -60.0B
graph 388.2KB 387.9KB -300.0B
infra 1.9MB 1.9MB -4.7KB
lens 1.4MB 1.4MB -48.0B
maps 2.9MB 2.9MB -4.9KB
ml 3.6MB 3.6MB -481.0B
securitySolution 13.1MB 13.1MB -389.0B
securitySolutionServerless 361.1KB 361.2KB +93.0B
unifiedDocViewer 58.6KB 58.4KB -168.0B
unifiedSearch 215.7KB 215.4KB -255.0B
visTypeTimeseries 511.8KB 511.8KB -18.0B
visTypeVega 1.8MB 1.8MB -4.7KB
total -16.3KB

Page load bundle

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

id before after diff
console 29.2KB 29.1KB -150.0B
core 375.1KB 375.3KB +186.0B
embeddable 78.7KB 78.7KB -78.0B
kbnUiSharedDeps-css 256.1KB 255.9KB -273.0B
kbnUiSharedDeps-npmDll 6.2MB 6.2MB -1.5KB
unifiedSearch 35.5KB 35.4KB -36.0B
total -1.9KB
Unknown metric groups

ESLint disabled line counts

id before after diff
securitySolution 468 467 -1

Total ESLint disabled count

id before after diff
securitySolution 538 537 -1

History

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

cc @tkajtoch

@cee-chen
Copy link
Member

We just spoke to @jbudz and @kevinlog and received acknowledgement / approval that the failing Cypress Defend tests are not related to the EUI upgrade (thank you so much for merging in main to check however Stratoula/Steph, y'all are awesome)!

As this PR has been open for a while now, and we've alerted multiple teams at various times, we'll be moving forward with asking for an admin merge on this PR shortly.

@jbudz jbudz merged commit b043545 into elastic:main Dec 18, 2023
44 of 50 checks passed
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 Feature:Embedding Embedding content via iFrame release_note:skip Skip the PR/issue when compiling release notes Team:APM All issues that need APM UI Team support Team:Fleet Team label for Observability Data Collection Fleet team Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team v8.13.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.