feat(color): color consistency enhancements #21507

stephenLYZ · 2022-09-18T15:56:44Z

SUMMARY

In the current world, we have this problem:
• SharedLabelColors are calculated when the dashboard loads, but only for the visible tab(s)
• SharedLabelColors are only "reserved" if the label is used twice or more in the VISIBLE charts at load time.
• This means if a label is used once at load time, and then used by another chart when a hidden tab is loaded, the color will be inconsistent.

Current solution by @rusackas and @geido :
• When loading the initial dashboard, make ALL labels use reserved colors... not just those that are used more than once. In other words, assume that ALL colors will be shared/reused.
• Then, when you change tabs, any labels that are used the second time (or more) will already be in the SharedLabelColors
• When loading/exposing a hidden tab, you might see a new series/label for the first time. These should also be added to SharedLabelColors, in case a third or subsequent tab uses those.

Module Diagrams:

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

Has associated issue:
Required feature flags:
Changes UI
Includes DB Migration (follow approval process in SIP-59)
- Migration is atomic, supports rollback & is backwards-compatible
- Confirm DB migration upgrade and downgrade tested
- Runtime estimates and downtime expectations provided
Introduces new feature or API
Removes existing feature or API

geido · 2022-09-19T11:38:39Z

/testenv up

github-actions · 2022-09-19T11:40:54Z

@geido Ephemeral environment spinning up at http://35.85.63.230:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

geido · 2022-09-21T11:30:51Z

@stephenLYZ thanks for this!

I tested it and I found that when setting up custom label colors, they only work for the first tab now, but not the other one. Have a look at the screenshots. I forced Michael to be yellow in the metadata

There is a similar problem with removing a custom label color. When you remove it, only the charts that are currently visible in the current tab will get updated. If you switch tab the charts will keep the custom color label as if it wasn't deleted.

stephenLYZ · 2022-09-21T11:34:38Z

@stephenLYZ thanks for this!

I tested it and I found that when setting up custom label colors, they only work for the first tab now, but not the other one. Have a look at the screenshots. I forced Michael to be yellow in the metadata

Good catch! This is a case that I have missed, I only dealt with the case of changing the color scheme.

codecov · 2022-09-21T17:31:59Z

Codecov Report

Merging #21507 (d20665a) into master (8f61e3c) will decrease coverage by 0.01%.
The diff coverage is 76.19%.

@@            Coverage Diff             @@
##           master   #21507      +/-   ##
==========================================
- Coverage   66.86%   66.84%   -0.02%     
==========================================
  Files        1803     1805       +2     
  Lines       68996    69052      +56     
  Branches     7349     7369      +20     
==========================================
+ Hits        46132    46158      +26     
- Misses      20971    20996      +25     
- Partials     1893     1898       +5

