fix(tooltip): show true opaque colors in tooltips by nickofthyme · Pull Request #629 · elastic/elastic-charts

nickofthyme · 2020-04-11T00:12:12Z

Summary

Add backgroundColor to tooltip colors to show true color. Uses background color from theme.background.color per #608.

Most noticeable with light colors.

fixes #628

Before

After

codecov-io · 2020-04-11T00:28:51Z

Codecov Report

Merging #629 into master will increase coverage by 2.84%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #629      +/-   ##
==========================================
+ Coverage   67.78%   70.63%   +2.84%     
==========================================
  Files         236      220      -16     
  Lines        6914     6439     -475     
  Branches     1312     1237      -75     
==========================================
- Hits         4687     4548     -139     
+ Misses       2208     1872     -336     
  Partials       19       19

Impacted Files	Coverage Δ
src/components/tooltip/index.tsx	`71.42% <ø> (ø)`
...chart_types/xy_chart/utils/stacked_series_utils.ts	`97.80% <100.00%> (ø)`
...hart_types/partition_chart/layout/config/config.ts	`67.74% <0.00%> (-1.01%)`	⬇️
src/index.ts	`100.00% <0.00%> (ø)`
.../chart_types/goal_chart/state/selectors/tooltip.ts
...art_types/goal_chart/layout/viewmodel/viewmodel.ts
...t_types/goal_chart/layout/types/viewmodel_types.ts
...s/goal_chart/state/selectors/is_tooltip_visible.ts
src/chart_types/goal_chart/specs/index.ts
...art_types/goal_chart/state/selectors/geometries.ts
... and 11 more

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 de7df75...9586b0f. Read the comment docs.

rshen91

I opened the stories locally and it rendered correctly for the vast majority. I did notice in there might be some weirdness with treemap stories with the tooltip that I could use clarification on. For instance, Treemap - Percentage story that the tooltips show the continents with a $euiColorGhost fill vs. no fill. I'm not sure if this is an intended change? The Treemap- Multi Color shows something similar.

The Treemap - Two Layers Stress Test shows the tooltip item color next to the tooltip text as a different size than the other layer in the tooltip. In master it looks like these are the same size. I'm including a giph to explain it better than I can hopefully:

markov00

We should adapt the background color with the used theme background color

markov00 · 2020-04-14T14:51:08Z

src/components/tooltip/_tooltip.scss

This color will be always #fff independently of the background color. I think, for a first fix, we should use a EUI theme color dependant, like the $euiColorFullShade that is white with the light theme and near black for the dark theme.

@rshen91 is working on adding a background prop to the chart to handle various background color, this will then needs to be connected with this component to pass the right color if specified

Ok good idea. I'll wait on #608

see 58cf176

nickofthyme · 2020-04-14T17:34:29Z

Good eye @rshen91 I'll test that case when I merge with your changes. 👍

codecov-commenter · 2020-06-08T22:08:24Z

Codecov Report

Merging #629 into master will increase coverage by 0.14%.
The diff coverage is 79.44%.

@@            Coverage Diff             @@
##           master     #629      +/-   ##
==========================================
+ Coverage   73.13%   73.27%   +0.14%     
==========================================
  Files         271      271              
  Lines        8784     8786       +2     
  Branches     1747     1747              
==========================================
+ Hits         6424     6438      +14     
+ Misses       2309     2297      -12     
  Partials       51       51

Impacted Files	Coverage Δ
...art_types/goal_chart/layout/viewmodel/viewmodel.ts	`3.12% <0.00%> (ø)`
.../chart_types/goal_chart/state/selectors/tooltip.ts	`41.66% <ø> (ø)`
src/chart_types/index.ts	`100.00% <ø> (ø)`
..._types/partition_chart/layout/circline_geometry.ts	`98.43% <0.00%> (ø)`
src/chart_types/partition_chart/layout/geometry.ts	`77.77% <ø> (ø)`
...types/partition_chart/layout/types/config_types.ts	`100.00% <ø> (ø)`
...es/partition_chart/layout/types/viewmodel_types.ts	`83.33% <ø> (ø)`
...c/chart_types/partition_chart/layout/utils/math.ts	`100.00% <ø> (ø)`
...hart_types/partition_chart/layout/utils/measure.ts	`100.00% <ø> (ø)`
...tion_chart/layout/viewmodel/hierarchy_of_arrays.ts	`88.88% <ø> (ø)`
... and 340 more

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 0a91351...b0e6eb9. Read the comment docs.

