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

[XY] Enables normal mode for percentage charts #119582

Merged
merged 9 commits into from
Dec 8, 2021
Merged

[XY] Enables normal mode for percentage charts #119582

merged 9 commits into from
Dec 8, 2021

Conversation

stratoula
Copy link
Contributor

@stratoula stratoula commented Nov 24, 2021 •
edited
Loading

Summary

Part of #118525 for 7.16.1
A new PR is going to be created for v8.0+ when the function will be released to v40.1.

This PR is using the computeRatioByGroups function of EC library to allow users creating unstacked percentage charts.
Until now, every time a user wanted to create a percentage chart, the mode was changing automatically to stacked. I removed this functionality and now we allow the users to create this kind of charts.

image

image

Checklist

@stratoula stratoula added backport:skip This commit does not require backporting Feature:XYAxis XY-Axis charts (bar, area, line) release_note:fix v7.16.1 labels Nov 24, 2021
@stratoula stratoula requested a review from markov00 November 24, 2021 10:46
@stratoula stratoula changed the title [XY] Enable normal mode for percentage charts [XY] Enables normal mode for percentage charts Nov 24, 2021
@stratoula stratoula added the Team:Visualizations Visualization editors, elastic-charts and infrastructure label Nov 25, 2021
@stratoula stratoula marked this pull request as ready for review November 25, 2021 06:58
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-vis-editors @elastic/kibana-vis-editors-external (Team:VisEditors)

@stratoula stratoula requested a review from a team November 25, 2021 10:56
@stratoula
Copy link
Contributor Author

@elasticmachine merge upstream

mbondyra
mbondyra approved these changes Nov 29, 2021
View reviewed changes
Copy link
Contributor

@mbondyra mbondyra left a comment

Choose a reason for hiding this comment

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

tested on Chrome, works great! Code LGTM

dej611
dej611 reviewed Nov 29, 2021
View reviewed changes
Copy link
Contributor

@dej611 dej611 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Noticed a regression here for stacked bar charts when having two metrics.

Previous behaviour:
Screenshot 2021-11-29 at 11 28 31

Behaviour in this PR:
Screenshot 2021-11-29 at 11 28 39

Edit:
Oh, nevermind, the second series was set to Normal while the first was Stacked. It is a bit confusing, but I guess it mapped the old behaviour. All good then 👍

@stratoula
Copy link
Contributor Author

@elasticmachine merge upstream

@stratoula stratoula mentioned this pull request Dec 2, 2021
Merged
1 task
@stratoula
Copy link
Contributor Author

@elasticmachine merge upstream

@nickofthyme nickofthyme mentioned this pull request Dec 2, 2021
Closed
@stratoula
Copy link
Contributor Author

This PR also includes this fix #118727

@stratoula
Copy link
Contributor Author

@elasticmachine merge upstream

@stratoula
Copy link
Contributor Author

@elasticmachine merge upstream

@stratoula
Copy link
Contributor Author

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

💚 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
visTypeXy 61.9KB 61.8KB -116.0B

Page load bundle

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

id before after diff
core 308.3KB 310.4KB +2.1KB
kbnUiSharedDeps-npmDll 5.0MB 5.0MB +4.3KB
total +6.4KB

History

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

@stratoula stratoula merged commit a75f472 into elastic:7.16 Dec 8, 2021
@stratoula stratoula mentioned this pull request Dec 8, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dej611 dej611 dej611 left review comments

@mbondyra mbondyra mbondyra approved these changes

@markov00 markov00 Awaiting requested review from markov00

Assignees
No one assigned
Labels
backport:skip This commit does not require backporting Feature:XYAxis XY-Axis charts (bar, area, line) release_note:fix Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.16.1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@stratoula @elasticmachine @kibana-ci @dej611 @mbondyra @kibanamachine