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 83.0.0 #160813

Merged
merged 37 commits into from
Jul 6, 2023
Merged

Upgrade EUI to 83.0.0 #160813

merged 37 commits into from
Jul 6, 2023

Conversation

breehall
Copy link
Contributor

@breehall breehall commented Jun 28, 2023

[email protected]83.0.0

⚠️ The biggest change in this PR by far is the EuiButtonEmpty Emotion conversion, which changes the DOM structure of the button slightly as well as several CSS classes around it.

EUI has attempted to convert any custom EuiButtonEmpty CSS overrides where possible, but would super appreciate it if CODEOWNERS checked their touched files. If anything other than a snapshot or test was touched, please double check the display of your button(s) and confirm everything still looks shipshape. Feel free to ping us for advice if not.


83.0.0

Bug fixes

  • Fixed EuiPaginationButton styling affected by EuiButtonEmpty's Emotion conversion (#6893)

Breaking changes

  • Removed isPlaceholder prop from EuiPaginationButton (#6893)

82.2.1

  • Updated supported Node engine versions to allow Node 16, 18 and >=20 (#6884)

82.2.0

  • Updated EUI's SVG icons library to use latest SVGO v3 optimization (#6843)
  • Added success color EuiNotificationBadge (#6864)
  • Added badgeColor prop to EuiFilterButton (#6864)
  • Updated EuiBadge to use CSS-in-JS for named colors instead of inline styles. Custom colors will still use inline styles. (#6864)

CSS-in-JS conversions

  • Converted EuiButtonGroup and EuiButtonGroupButton to Emotion (#6841)
  • Converted EuiButtonIcon to Emotion (#6844)
  • Converted EuiButtonEmpty to Emotion (#6863)
  • Converted EuiCollapsibleNav and EuiCollapsibleNavGroup to Emotion (#6865)
  • Removed Sass variables $euiCollapsibleNavGroupLightBackgroundColor, $euiCollapsibleNavGroupDarkBackgroundColor, and $euiCollapsibleNavGroupDarkHighContrastColor (#6865)

@breehall breehall added release_note:skip Skip the PR/issue when compiling release notes EUI backport:skip This commit does not require backporting v8.9.0 labels Jun 28, 2023
@breehall breehall changed the title Upgrade EUI to 82.2.1 Upgrade EUI to 83.0.0 Jun 29, 2023
@cee-chen cee-chen force-pushed the eui/82.2.1 branch 4 times, most recently from 9d2134c to fe30181 Compare June 30, 2023 17:11
@cee-chen
Copy link
Member

@elasticmachine merge upstream

breehall and others added 16 commits June 30, 2023 17:53
- comment is likely no longer accurate (4 years old), and `EuiButtonIcon` supports a loading spinner just fine
- Remove references to removed modifier classes

- Prefer checks for `disabled` prop over className checks
- these classNames no longer have any CSS attached due to the Emotion conversion

- AFAICT, the overrides are primarily around text size and weight, so set those intead

- use components where possible and Emotion CSS instead of inline style
- these classNames no longer have any CSS attached due to the Emotion conversion

- AFAICT, the overrides are primarily around text size and weight, as well as an incidental hover, so set those instead
- remove `__content` selector - padding is now set on the parent button

- use EUI theme var instead of static px width

- clean up minimizePadding ternary

- removing padding class on `EuiButtonIcon` which does not set any padding
- EuiButtonEmpty now has its padding set on the top-most button, and not on `euiButtonEmpty__content`

+ use new logical `-inline` property while here, which sets both horizontal sides at once
`.euiButtonEmpty__text` no longer uses `margin-left` to set a distance between any icon(s) and text, instead `.euiButtonEmpty__content` now sets a flex `gap` property
`.euiButtonContent__icon` is no longer in use (removed/deprecated) - asert on the data type instead
- there aren't any being used in Sass in any case
Copy link
Contributor

@ThomThomson ThomThomson left a comment

Choose a reason for hiding this comment

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

Presentation team snapshot changes LGTM! I also tested locally to ensure that the maps layers expand button properly shows the loading state, and I noticed something small - the drop shadow is removed while the button is loading. Are custom classes not properly applied to the icon button when it's loading?

shadow

Either way, this doesn't block the EUI upgrade

return (
<EuiButtonIcon
className="mapLayerControl__openLayerTOCButton"
color="text"
isLoading={isLoading}
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice simplification!

Copy link
Contributor

@jpdjere jpdjere left a comment

Choose a reason for hiding this comment

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

Reviewed and tested our Detection Rules Management codeowner changes; everything looks fine.

Not approving yet, but delegating to @spong so he can take a look at the changes @banderror tagged him in.

@jpdjere jpdjere requested a review from spong July 4, 2023 20:53
@breehall
Copy link
Contributor Author

breehall commented Jul 5, 2023

@elasticmachine merge upstream

@breehall
Copy link
Contributor Author

breehall commented Jul 5, 2023

@elasticmachine merge upstream

Copy link
Member

@spong spong left a comment

Choose a reason for hiding this comment

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

Security Assistant changes LGTM, so that rounds out the Security Solution review. 👍 Thank you @banderror & @jpdjere for the ping here, and thank you @breehall for the upgrade! 🙌

kevinlog added a commit that referenced this pull request Jul 6, 2023
## Summary

Skips flaky cypress tests to unblock PRs. These tests are determined to
be flaky as they are failing in two unrelated PRs.

#160912
#160813

### Checklist
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: Kibana Machine <[email protected]>
@breehall
Copy link
Contributor Author

breehall commented Jul 6, 2023

@elasticmachine merge upstream

@breehall
Copy link
Contributor Author

breehall commented Jul 6, 2023

@elasticmachine merge upstream

Copy link
Contributor

@delanni delanni left a comment

Choose a reason for hiding this comment

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

Licence checker changes 👌

@breehall
Copy link
Contributor Author

breehall commented Jul 6, 2023

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Rules, Alerts and Exceptions ResponseOps Cypress Tests on Security Solution #2 / Export rules exports only custom rules
  • [job] [logs] FTR Configs #30 / Machine Learning jobs update groups "before all" hook for "returns expected list of groups after update"

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
cases 421.7KB 421.9KB +167.0B
cloudSecurityPosture 229.1KB 228.9KB -144.0B
enterpriseSearch 2.5MB 2.5MB -156.0B
infra 1.9MB 1.9MB +198.0B
kubernetesSecurity 38.8KB 38.8KB -25.0B
lens 1.3MB 1.3MB -34.0B
maps 2.7MB 2.7MB -230.0B
security 567.0KB 567.0KB +42.0B
securitySolution 11.0MB 11.0MB -571.0B
synthetics 907.1KB 907.0KB -69.0B
unifiedSearch 212.9KB 212.9KB -30.0B
uptime 518.8KB 518.7KB -118.0B
visTypeVega 1.7MB 1.7MB +198.0B
total -772.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 312.5KB 298.8KB -13.7KB
kbnUiSharedDeps-npmDll 6.0MB 6.0MB +5.5KB
visualizationUiComponents 38.3KB 38.2KB -31.0B
total -8.2KB
Unknown metric groups

ESLint disabled line counts

id before after diff
enterpriseSearch 14 16 +2
securitySolution 410 414 +4
total +6

Total ESLint disabled count

id before after diff
enterpriseSearch 15 17 +2
securitySolution 489 493 +4
total +6

History

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

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:all-cypress-suites 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.10.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.