Flag	Coverage Δ
hive	`52.92% <ø> (-0.01%)`	⬇️
javascript	`53.19% <76.19%> (-0.01%)`	⬇️
mysql	`78.24% <ø> (-0.01%)`	⬇️
postgres	`78.30% <ø> (-0.01%)`	⬇️
presto	`52.82% <ø> (-0.01%)`	⬇️
python	`81.45% <ø> (-0.01%)`	⬇️
sqlite	`76.79% <ø> (-0.01%)`	⬇️
unit	`51.06% <ø> (+0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...ntend/packages/superset-ui-core/src/color/index.ts	`100.00% <ø> (ø)`
...t-frontend/src/dashboard/actions/dashboardState.js	`37.09% <ø> (+0.04%)`	⬆️
.../src/dashboard/components/gridComponents/Chart.jsx	`53.57% <0.00%> (-1.48%)`	⬇️
...ashboard/components/gridComponents/ChartHolder.tsx	`91.30% <ø> (+2.41%)`	⬆️
...nd/src/dashboard/containers/DashboardComponent.jsx	`85.00% <ø> (ø)`
...ntend/src/dashboard/containers/DashboardHeader.jsx	`66.66% <ø> (ø)`
...rontend/src/dashboard/containers/DashboardPage.tsx	`7.14% <0.00%> (-0.12%)`	⬇️
...-frontend/src/dashboard/reducers/dashboardState.js	`76.08% <ø> (-0.51%)`	⬇️
superset-frontend/src/explore/ExplorePage.tsx	`0.00% <0.00%> (ø)`
superset/config.py	`91.63% <ø> (ø)`
... and 25 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

rusackas · 2022-09-23T04:44:47Z

/testenv up

github-actions · 2022-09-23T04:49:46Z

@rusackas Ephemeral environment spinning up at http://35.160.87.160:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

geido · 2022-09-27T12:47:38Z

@stephenLYZ as I work on the E2E I report here any issue that I encounter.

I just found an issue with already rendered charts under a hidden tab. If you first visit the tab and let the chart load, then move away to another tab, add a custom label color and go back to the tab with the chart, you will see that the custom label color isn't applied.

Tabbed.Dashboard.mp4

Same thing happens with changing the color scheme

Tabbed.Dashboard.1.mp4

geido · 2022-09-27T13:51:10Z

@stephenLYZ the above happens also for main tabs. If you go to tab B, then go to tab A, change scheme, go back to tab B and you will see the color scheme isn't applied.

I also found an even weirder bug where if you change the color scheme from tab B, it picks different colors than if you did change it from tab A.

Tabbed.Dashboard.2.mp4

geido · 2022-09-28T13:03:12Z

@stephenLYZ not sure if this is all about the same issue, but when changing color scheme, it works for a nested tab in the initial dashboard tab, but it is not applied correctly to the second tab. As you can see the label colors for "Anthony" are different

Tabbed.Dashboard.3.mp4

stephenLYZ · 2022-09-30T07:25:17Z

@geido It looks like I can't reproduce in my local. I actually fixed this issue in the 'Chart.jsx' file:

2022-09-30.3.11.25.mov

geido · 2022-09-30T14:22:11Z

@geido It looks like I can't reproduce in my local. I actually fixed this issue in the 'Chart.jsx' file:

2022-09-30.3.11.25.mov

@stephenLYZ I can reproduce this pretty consistently with the "Tabbed Dashboard" that is available as a sample dashboard in the E2E testing env. I suggest that you try your changes using my E2E tests PR #21622 where the above issues should be apparent immediately.

geido

All tests are passing now!

geido · 2022-10-11T12:35:40Z

Adding @jinghua-qa for an upmost confirmation from QA. Let's hold for her feedback before merging

jinghua-qa · 2022-10-12T18:40:17Z

/testenv up

github-actions · 2022-10-12T21:12:34Z

@jinghua-qa Ephemeral environment spinning up at http://54.200.28.129:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

jinghua-qa · 2022-10-13T07:03:51Z

I found an issue that if a series in different charts in the same dashboard, then it is not consistent for the color on that series
here how i reproduce this issue:
step 1: Run this sql query and create dataset:
SELECT * FROM
(SELECT 'a' as color, 1 as val
UNION
SELECT 'b', 1
UNION
SELECT 'c', 1
UNION
SELECT 'd', 1
UNION
SELECT 'e', 1
UNION
SELECT 'f', 1
UNION
SELECT 'g', 1
UNION
SELECT 'h', 1
UNION
SELECT 'i', 1
UNION
SELECT 'j', 1
UNION
SELECT 'k', 1
UNION
SELECT 'l', 1
UNION
SELECT 'm', 1
UNION
SELECT 'n', 1
UNION
SELECT 'o', 1
UNION
SELECT 'p', 1
UNION
SELECT 'q', 1
UNION
SELECT 'r', 1
UNION
SELECT 's', 1
UNION
SELECT 't', 1
UNION
SELECT 'u', 1
UNION
SELECT 'v', 1
UNION
SELECT 'w', 1
UNION
SELECT 'x', 1
UNION
SELECT 'y', 1
UNION
SELECT 'z', 1
) as _view
ORDER BY COLOR
step2: create 3 charts using the same dataset:
chart 1: metrics: count(), dimension: color
chart 2:metrics: count(), dimension: color, filter: color in a, b, c
chart 3:metrics: count(*), dimension: color, filter: color in d, e, f
add 3 charts to same dashboard
step 3: create native filter for color name, then apply filter with color name a, e

Expect:
chart 1: a, e show different color
chart 2, a show same color with chart 1, series a
chart 3, e show same color with chart 1 series ec

Actual:
chart 2 series a show same color as series e

…Z/superset into feat-color-enhancement

stephenLYZ · 2022-10-13T07:46:51Z

@jinghua-qa Thanks for reporting this issue. This is a bug once we enter the dashboard from explore. I have fixed it and maybe we can add some e2e tests for the native filter scene in the future. cc @geido

stephenLYZ · 2022-10-13T10:26:02Z

/testenv up

github-actions · 2022-10-13T10:30:32Z

@stephenLYZ Ephemeral environment spinning up at http://35.89.74.57:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

geido

I am sorry but I found another issue :(

If you look at the video you will see that the label "Daniel" changes color when you set a custom label color "Anthony". I am not sure why this happens but it is causing all sorts of weird behaviours when removing custom label colors. The colors should be deterministic. If you add a custom label color, it should not affect the colors of all the other labels, but only that of the target label.

Tabbed.Dashboard.1.mp4

stephenLYZ · 2022-10-14T04:17:06Z

@geido The problem may have existed before. I will fix this in this PR.

stephenLYZ · 2022-10-14T06:47:45Z

/testenv up

github-actions · 2022-10-14T06:52:55Z

@stephenLYZ Ephemeral environment spinning up at http://52.36.247.190:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

geido

Tentatively re-approving as all new test scenarios are passing. @jinghua-qa another look from your side would be very appreciated!

jinghua-qa

LGTM

github-actions · 2022-10-17T10:12:29Z

Ephemeral environment shutdown and build artifacts deleted.

pull-request-size bot added the size/L label Sep 18, 2022

geido self-requested a review September 21, 2022 11:22

stephenLYZ force-pushed the feat-color-enhancement branch from 8f49450 to 831676d Compare September 21, 2022 14:23

geido mentioned this pull request Sep 28, 2022

chore: Color consistency E2E tests #21622

Merged

9 tasks

stephenLYZ requested review from craig-rueda, dpgaspar, villebro, michael-s-molina, ktmud and a team as code owners October 3, 2022 16:37

pull-request-size bot added size/XXL and removed size/L labels Oct 3, 2022

stephenLYZ force-pushed the feat-color-enhancement branch from 0a35a5f to df73632 Compare October 3, 2022 16:39

pull-request-size bot added size/L and removed size/XXL labels Oct 3, 2022

stephenLYZ removed request for a team, ktmud and craig-rueda October 3, 2022 16:40

stephenLYZ added 3 commits October 10, 2022 14:02

fix: test

37e8e80

fix: test

a9bacb5

fix

6db4c30

geido approved these changes Oct 11, 2022

View reviewed changes

geido requested a review from jinghua-qa October 11, 2022 12:35

Merge branch 'master' into feat-color-enhancement

6b7b942

stephenLYZ added 2 commits October 13, 2022 15:41

fix: color source

c9c851c

Merge branch 'feat-color-enhancement' of https://github.com/stephenLY…

5a2081f

…Z/superset into feat-color-enhancement

geido added the need:qa-review Requires QA review label Oct 13, 2022

geido requested changes Oct 13, 2022

View reviewed changes

fix label color will change color order

d20665a

geido approved these changes Oct 14, 2022

View reviewed changes

jinghua-qa approved these changes Oct 17, 2022

View reviewed changes

stephenLYZ merged commit 7a7181a into apache:master Oct 17, 2022

mistercrunch added the 🚢 2.1.3 label Feb 18, 2024

mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 2.1.0 and removed 🚢 2.1.3 labels Mar 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(color): color consistency enhancements #21507

feat(color): color consistency enhancements #21507

stephenLYZ commented Sep 18, 2022 •

edited

Loading

geido commented Sep 19, 2022

github-actions bot commented Sep 19, 2022

geido commented Sep 21, 2022 •

edited

Loading

stephenLYZ commented Sep 21, 2022

codecov bot commented Sep 21, 2022 •

edited

Loading

rusackas commented Sep 23, 2022

github-actions bot commented Sep 23, 2022

geido commented Sep 27, 2022

geido commented Sep 27, 2022 •

edited

Loading

geido commented Sep 28, 2022

stephenLYZ commented Sep 30, 2022

geido commented Sep 30, 2022

geido left a comment

geido commented Oct 11, 2022

jinghua-qa commented Oct 12, 2022

github-actions bot commented Oct 12, 2022

jinghua-qa commented Oct 13, 2022

stephenLYZ commented Oct 13, 2022

stephenLYZ commented Oct 13, 2022

github-actions bot commented Oct 13, 2022

geido left a comment •

edited

Loading

stephenLYZ commented Oct 14, 2022 •

edited

Loading

stephenLYZ commented Oct 14, 2022

github-actions bot commented Oct 14, 2022

geido left a comment

jinghua-qa left a comment

github-actions bot commented Oct 17, 2022

feat(color): color consistency enhancements #21507

feat(color): color consistency enhancements #21507

Conversation

stephenLYZ commented Sep 18, 2022 • edited Loading

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

geido commented Sep 19, 2022

github-actions bot commented Sep 19, 2022

geido commented Sep 21, 2022 • edited Loading

stephenLYZ commented Sep 21, 2022

codecov bot commented Sep 21, 2022 • edited Loading

Codecov Report

rusackas commented Sep 23, 2022

github-actions bot commented Sep 23, 2022

geido commented Sep 27, 2022

geido commented Sep 27, 2022 • edited Loading

geido commented Sep 28, 2022

stephenLYZ commented Sep 30, 2022

geido commented Sep 30, 2022

geido left a comment

Choose a reason for hiding this comment

geido commented Oct 11, 2022

jinghua-qa commented Oct 12, 2022

github-actions bot commented Oct 12, 2022

jinghua-qa commented Oct 13, 2022

stephenLYZ commented Oct 13, 2022

stephenLYZ commented Oct 13, 2022

github-actions bot commented Oct 13, 2022

geido left a comment • edited Loading

Choose a reason for hiding this comment

stephenLYZ commented Oct 14, 2022 • edited Loading

stephenLYZ commented Oct 14, 2022

github-actions bot commented Oct 14, 2022

geido left a comment

Choose a reason for hiding this comment

jinghua-qa left a comment

Choose a reason for hiding this comment

github-actions bot commented Oct 17, 2022

stephenLYZ commented Sep 18, 2022 •

edited

Loading

geido commented Sep 21, 2022 •

edited

Loading

codecov bot commented Sep 21, 2022 •

edited

Loading

geido commented Sep 27, 2022 •

edited

Loading

geido left a comment •

edited

Loading

stephenLYZ commented Oct 14, 2022 •

edited

Loading