markov00

LGTM, tested on safari, chrome and firefox

markov00

Testing locally on playground looks good, but I've taken a deeper look at the CI errors and all of them seems legit errors:
There are two errors that are significant:

the sunburst tooltip shows the old color version:
in this other screenshot I can see that values are not pushed to the edge

nickofthyme · 2020-06-09T13:24:23Z

@markov00!!! thanks, good catch! The first issue I think is expected, what do you mean by the sunburst tooltip shows the old color version? The color is the only diff in that png.

The second one I will fix now.

markov00 · 2020-06-09T15:49:52Z

@monfera thanks, good catch! The first issue I think is expected, what do you mean by the sunburst tooltip shows the old color version? The color is the only diff in that png.

The second one I will fix now.

The review was mine :D
by the sunburst tooltip shows the old color version I mean: this error was expected, the screenshot diff shows that we forgot to update that screenshot

nickofthyme · 2020-06-09T20:02:31Z

Sorry Marco, I don't know why I thought that was Robert. Gotcha, thanks I'll update both bullets.

- Add flex value to inner item container - Update screenshots per color changes

nickofthyme · 2020-06-09T21:32:16Z

@markov00 Fixed both in b0e6eb9. I verified the tooltip style changes in IE11, safari, firefox and chrome, but feel free to check yourself.

markov00

LGTM

# [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:38Z

🎉 This PR is included in version 19.5.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

nickofthyme added :styling Styling related issue :tooltip Related to hover tooltip labels Apr 11, 2020

nickofthyme requested review from markov00 and rshen91 April 11, 2020 00:12

rshen91 reviewed Apr 13, 2020

View reviewed changes

markov00 requested changes Apr 14, 2020

View reviewed changes

rshen91 mentioned this pull request Apr 14, 2020

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

Merged

3 tasks

markov00 added the wip work in progress label Apr 20, 2020

markov00 marked this pull request as draft April 20, 2020 09:16

nickofthyme added 2 commits June 8, 2020 17:03

fix(tooltip): opac color in tooltip to match chart

58cf176

fix: linting to error on commit

9cd8c89

nickofthyme force-pushed the fix/tooltip-colors branch from 9586b0f to 9cd8c89 Compare June 8, 2020 22:08

nickofthyme marked this pull request as ready for review June 8, 2020 22:09

nickofthyme requested review from markov00 and rshen91 June 8, 2020 22:09

markov00 approved these changes Jun 9, 2020

View reviewed changes

markov00 self-requested a review June 9, 2020 08:18

markov00 requested changes Jun 9, 2020

View reviewed changes

fix: regression in styles

b0e6eb9

- Add flex value to inner item container - Update screenshots per color changes

nickofthyme requested review from markov00 and monfera June 9, 2020 21:32

markov00 approved these changes Jun 10, 2020

View reviewed changes

markov00 removed the wip work in progress label Jun 10, 2020

nickofthyme merged commit 23290be into elastic:master Jun 10, 2020

nickofthyme deleted the fix/tooltip-colors branch June 10, 2020 01:37

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 16, 2020

fix: popper scroll issue #712

Merged

Conversation

nickofthyme commented Apr 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Before

After

Uh oh!

codecov-io commented Apr 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

rshen91 left a comment

Choose a reason for hiding this comment

Uh oh!

markov00 left a comment

Choose a reason for hiding this comment

Uh oh!

markov00 Apr 14, 2020

Choose a reason for hiding this comment

Uh oh!

nickofthyme Apr 14, 2020

Choose a reason for hiding this comment

Uh oh!

nickofthyme Jun 8, 2020

Choose a reason for hiding this comment

Uh oh!

nickofthyme commented Apr 14, 2020

Uh oh!

codecov-commenter commented Jun 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

markov00 left a comment

Choose a reason for hiding this comment

Uh oh!

markov00 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nickofthyme commented Jun 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

markov00 commented Jun 9, 2020

Uh oh!

nickofthyme commented Jun 9, 2020

Uh oh!

nickofthyme commented Jun 9, 2020

Uh oh!

markov00 left a comment

Choose a reason for hiding this comment

Uh oh!

markov00 commented Jun 15, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

nickofthyme commented Apr 11, 2020 •

edited

Loading

codecov-io commented Apr 11, 2020 •

edited

Loading

codecov-commenter commented Jun 8, 2020 •

edited

Loading

markov00 left a comment •

edited

Loading

nickofthyme commented Jun 9, 2020 •

edited

Loading