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

[VisTypePie] Use a different advanced setting for pie charts #103049

Merged
merged 3 commits into from
Jun 24, 2021

Conversation

stratoula
Copy link
Contributor

@stratoula stratoula commented Jun 23, 2021

Summary

This PR adds a separate advanced setting for the pie charts legacy library. Until now they had the same switch but this causes some problems with our Make it minor initiative.

Our plan is to remove the legacy charts incrementally. So, when the new XY charts are stable, we want to remove the legacy XY charts and the switch (but the pie chart will still have both implementations) etc.

image

Checklist

Delete any items that are not applicable to this PR.

@stratoula stratoula changed the title Different switch for pie [VisTypePie] Use a different advanced setting for pie charts Jun 23, 2021
@stratoula stratoula added Feature:Pie Chart Pie chart visualization feature release_note:enhancement Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.14.0 v8.0.0 labels Jun 23, 2021
@stratoula stratoula marked this pull request as ready for review June 23, 2021 11:40
@stratoula stratoula requested a review from a team as a code owner June 23, 2021 11:40
@stratoula stratoula requested a review from a team June 23, 2021 11:40
@stratoula stratoula requested review from a team as code owners June 23, 2021 11:40
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

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.

Presentation Team Changes LGTM

@flash1293
Copy link
Contributor

@elasticmachine merge upstream

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.

Tested all combinations and works as expected, LGTM

@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
visTypePie 74.6KB 74.6KB +46.0B
visTypeVislib 564.7KB 564.7KB +1.0B
visTypeXy 113.7KB 113.9KB +152.0B
total +199.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
visTypePie 20.1KB 20.2KB +74.0B
visTypeVislib 32.3KB 32.5KB +122.0B
visTypeXy 63.3KB 63.5KB +211.0B
visualizations 55.6KB 55.4KB -178.0B
total +229.0B

History

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

@stratoula stratoula merged commit 1d2ceba into elastic:master Jun 24, 2021
stratoula added a commit to stratoula/kibana that referenced this pull request Jun 24, 2021
…#103049)

* Different switch for pie

* Remove unused translations

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
stratoula added a commit that referenced this pull request Jun 24, 2021
#103222)

* Different switch for pie

* Remove unused translations

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Pie Chart Pie chart visualization feature release_note:enhancement Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.14.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants