feat: add 4.5 contrast for text in partition slices #608

rshen91 · 2020-03-26T14:12:55Z

Summary

Closes #606
Iterated off some logic from elastic/kibana#60261 into the elastic-charts repo to calculate light or dark text for the partition and tree map charts.

Text color changes accounting for default 4.5 contrast within the pie or tree map slices or the link labels outside of the pie charts. If black or white text on whatever slice background can achieve the 4.5 contrast ratio, or user specified contrast ratio, then the text will be either black or white. Otherwise, the text will adjust grayscale to try and achieve the desired contrast ratio.
If textInvertible is set true, then the contrast of the text on the slices and the container background color is taken into account.
This PR also allows the user to provide the background color of the container so that contrast can be calculated effectively for any opacity/transparency of a pie slice.

New stories added:

Stylings - Partition Background: which has a pie chart with varying opacity values. Knobs are included to set textInvertible and textContrast to true or false. Additionally you can specify a number for textContrast to try and achieve.
Stylings - Partition Labels: showing the link labels adjusting in color to different background colors. There is a knob provided for the background to be set to show textColor changing in the labels and the text within the pie slices.

Main files with changes for reviewers:

src/chart_types/partition_chart/layout/utils/calcs.ts
src/chart_types/partition_chart/layout/viewmodel/fill_text_layout.ts
src/chart_types/partition_chart/layout/viewmodel/link_text_layout.ts
There is also a Google sheet with color and contrast with black text vs. white text worth checking out to justify why some of the stories changed.

Checklist

Delete any items that are not applicable to this PR.

This was checked for cross-browser compatibility, including a check against IE11
Proper documentation or storybook story was added for features that require explanation or tutorials
Unit tests were updated or added to match the most common scenarios

codecov-io · 2020-03-26T14:25:17Z

Codecov Report

Merging #608 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #608   +/-   ##
=======================================
  Coverage   72.77%   72.77%           
=======================================
  Files         266      266           
  Lines        8716     8716           
  Branches     1645     1645           
=======================================
  Hits         6343     6343           
  Misses       2330     2330           
  Partials       43       43

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0a24a5e...0a24a5e. Read the comment docs.

rshen91 · 2020-04-01T22:26:39Z

@cchaos when you have the time, I'd love to hear your thoughts and make sure I'm interpreting this correctly:
This PR is meant to address the earlier conversation here about taking into account relative luminosity: https://elastic.slack.com/archives/CFK9BJ248/p1584979319002100

Currently, this PR will take into account contrast (which I believe is closely tied to luminosity) but in the sunburst heterogenous story (screenshot below) it looks like the lighter red partitions should be black text instead of white? Or am I misinterpreting and the white text on the rgba(228, 26, 28, 0.3) background is actually ok?

Is there a different term (like transparency?) that I should be considering in calculating the text color with a certain background color? Thanks for you help in advance!

cchaos · 2020-04-03T21:01:49Z

@rshen91 Yes you are correct. That white text on the transparent sections does not have high contrast which means that your calculations don't take transparency into account. Though the only way it'd be able to do so is if you also knew the background color of the container.

Opacity is a killer for trying to calculate contrast ratios. In EUI, we don't use transparency but "tint" instead which is when a color is blended with white. Or "shade" for blending with black.

But as user and consumer created object, you'll never truly know the background color unless you ask for it.

Also, it still doesn't look like the calculations are correct as even the inner-most slice, which I'd assume is not already transparent, fails at only 3.06

rshen91 · 2020-04-07T15:41:31Z

Issue #238 is tied to this issue in that if we get the background color in order to determine tint or shade for this PR, that same prop can be used to help solve issue #238 detect the background color and apply an inverse color or one color from a dark/light palette

rshen91 · 2020-04-14T16:44:08Z

This PR will be relevant for PR #629 where the background color can be used for the tooltip color icon

nickofthyme

One suggested change but otherwise looks great 🎉

Nice work, this makes the pie charts look that much better!

src/components/chart_background.tsx

- handle transparent string as color - use optional chaining for accessing theme.background.color - verify in IE11

src/chart_types/partition_chart/layout/utils/calcs.ts

nickofthyme · 2020-06-08T15:59:50Z

src/components/chart_background.tsx

