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.

Sign up for GitHub

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

Jump to bottom

Upgrade EUI to v40.0.0 #115639

Merged
merged 11 commits into from
Oct 25, 2021
Merged

Upgrade EUI to v40.0.0 #115639

merged 11 commits into from
Oct 25, 2021

Conversation

thompsongl
Copy link
Contributor

@thompsongl thompsongl commented Oct 19, 2021 •
edited
Loading

Summary

[email protected] ⏩ [email protected]

Breaking change not relevant to Kibana

40.0.0

  • Updated tokenKeyword to match the definition of keyword field type (#5251)
  • Added element, buttonElement, and arrowProps props to further customize EuiAccordion (#5258)
  • Fixed missing id for EuiCombobox by generating one if prepend or append exists (#5229)

Breaking changes

  • Renamed tokenKeyword icon to tokenTag in EuiToken (#5251)

@thompsongl thompsongl added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes EUI labels Oct 19, 2021
@thompsongl thompsongl changed the title Upgrae EUI to v40.0.0 Upgrade EUI to v40.0.0 Oct 19, 2021
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

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
core 132.4KB 132.4KB +1.0B
dataVisualizer 533.0KB 533.0KB -4.0B
enterpriseSearch 1.4MB 1.4MB -32.0B
fleet 619.6KB 619.6KB +1.0B
kibanaReact 217.3KB 217.3KB +1.0B
kibanaUtils 52.1KB 52.1KB +1.0B
security 507.8KB 507.8KB +1.0B
securitySolution 4.6MB 4.6MB +1.0B
total -30.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 597.0KB 596.8KB -251.0B
kbnUiSharedDeps-npmDll 5.0MB 5.0MB +348.0B
total +97.0B

History

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

thompsongl
thompsongl commented Oct 20, 2021
View reviewed changes
...se_search/public/applications/app_search/components/curations/components/curations_table.tsx
enlarge: true,
width: '100%',
truncateText: false,
Copy link
Contributor Author

@thompsongl thompsongl Oct 20, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Per https://elastic.github.io/eui/#/tabular-content/tables#responsive-tables truncateText has no effect without a custom render

Copy link
Contributor

Choose a reason for hiding this comment

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

Haha, I remember adding this comment. I wish I could approve the change now :trollface:

Copy link
Contributor

Choose a reason for hiding this comment

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

Although, question, shouldn't the non-mobile truncateText on line 73 up above also get removed? Or am I not understanding what you're saying?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

truncateText on L73 is valid. It's just not valid in the mobileOptions config.

Copy link
Contributor

Choose a reason for hiding this comment

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

Gotcha, thanks!

thompsongl
thompsongl commented Oct 20, 2021
View reviewed changes
...earch/public/applications/app_search/components/engines/components/tables/shared_columns.tsx
enlarge: true,
width: '100%',
truncateText: false,
Copy link
Contributor Author

@thompsongl thompsongl Oct 20, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Per https://elastic.github.io/eui/#/tabular-content/tables#responsive-tables truncateText has no effect without a custom render

@thompsongl thompsongl marked this pull request as ready for review October 20, 2021 17:54
@thompsongl thompsongl requested a review from a team October 20, 2021 17:54
@thompsongl thompsongl requested review from a team as code owners October 20, 2021 17:54
@thompsongl thompsongl requested a review from a team October 20, 2021 17:54
@thompsongl thompsongl requested review from a team as code owners October 20, 2021 17:54
@thompsongl thompsongl requested a review from a team October 20, 2021 17:54
@thompsongl thompsongl requested a review from a team as a code owner October 20, 2021 17:54
@elasticmachine
Copy link
Contributor

Pinging @elastic/eui-design (EUI)

@botelastic botelastic bot added the Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability label Oct 20, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/uptime (Team:uptime)

nreese
nreese approved these changes Oct 20, 2021
View reviewed changes
Copy link
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

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

maps changes LGTM

spong
spong approved these changes Oct 20, 2021
View reviewed changes
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 Solution changes LGTM! Just a few snapshot updates -- we're over the hill now, thanks @thompsongl! 🙂

estermv
estermv approved these changes Oct 21, 2021
View reviewed changes
Copy link
Contributor

@estermv estermv left a comment

Choose a reason for hiding this comment

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

Stack Monitoring changes LGTM

justinkambic
justinkambic approved these changes Oct 21, 2021
View reviewed changes
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.

Uptime changes LGTM.

@qn895
Copy link
Member

qn895 commented Oct 21, 2021

ML changes LGTM 🎉

@thompsongl
Copy link
Contributor Author

@elastic/app-search-frontend review is for non-snapshot code.

@elastic/kibana-app-services, @elastic/kibana-presentation, @elastic/kibana-reporting-services, @elastic/kibana-vis-editors reviews are for snapshot changes only.

I will plan to merge EOD to allow time before FF

flash1293
flash1293 approved these changes Oct 25, 2021
View reviewed changes
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, simple snapshot updates, code review only

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

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
core 132.4KB 132.4KB +1.0B
enterpriseSearch 1.4MB 1.4MB -32.0B
fleet 621.3KB 621.3KB +1.0B
kibanaReact 217.3KB 217.3KB +1.0B
kibanaUtils 52.1KB 52.1KB +1.0B
security 507.8KB 507.8KB +1.0B
securitySolution 4.6MB 4.6MB +1.0B
total -26.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 597.0KB 596.8KB -251.0B
kbnUiSharedDeps-npmDll 5.0MB 5.0MB +348.0B
total +97.0B

History

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

@thompsongl thompsongl merged commit 59815a0 into elastic:master Oct 25, 2021
@dgieselaar dgieselaar mentioned this pull request Oct 27, 2021
Closed
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 115639 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 27, 2021
@kibanamachine
Copy link
Contributor

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

2 similar comments
@kibanamachine
Copy link
Contributor

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

@kibanamachine
Copy link
Contributor

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

@thompsongl thompsongl added the backport:skip This commit does not require backporting label Nov 1, 2021
@spalger spalger added backport:skip This commit does not require backporting and removed backport:skip This commit does not require backporting labels Nov 1, 2021
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Nov 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cee-chen cee-chen cee-chen left review comments

@qn895 qn895 qn895 approved these changes

@justinkambic justinkambic justinkambic approved these changes

@jbudz jbudz jbudz approved these changes

@spong spong spong approved these changes

@estermv estermv estermv approved these changes

@lukeelmers lukeelmers lukeelmers approved these changes

@flash1293 flash1293 flash1293 approved these changes

@JasonStoltz JasonStoltz JasonStoltz approved these changes

@nreese nreese nreese approved these changes

Assignees
No one assigned
Labels
backport:skip This commit does not require backporting EUI release_note:skip Skip the PR/issue when compiling release notes Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.