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

Upgraded EUI to v29.3.0 #78870

Merged
merged 11 commits into from
Oct 5, 2020
Merged

Conversation

chandlerprall
Copy link
Contributor

@chandlerprall chandlerprall commented Sep 29, 2020

Summary

29.3.0

  • Added both option to flush prop of EuiButtonEmpty (#4084)

Bug fixes

  • Fixed EuiRange and EuiDualRange display of internal spacer (#4084)
  • Fixed EuiFieldSearch padding for the different states (#4084)
  • Fixed EuiCheckableCard disabled but checked styles (#4084)

Theme: Amsterdam

  • Fixed line-height on EuiTitle (#4079)

29.2.0

  • Improved contrast for EuiIcon and EuiButtonIcon named colors. This affects EuiHealth which uses the EuiIcon colors. (#4049)
  • Added color accent to EuiButtonIcon (#4049)

Theme: Amsterdam

  • Removed border-radius from EuiCallout (#4066)
  • Updated styles for EuiToast (#4076)

29.1.0

  • Added footer row to EuiDataGrid via the renderFooterCellValue prop (#3770)
  • Added column header menu to EuiDataGrid (#3087)
  • Added horizontal line separator to EuiContextMenu (#4018)
  • Added controlled pagination props to EuiInMemoryTablee (#4038)
  • Added gutterSize, popoverBreakpoints, popoverButtonProps, and popoverProps props to EuiHeaderLinks (#4046)
  • Added 'all' and 'none' options to the sizes prop of EuiHideFor and EuiShowFor (#4046)

Bug fixes

  • Fixed EuiTextColor playground error due to color prop not getting captured by the documentation generator (#4058)

@chandlerprall chandlerprall added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.10.0 labels Sep 29, 2020
@chandlerprall chandlerprall requested a review from a team September 29, 2020 20:54
@chandlerprall chandlerprall requested review from a team as code owners September 29, 2020 22:16
@chandlerprall
Copy link
Contributor Author

Putting back in draft while I make this actually an upgrade.

@chandlerprall chandlerprall marked this pull request as draft September 29, 2020 23:29
@cchaos cchaos changed the title Upgraded EUI to v29.2.0 Upgraded EUI to v29.3.0 Oct 1, 2020
I don’t think this was intended to change when I ran the updater
@cchaos
Copy link
Contributor

cchaos commented Oct 1, 2020

Test looks flakey. One last time for the night.
Jenkins, test this

Whelp, I tried. Jenkins ain't listening to me ☹️

@chandlerprall chandlerprall marked this pull request as ready for review October 1, 2020 04:07
@chandlerprall chandlerprall requested review from a team as code owners October 1, 2020 04:07
@spalger
Copy link
Contributor

spalger commented Oct 1, 2020

We're getting pretty close to implementing limits for bundle size increases in #78205 starting with limits that prevent increasing any bundle more than 5kb above its current size. This upgrade increases the size of the shared-ui-deps bundle by 500kb. Is this an expected or understood increase? I don't see anything in the changelog that would justify a size increase like that.

@chandlerprall
Copy link
Contributor Author

We're getting pretty close to implementing limits for bundle size increases in #78205 starting with limits that prevent increasing any bundle more than 5kb above its current size. This upgrade increases the size of the shared-ui-deps bundle by 500kb. Is this an expected or understood increase? I don't see anything in the changelog that would justify a size increase like that.

That is quite the jump. I'll take a look into what's causing it.

@walterra
Copy link
Contributor

walterra commented Oct 1, 2020

I checked the existing uses of data grid in the ML/Transform plugin. Everything works well, we'll just need to adjust the design of the custom headers with the histogram charts, I created an issue here to follow up once this PR in (#79066). Great update with the new actions for data grid columns!

Copy link
Contributor

@lizozom lizozom 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

@cchaos cchaos left a comment

Choose a reason for hiding this comment

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

Design LGTM! 😉

Copy link
Contributor

@poffdeluxe poffdeluxe 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

@spalger spalger left a comment

Choose a reason for hiding this comment

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

Looks like the size increase is caused by some new proptypes being automatically generated from TypeScript types, which are pretty massive in the uncompressed code but the 500kb increase is reduced to about 3kb with brotoli compression and 15kb with gzip compression. I think that's still pretty high, and something that I highly doubt provides any benefit to Kibana as we already validate against the TypeScript types these are derived from.

@spalger
Copy link
Contributor

spalger commented Oct 1, 2020

Just pushed a commit that will strip the proptypes from eui when building the ui-shared-deps bundle for production. Since these prop-types are added in a build step there's little-to-no risk that the propTypes are actually required for EUI to function correctly. In my testing locally this actually causes a reduction in the @elastic specific portion of the ui-shared-deps bundles.

Copy link
Contributor

@spalger spalger 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

@patrykkopycinski patrykkopycinski left a comment

Choose a reason for hiding this comment

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

SIEM/Endpoint changes LGTM!

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.

Doesn't seem that any KibanaApp code was modified by this update, but I've quickly checked the KibanaApp owned apps, and they are still alive and flourishing. Looking forward to used the features of this update!

@kibanamachine
Copy link
Contributor

💛 Build succeeded, but was flaky


Test Failures

Creates and activates a new EQL rule with a sequence.Detection rules, EQL Creates and activates a new EQL rule with a sequence

Link to Jenkins

Stack Trace

Failed Tests Reporter:
  - Test has failed 2 times on tracked branches: https://github.com/elastic/kibana/issues/79522

TypeError: Cannot read property 'apply' of undefined
    at listener (http://elastic:changeme@localhost:6111/__cypress/runner/cypress_runner.js:173184:19)
    at arrayMap (http://elastic:changeme@localhost:6111/__cypress/runner/cypress_runner.js:25373:23)
    at Function.map (http://elastic:changeme@localhost:6111/__cypress/runner/cypress_runner.js:34314:14)
    at $Cypress.events.emitMap (http://elastic:changeme@localhost:6111/__cypress/runner/cypress_runner.js:173188:16)
    at $Cypress.parent.<computed> [as emitMap] (http://elastic:changeme@localhost:6111/__cypress/runner/cypress_runner.js:173131:33)
    at $Cypress.action (http://elastic:changeme@localhost:6111/__cypress/runner/cypress_runner.js:166487:31)
    at fail (http://elastic:changeme@localhost:6111/__cypress/runner/cypress_runner.js:169982:22)
    at http://elastic:changeme@localhost:6111/__cypress/runner/cypress_runner.js:169811:14
    at tryCatcher (http://elastic:changeme@localhost:6111/__cypress/runner/cypress_runner.js:10325:23)
    at Promise._settlePromiseFromHandler (http://elastic:changeme@localhost:6111/__cypress/runner/cypress_runner.js:8260:31)
    at Promise._settlePromise (http://elastic:changeme@localhost:6111/__cypress/runner/cypress_runner.js:8317:18)
    at Promise._settlePromise0 (http://elastic:changeme@localhost:6111/__cypress/runner/cypress_runner.js:8362:10)
    at Promise._settlePromises (http://elastic:changeme@localhost:6111/__cypress/runner/cypress_runner.js:8438:18)
    at _drainQueueStep (http://elastic:changeme@localhost:6111/__cypress/runner/cypress_runner.js:5032:12)
    at _drainQueue (http://elastic:changeme@localhost:6111/__cypress/runner/cypress_runner.js:5025:9)
    at Async.../../node_modules/bluebird/js/release/async.js.Async._drainQueues (http://elastic:changeme@localhost:6111/__cypress/runner/cypress_runner.js:5041:5)

Metrics [docs]

@kbn/ui-shared-deps asset size

id before after diff
css 643.8KB 645.2KB +1.3KB
[email protected] 2.8MB 2.3MB -520.2KB
kbn-ui-shared-deps.js 4.6MB 4.6MB -11.0B
total -518.9KB

async chunks size

id before after diff
canvas 1.5MB 1.5MB +8.0B
discover 438.4KB 438.4KB +8.0B
enterpriseSearch 428.6KB 428.7KB +28.0B
lens 1.0MB 1.0MB +8.0B
securitySolution 10.3MB 10.3MB +398.0B
total +450.0B

distributable file count

id before after diff
default 47107 47114 +7
oss 28597 28604 +7

page load bundle size

id before after diff
embeddable 290.7KB 290.7KB +8.0B
navigation 21.9KB 21.7KB -174.0B
visTypeTimeseries 136.9KB 136.9KB +8.0B
total -158.0B

History

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

@chandlerprall chandlerprall merged commit 3bad1fc into elastic:master Oct 5, 2020
@chandlerprall chandlerprall deleted the eui-29-2-0 branch October 5, 2020 18:44
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 78870 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 Oct 7, 2020
@chandlerprall chandlerprall added the backport:skip This commit does not require backporting label Oct 7, 2020
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Oct 7, 2020
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 release_note:skip Skip the PR/issue when compiling release notes v7.10.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants