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 v52.2.0 #128313

Merged
merged 20 commits into from
Mar 29, 2022
Merged

Upgrade EUI to v52.2.0 #128313

merged 20 commits into from
Mar 29, 2022

Conversation

breehall
Copy link
Contributor

@breehall breehall commented Mar 22, 2022

Summary

eui@51.1.0eui@52.2.0

  • Snapshot churn is largely due to mocked EuiIcon content including the aria-label as text. This helps the automated a11y testing suite, but the EUI team is evaluating a different approach that would be less invasive. It's possible that these snapshot changes will be negated in the next release of EUI.

52.2.0

  • Added branchUser, desktop and sessionViewer glyphs to EuiIcon (#5740)

52.1.0

  • Added anchor prop to EuiTourStep to allow for DOM selector attachment (#5696)
  • EuiDataGrid now forces isExpandable to be true if any cellActions are passed, as keyboard users are otherwise unable to access cell actions without the expansion popover (#5710)

Bug fixes

  • Fixed EuiDataGrid not rerendering correctly on row heights change (#5712)
  • Fixed EuiContextMenu requiring two tab keypresses to advance to the next focusable menu item (#5719)
  • Fixed EuiDataGrid footer cell focus bugging out after moving its column (#5720)

52.0.0

  • Added editorChecklist glyph to EuiIcon (#5705)
  • Updated testenv mock for EuiIcon to render aria-label as text (#5709)
  • Added compressed prop to EuiFilterGroup and reduced the size of the EuiFilterButton notification badge (#5717)
  • Increased contrast of EuiSelectableTemplateSitewide input text when in dark header (#5724)

Breaking changes

  • Removed Legacy theme including compiled CSS (#5688)
  • Removed flush and size props in EuiFilterButtonProps (#5717)

CSS-in-JS conversions

  • Converted EuiMark to CSS-in-JS styling (#4575)

@breehall breehall added release_note:skip Skip the PR/issue when compiling release notes EUI v8.3.0 ci:deploy-cloud labels Mar 22, 2022
…ing for strict text equality to account for text coming from the EuiScreenReaderOnly component. Also updated tests to account for EuiIcon text that is now rendered when the icon is imported from .testenv (PR 5709 - elastic/eui#5709).
…into upgrade-eui-52.1.0

Pulling the latest code from the upgrade-eui-52.1.0 branch
@thompsongl thompsongl changed the title Upgrade EUI to v52.1.0 Upgrade EUI to v52.2.0 Mar 24, 2022
…apshots.

Updated tests using getAllByLabelText and getByLabelText to getAllByText and getByText respectively as the former have been deprecated
…pshots. Updated instances of getByLabelText and getAllByLabelText to getByText and getAllByText as the former are now deprecated.
@@ -25,7 +25,7 @@ describe('NotFoundErrors component', () => {
const callOut = mounted.find('EuiCallOut');
expect(callOut).toMatchSnapshot();
expect(mounted.text()).toMatchInlineSnapshot(
`"There is a problem with this saved objectThe saved search associated with this object no longer exists.If you know what this error means, you can use the Saved objects APIs(opens in a new tab or window) to fix it — otherwise click the delete button above."`
`"There is a problem with this saved objectThe saved search associated with this object no longer exists.If you know what this error means, you can use the Saved objects APIsExternal link(opens in a new tab or window) to fix it — otherwise click the delete button above."`
Copy link
Member

Choose a reason for hiding this comment

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

Hmm I am not sure this change from APIs to APIsExternal link is intended?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey Bamieh, this change comes from PR 5709 (elastic/eui#5709) where all of the icons being used by .testenv now render text as a <span> to notate what the icon is. Now, where an icon would be used, we will see text for that icon inline. I'm in the process of updating the tests that were affected by this.

Copy link
Member

Choose a reason for hiding this comment

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

OK cool. Thank you 👍

@thompsongl thompsongl marked this pull request as ready for review March 28, 2022 22:55
@thompsongl thompsongl requested review from a team as code owners March 28, 2022 22:55
@elasticmachine
Copy link
Contributor

Pinging @elastic/uptime (Team:uptime)

Copy link
Contributor

@YulNaumenko YulNaumenko left a comment

Choose a reason for hiding this comment

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

security_solution and timelines changes LGTM

Copy link
Contributor

@alisonelizabeth alisonelizabeth left a comment

Choose a reason for hiding this comment

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

deployment management changes LGTM

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Test Failures

  • [job] [logs] Security Solution Tests / risk tab filters the table
  • [job] [logs] Security Solution Tests / risk tab renders the table
  • [job] [logs] Security Solution Tests / risk tab should be able to change items count per page
  • [job] [logs] Security Solution Tests / risk tab should not allow page change when page is empty
  • [job] [logs] Security Solution Tests / value lists user with restricted access role "before each" hook for "Does not allow a t1 analyst user to upload a value list"

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 2.8MB 2.8MB -1.0B
infra 1002.1KB 1002.1KB -9.0B
total -10.0B

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 595.5KB 595.5KB -47.0B
kbnUiSharedDeps-npmDll 4.8MB 4.8MB +557.0B
total +510.0B

History

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

Copy link
Member

@dgieselaar dgieselaar left a comment

Choose a reason for hiding this comment

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

APM changes LGTM

Copy link
Member

@Bamieh Bamieh left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@thomheymann thomheymann left a comment

Choose a reason for hiding this comment

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

security changes LGTM

Copy link
Contributor

@cqliu1 cqliu1 left a comment

Choose a reason for hiding this comment

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

Canvas changes LGTM

Copy link
Contributor

@flash1293 flash1293 left a comment

Choose a reason for hiding this comment

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

VisEditors changes LGTM, code review only

Copy link
Member

@kertal kertal left a comment

Choose a reason for hiding this comment

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

LGTM, just snapshot were changed, did also a very basic cloud test, works as expected

@tylersmalley
Copy link
Contributor

Merging ahead of the awaiting reviews at the request of the EUI team. If you have any issues here please raise them with EUI, thanks.

@tylersmalley tylersmalley merged commit dccd829 into main Mar 29, 2022
@tylersmalley tylersmalley deleted the upgrade-eui-52.1.0 branch March 29, 2022 20:39
@tylersmalley
Copy link
Contributor

tylersmalley commented Mar 29, 2022

This resulted in some Jest failures once it hit main, so, unfortunately, it will need to be reverted. Working on that now.

edit: revert at f782f8b

tylersmalley pushed a commit that referenced this pull request Mar 29, 2022
@cee-chen cee-chen restored the upgrade-eui-52.1.0 branch March 29, 2022 22:09
@tylersmalley tylersmalley deleted the upgrade-eui-52.1.0 branch March 29, 2022 22:10
This was referenced Mar 29, 2022
Copy link
Contributor

@awahab07 awahab07 left a comment

Choose a reason for hiding this comment

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

Uptime and User Experience plugin changes LGTM. Only reviewed code as just a few tests are changed.

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 128313 or prevent reminders by adding the backport:skip label.

@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 30, 2022
@thompsongl thompsongl added backport:skip This commit does not require backporting and removed backport missing Added to PRs automatically when the are determined to be missing a backport. labels Mar 30, 2022
@tylersmalley tylersmalley added ci:cloud-deploy Create or update a Cloud deployment and removed ci:deploy-cloud labels Aug 17, 2022
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 ci:cloud-deploy Create or update a Cloud deployment EUI release_note:skip Skip the PR/issue when compiling release notes Team:APM All issues that need APM UI Team support Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v8.2.0 v8.3.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.