-
Notifications
You must be signed in to change notification settings - Fork 8.5k
Upgrade EUI to v55.1.2 #131203
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 v55.1.2 #131203
Conversation
* Updating package.json to latest EUI * Adding one translation.
|
@elasticmachine merge upstream |
YulNaumenko
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Security Solution changes LGTM
shahzad31
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uptime changes LGTM !!
cnasikas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ResponseOps changes LGTM
| </KibanaContextProvider> | ||
| ); | ||
| expect(comp.children()).toMatchSnapshot(); | ||
| expect(comp.children().render()).toMatchSnapshot(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Data Discovery would like to revert changes in src/plugins/discover/public/services/doc_views/components/doc_viewer_source/source.test.tsx and source.test.tsx.snap otherwise the functionality which tests are checking is excluded in snapshots.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jughosta I removed the render() call and updated the snapshot. CI build is running now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
efegurkan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enterprise Search Changes looks good, tested locally.
| button={<button />} | ||
| Array [ | ||
| <button | ||
| aria-controls="collapsibe-nav" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a typo? Shouldn't it be collapsible?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't speak to it shoudl being collapsible but don't believe it's a typo. If the "collapsible-nav" id didn't exist, a11y tests would throw an error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was not introduced by the upgrade anyway, see line 313 of the old version of this file, so I think we're fine
jughosta
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Data Discovery changes look good!
peteharverson
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ML code changes LGTM
paul-tavares
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Files under x-pack/plugins/security_solution/public/management/* LGTM 👍
ThomThomson
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The smaller snapshots are great! LGTM!
|
@elasticmachine merge upstream |
💚 Build SucceededMetrics [docs]Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: cc @1Copenut |
|
@elastic/kibana-reporting-services @elastic/kibana-app-services @elastic/apm-ui @joeypoon We'll plan on merging this by EOD May 12. Please get your reviews in before then 🙇 |
joeypoon
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👌 for OLM
| button={<button />} | ||
| Array [ | ||
| <button | ||
| aria-controls="collapsibe-nav" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was not introduced by the upgrade anyway, see line 313 of the old version of this file, so I think we're fine
Dosant
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
App services tests/snapshot changes lgmt 👍
tsullivan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reporting changes LGTM
* Bumping EUI to v55.1.0 * Updating package.json to latest EUI * Adding one translation. * Updating theme config to pass Jest Tests elastic#4 / CaseView. * Updating theme config to pass Jest Tests 3, CauseStacktrace. * Updating six x-pack synthetic test snapshots. * Updating snapshots for discover, reporting, security_solution. * Replacing instances of spacerSizes with euiSize. * Updating a number of snapshots for Emotion styles. * Adding more snapshots for Emotion style upgrade. * Updating seven snapshot tests for Emotion styles. * Updating two snapshot tests for Emotion styles. * Adding two more snapshots for License and Upload. * Updating a Typescript check, and classname count in one unit test. * Updating 1 snapshot and refining EuiLoadingChart selector to avoid off-by-one error. * Bumping EUI to 55.1.1 for a change in EUI Flyout behavior. * update newsfeed flyout to use shards * snapshot update * eui to v55.1.2 * update onClose * onClose types * reduce snapshot noise * reduce snapshot noise * Adding back Emotion output at request of Data Discovery reviewer. Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Greg Thompson <[email protected]>
Summary
[email protected] ⏩ [email protected]
Added
.render()calls to snapshot tests are intended to reduce the output of EUI components that use Emotion for styling. If you prefer the more verbose snapshot output in your plugin, we can remove the call.55.1.2EuiOverlayMaskto accept a React ref (#5876)Bug fixes
EuiFlyoutoutsideClickClosesnot being scoped to overlay mask whenownFocus=true(#5876)55.1.1focusTrapPropsprop toEuiFlyoutto aid outside click detection and closing event (#5860)55.1.0EuiTimelinea11y by using better semantic elements (#5791)EuiAspectRatiowith inline styles usingaspect-ratioproperty of css (#5818)EuiLoadingChart(#5821)euiFontSize()anduseEuiFontSize()JS function and React hook for font sizing (#5822)levelsobject toEuiTheme(#5827)@emotion/cacheto include all@emotionstyles (#5831)Bug fixes
EuiAccordionchildren that useposition: fixed;(#5806)EuiFlyoutso that it no longer closes when a click starts inside the flyout but completes outside (#5810)EuiBasicTablemobile styles being in sync between JS and Sass (#5822)CSS-in-JS conversions
EuiSpacerto Emotion; Removed$spacerSizes(#5812)EuiBeaconto Emotion (#5814)euiCanAnimateto a constant (#5814)EuiHorizontalRuleto emotion; Removed$ruleMargins(#5815)EuiLoadingChartto Emotion (#5821)