@@ -34,18 +33,19 @@ export class ChartBackgroundComponent extends React.Component<ChartBackgroundPro
 }

 render() {
- return <div className="echChartBackground" style={{ background: this.props?.backgroundStyles?.color }} />;
+ const { background } = this.props;
+ return <div className="echChartBackground" style={{ background }} />;


This could allow non-colors to be passed to the background. Is this a good idea?

body { background: lightblue url("img_tree.gif") no-repeat fixed center; }

changed here 9c91e74

nickofthyme · 2020-06-08T16:00:59Z

src/components/chart_background.tsx

 }
 }

 const mapStateToProps = (state: GlobalChartState): ChartBackgroundProps => {
 if (!getInternalIsInitializedSelector(state)) {
 return {
- backgroundStyles: getChartThemeSelector(state).background,
+ background: 'transparent',


# [19.5.0](v19.4.1...v19.5.0) (2020-06-15) ### Bug Fixes * **tooltip:** show true opaque colors in tooltips ([#629](#629)) ([23290be](23290be)), closes [#628](#628) * path of stacked area series with missing values ([#703](#703)) ([2541180](2541180)) * remove double rendering ([#693](#693)) ([ebf2748](ebf2748)), closes [#690](#690) ### Features * **partition:** add 4.5 contrast for text in partition slices ([#608](#608)) ([eded2ac](eded2ac)), closes [#606](#606) * add screenshot functions to partition/goal ([#697](#697)) ([5581c3c](5581c3c))

markov00 · 2020-06-15T07:20:50Z

🎉 This PR is included in version 19.5.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

rshen91 changed the title ~~feat: add luminance value calculation for text in partition slices~~ feat: add luminosity value calculation for text in partition slices Mar 26, 2020

rshen91 changed the title ~~feat: add luminosity value calculation for text in partition slices~~ feat: add 4.5 contrast for text in partition slices Apr 1, 2020

nickofthyme mentioned this pull request Apr 14, 2020

fix(tooltip): show true opaque colors in tooltips #629

Merged

rshen91 added 6 commits April 15, 2020 08:18

feat: move luminance value code from kibana in charts

02f2f53

feat: textColor takes into account contrast with background slice

325b5bd

feat: account for initial non black or white textColor

6acca30

feat: add background color knob for xy charts

8f4ddf5

test: fix backgrounds to white for vrt

23eb310

fix: add color background functionality to partition chart simple

d4b43ca

rshen91 force-pushed the luminance-pie branch from ee06126 to cca44ab Compare April 15, 2020 14:27

rshen91 added 3 commits April 15, 2020 08:41

docs: add story for partition in styling

361200e

feat: add combine color helper function

963572b

Merge remote-tracking branch 'upstream/master' into luminance-pie

01d1e2e

rshen91 force-pushed the luminance-pie branch from cca44ab to 01d1e2e Compare April 15, 2020 18:18

rshen91 added 10 commits April 16, 2020 08:48

Merge remote-tracking branch 'upstream/master' into luminance-pie

c7eec32

feat: add background to geometries

c36c79a

fix: improve contrast

fe5be74

Merge remote-tracking branch 'upstream/master' into luminance-pie

cd31813

fix: add updated story file

38e1250

Merge remote-tracking branch 'upstream/master' into luminance-pie

69168b6

feat: update colorIsDark to adjust text for contrast

c1bc892

Merge remote-tracking branch 'upstream/master' into luminance-pie

7c3c7b0

fix: readd textInvertible functionality in fill_text_layout

69a2cc0

Merge remote-tracking branch 'upstream/master' into luminance-pie

7059318

rshen91 added 4 commits May 28, 2020 11:58

Merge remote-tracking branch 'upstream/master' into luminance-pie

462debe

docs: add docs to storybook docs about textContrast

01b7052

Merge remote-tracking branch 'upstream/master' into luminance-pie

9bb1e8e

feat: update baseThemes

36baf2e

rshen91 requested review from markov00, nickofthyme and monfera June 3, 2020 19:22

nickofthyme approved these changes Jun 4, 2020

View reviewed changes

src/components/chart_background.tsx Outdated Show resolved Hide resolved

nickofthyme and others added 8 commits June 7, 2020 14:05

Merge branch 'master' into luminance-pie

1864be2

fix: update pr with remaining checks

f14f2fa

- handle transparent string as color - use optional chaining for accessing theme.background.color - verify in IE11

refactor: return types and rgb format

33aeb59

fix: get correct theme via selector and minor refactoring

979ffe0

fix: refactor chart size

af65d11

fix: cleanup theme colors and add color check

9012b54

fix: api docs

20a11c0

Merge branch 'master' into luminance-pie

44e5101

nickofthyme reviewed Jun 8, 2020

View reviewed changes

src/chart_types/partition_chart/layout/utils/calcs.ts Show resolved Hide resolved

nickofthyme reviewed Jun 8, 2020

View reviewed changes

fix: broken tests, update prop naming

9c91e74

nickofthyme merged commit eded2ac into elastic:master Jun 8, 2020

rshen91 deleted the luminance-pie branch June 10, 2020 15:13

markov00 added the released Issue released publicly label Jun 15, 2020

markov00 mentioned this pull request Jun 15, 2020

Update dependency @elastic/charts to v19.5.2 elastic/kibana#69126

Merged

1 task

nickofthyme mentioned this pull request Jun 23, 2020

Fix discover, tsvb and Lens chart theming issues elastic/kibana#69695

Merged

2 tasks

rshen91 mentioned this pull request Oct 12, 2020

Adjust the color contrast based on the background color #589

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add 4.5 contrast for text in partition slices #608

feat: add 4.5 contrast for text in partition slices #608

rshen91 commented Mar 26, 2020 •

edited

Loading

codecov-io commented Mar 26, 2020 •

edited

Loading

rshen91 commented Apr 1, 2020

cchaos commented Apr 3, 2020

rshen91 commented Apr 7, 2020

rshen91 commented Apr 14, 2020

nickofthyme left a comment

nickofthyme Jun 8, 2020 •

edited

Loading

nickofthyme Jun 8, 2020

nickofthyme Jun 8, 2020

markov00 commented Jun 15, 2020

feat: add 4.5 contrast for text in partition slices #608

feat: add 4.5 contrast for text in partition slices #608

Conversation

rshen91 commented Mar 26, 2020 • edited Loading

Summary

Checklist

codecov-io commented Mar 26, 2020 • edited Loading

Codecov Report

rshen91 commented Apr 1, 2020

cchaos commented Apr 3, 2020

rshen91 commented Apr 7, 2020

rshen91 commented Apr 14, 2020

nickofthyme left a comment

Choose a reason for hiding this comment

nickofthyme Jun 8, 2020 • edited Loading

Choose a reason for hiding this comment

nickofthyme Jun 8, 2020

Choose a reason for hiding this comment

nickofthyme Jun 8, 2020

Choose a reason for hiding this comment

markov00 commented Jun 15, 2020

rshen91 commented Mar 26, 2020 •

edited

Loading

codecov-io commented Mar 26, 2020 •

edited

Loading

nickofthyme Jun 8, 2020 •

edited

Loading