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

[Charts] Allow partition label colors and fix stacked polarity bug #120271

Merged
merged 3 commits into from
Dec 13, 2021
Merged

[Charts] Allow partition label colors and fix stacked polarity bug #120271

merged 3 commits into from
Dec 13, 2021

Conversation

nickofthyme
Copy link
Contributor

@nickofthyme nickofthyme commented Dec 2, 2021 •
edited
Loading

Summary

Upgrades charts to latest patch version v40.0.1. This includes a fix for using textColor on linkLabels and fillLabels with partition charts. Previously, this value was ignored in favor of color contrast logic. Default value is ColorVariant.Adaptive which applied color contrast logic. See elastic/elastic-charts#1498

This will facilitate #116777, but does not yet fix it.

If you apply a color string value, no contrast logic will be performed.

TextColor fix

Screen Shot 2021-12-09 at 5 40 42 PM

Screen Shot 2021-12-09 at 5 43 50 PM

Negative and positive stacked fix

Screen Shot 2021-12-09 at 5 35 41 PM

Screen Shot 2021-12-09 at 5 36 30 PM

Screen Shot 2021-12-09 at 5 36 47 PM

With stacked offset

ezgif.com-gif-maker.mp4

This fix will be applied to 8.1 in a future version of elastic-charts.

@nickofthyme nickofthyme added Feature:ElasticCharts Issues related to the elastic-charts library v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v8.1.0 labels Dec 2, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/datavis (Feature:ElasticCharts)

@nickofthyme nickofthyme changed the title upgrade charts to fix partition label colors [Charts] Upgrade charts to fix partition label colors Dec 2, 2021
@nickofthyme nickofthyme added backport:skip This commit does not require backporting and removed v8.1.0 labels Dec 2, 2021
@nickofthyme nickofthyme mentioned this pull request Dec 9, 2021
Closed
@nickofthyme nickofthyme linked an issue Dec 9, 2021 that may be closed by this pull request
[Lens] Stacked problem with negative values #118066
Closed
@nickofthyme nickofthyme changed the title [Charts] Upgrade charts to fix partition label colors [Charts] fix partition label colors and stacked polarity bug Dec 9, 2021
@nickofthyme nickofthyme changed the title [Charts] fix partition label colors and stacked polarity bug [Charts] Allow partition label colors and fix stacked polarity bug Dec 9, 2021
@nickofthyme nickofthyme force-pushed the fix-partition-chart-label-color branch from c51bde1 to 8937116 Compare December 10, 2021 15:23
@nickofthyme nickofthyme linked an issue Dec 10, 2021 that may be closed by this pull request
[Lens] Pie/Treemap chart external labels are hard to read when in dark mode #116777
Closed
@nickofthyme
Copy link
Contributor Author

@dej611 & @flash, could you confirm this fixes the 2 linked issues? Thanks

@nickofthyme
Copy link
Contributor Author

@elasticmachine merge upstream

flash1293
flash1293 approved these changes Dec 13, 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.

Pie chart looks good in dark mode and bar charts render well with negative values - LGTM!

@nickofthyme
Copy link
Contributor Author

@elasticmachine merge upstream

@nickofthyme nickofthyme enabled auto-merge (squash) December 13, 2021 15:45
@nickofthyme nickofthyme merged commit f314251 into elastic:8.0 Dec 13, 2021
@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
visTypeTimeseries 447.3KB 447.3KB -1.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-npmDll 5.0MB 5.0MB +2.5KB

History

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

@nickofthyme nickofthyme deleted the fix-partition-chart-label-color branch December 13, 2021 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@flash1293 flash1293 flash1293 approved these changes

@dej611 dej611 Awaiting requested review from dej611

Assignees
No one assigned
Labels
backport:skip This commit does not require backporting Feature:ElasticCharts Issues related to the elastic-charts library release_note:skip Skip the PR/issue when compiling release notes v8.0.0
Projects
None yet
Milestone
No milestone
5 participants
@nickofthyme @elasticmachine @kibana-ci @flash1293 @kibanamachine