fix(Dashboard): Retain colors when color scheme not set #30646

geido · 2024-10-17T17:29:00Z

SUMMARY

This PR does the following:

It fixes an issue for which colors would sometimes not keep their consistency when leaving and revisiting the Dashboard.
It fixes an issue for which charts original color schemes were not tracked making falling back to their origin color scheme impossible when removing the color scheme from the Dashboard.
It attempts to fix and re-enable skipped E2E tests that are critical for the coverage of the color consistency implementation.
Fixes Cannot set color scheme for dashboard-associated chart even when dashboard color scheme is cleared #23874 with limitations:
- The color scheme cannot be changed if the chart shares labels with multiple charts in the Dashboard.
- When setting custom label colors via the advanced settings of the Dashboard, the custom label colors will always be enforced also when a color scheme is not set for the Dashboard.
- The chart should consistently display colors that match those used in the Dashboard.

BEFORE

Color.Consistency.Test.mp4

AFTER

Color.Consistency.Test.1.mp4

TESTING INSTRUCTIONS

Open a Dashboard with a color scheme and shared label colors
Edit the Dashboard properties and remove the color scheme
After applying the changes, verify that shared label colors are still respected
For charts that do not share labels, verify their original color scheme is respected
Save and exit the Dashboard by going to the Dashboard lists
Come back again to the same Dashboard you just saved
Make sure colors haven't changed from the previous state

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

hanslemm · 2024-10-18T14:17:20Z

Upvote for this. This problem is annoying.

rusackas · 2024-10-18T16:00:29Z

OMG thank you!

superset-frontend/cypress-base/cypress/e2e/dashboard/editmode.test.ts

rusackas

Thanks for fixing, LGTM!

geido · 2024-10-18T16:24:18Z

I will try to tackle an issue for which after deleting a color scheme from a Dashboard and then editing the Chart from the Dashboard, Explore won't allow to customize the color.

fisjac

LGTM

…board

superset-frontend/src/explore/components/controls/ColorSchemeControl/index.tsx

kgabryje · 2024-11-13T16:13:50Z

/testenv up

github-actions · 2024-11-13T16:27:31Z

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

…o/fix/color-consistency-race-conditions

* fix: Exception handling for SQL Lab views (apache#30897) * fix: don't show metadata for embedded dashboards (apache#30875) * feat: add logging durations for screenshot async service (apache#30884) * refactor(Avatar): Migrate Avatar to Ant Design 5 (apache#30740) * refactor(input): Migrate Input component to Ant Design 5 (apache#30730) * fix(empty dashboards): Allow downloading a screenshot of an empty dashboard (apache#30767) Co-authored-by: Geido <[email protected]> * chore(ci): add tai and michael to helm owners (apache#30925) * fix(helm): use submodule on helm release action (apache#30924) * docs: add Free2Move to INTHEWILD.md (apache#30930) * fix: blocks UI elements on right side (apache#30886) Co-authored-by: Evan Rusackas <[email protected]> * chore(deps): Migrate from `crate[sqlalchemy]` to `sqlalchemy-cratedb` (apache#29243) * chore: update change log, UPDATING.md and bug-report.yml for 4.1 release (apache#30915) * chore(docs): Update list of supported databases to include CrateDB (apache#30946) * feat(trino,presto): add missing time grains (apache#30926) * fix(Dashboard): Exclude edit param in async screenshot (apache#30962) * fix(Card): Use correct class names for Ant Design 5 Card component (apache#30964) * chore(Accessibility): Fix accessibility for 'Show x entries' dropdown in tables (apache#30818) * build(deps): bump cross-spawn from 7.0.3 to 7.0.6 in /superset-frontend/cypress-base (apache#30969) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * fix(release validation): scripts now support RSA and EDDSA keys. (apache#30967) * build(deps): bump cross-spawn from 7.0.3 to 7.0.6 in /docs (apache#30970) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore: add performance information to tooltip (apache#30948) * fix(helm-chart): Fix broken PodDisruptionBudget due to introduction of extraLabels. (apache#30966) * chore(helm): bumping app version to 4.1.1 in helm chart (apache#30918) * chore: add unit tests for `is_mutating()` (apache#31021) * chore(🦾): bump python numexpr 2.10.0 -> 2.10.1 (apache#31006) Co-authored-by: GitHub Action <[email protected]> * chore(🦾): bump python cffi 1.16.0 -> 1.17.1 (apache#31002) Co-authored-by: GitHub Action <[email protected]> * chore(🦾): bump python flask-babel subpackage(s) (apache#31000) Co-authored-by: GitHub Action <[email protected]> * fix(Dashboard): Retain colors when color scheme not set (apache#30646) * fix(explore): verified props is not updated (apache#31008) * chore: publish wheels (apache#30981) * docs: Embedded sdk (apache#30972) * fix(imports): import query_context for imports with charts (apache#30887) * docs: Update doc about CSV upload (apache#30867) Co-authored-by: Evan Rusackas <[email protected]> * chore(🦾): bump python cattrs 23.2.3 -> 24.1.2 (apache#30998) Co-authored-by: GitHub Action <[email protected]> * fix(dataset): use sqlglot for DML check (apache#31024) --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: Michael S. Molina <[email protected]> Co-authored-by: Joe Li <[email protected]> Co-authored-by: Maxime Beauchemin <[email protected]> Co-authored-by: Mehmet Salih Yavuz <[email protected]> Co-authored-by: Geido <[email protected]> Co-authored-by: Ville Brofeldt <[email protected]> Co-authored-by: Paolo Terzi <[email protected]> Co-authored-by: Sukuna <[email protected]> Co-authored-by: Evan Rusackas <[email protected]> Co-authored-by: Andreas Motl <[email protected]> Co-authored-by: Andreas Motl <[email protected]> Co-authored-by: Levis Mbote <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Elizabeth Thompson <[email protected]> Co-authored-by: Christoph Keller <[email protected]> Co-authored-by: lodu <[email protected]> Co-authored-by: Beto Dealmeida <[email protected]> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: GitHub Action <[email protected]> Co-authored-by: JUST.in DO IT <[email protected]> Co-authored-by: David Hotham <[email protected]> Co-authored-by: Giampaolo Capelli <[email protected]> Co-authored-by: Linden <[email protected]> Co-authored-by: Seiya <[email protected]> Co-authored-by: Asher Manangan <[email protected]>

(cherry picked from commit 90572be)

geido added 3 commits October 17, 2024 17:46

fix(Dashboard): Retain colors when color scheme not set

b8db377

fix(E2E): Re-enable tests in editmode

66ba4fb

fix(Dashboard): Keep track of charts origin color schemes

208a929

pull-request-size bot added the size/L label Oct 17, 2024

github-actions bot added the packages label Oct 17, 2024

chore(Tests): Increase coverage

a41c6b5

geido force-pushed the geido/fix/color-consistency-race-conditions branch from f65b57b to 23dbe5e Compare October 18, 2024 15:22

geido marked this pull request as ready for review October 18, 2024 15:23

geido requested review from jinghua-qa, eschutho, rusackas and betodealmeida as code owners October 18, 2024 15:23

dosubot bot added the dashboard:colors Related to the color scheme of the Dashboard label Oct 18, 2024

chore(Cypress): Enhance tests reliability

7e23659

geido force-pushed the geido/fix/color-consistency-race-conditions branch from 23dbe5e to 7e23659 Compare October 18, 2024 15:43

geido requested a review from fisjac October 18, 2024 16:03

rusackas reviewed Oct 18, 2024

View reviewed changes

superset-frontend/cypress-base/cypress/e2e/dashboard/editmode.test.ts Show resolved Hide resolved

apache deleted a comment from github-actions bot Oct 18, 2024

rusackas approved these changes Oct 18, 2024

View reviewed changes

fisjac approved these changes Oct 18, 2024

View reviewed changes

michael-s-molina added review:checkpoint Last PR reviewed during the daily review standup v4.1 Label added by the release manager to track PRs to be included in the 4.1 branch labels Oct 18, 2024

geido added the hold! On hold label Oct 18, 2024

michael-s-molina removed the review:checkpoint Last PR reviewed during the daily review standup label Oct 21, 2024

sadpandajoe assigned geido Oct 22, 2024

feat(Dashboard): Edit original scheme when no scheme set for the Dash…

3317776

…board

geido requested a review from villebro as a code owner October 24, 2024 15:28

kgabryje reviewed Nov 7, 2024

View reviewed changes

superset-frontend/src/explore/components/controls/ColorSchemeControl/index.tsx Show resolved Hide resolved

kgabryje reviewed Nov 7, 2024

View reviewed changes

superset-frontend/src/explore/components/controls/ColorSchemeControl/index.tsx Outdated Show resolved Hide resolved

geido added 3 commits November 8, 2024 19:54

fix(Explore): Differentiate dashboard color scheme

7d8a6bc

chore(Dashboard): dry-up color management logics

16f141f

Merge

1d8d647

This comment was marked as outdated.

Sign in to view

geido added the hold:testing! On hold for testing label Nov 13, 2024

fix(Dashboard): Apply colors in time for viz re-rendering

46a5072

kgabryje approved these changes Nov 13, 2024

View reviewed changes

geido added 4 commits November 15, 2024 22:09

refactor(Dashboard) Split color application logics

dc762a5

fix(E2E): Remove onbeforeunload event in Cypress run

670579f

chore(E2E): Add color consistency test scenarios

6cf2615

Merge branch 'master' of https://github.com/apache/superset into geid…

69cd840

…o/fix/color-consistency-race-conditions

geido force-pushed the geido/fix/color-consistency-race-conditions branch 6 times, most recently from 3f7df9f to 84ea4d6 Compare November 21, 2024 16:14

geido removed the hold:testing! On hold for testing label Nov 21, 2024

geido force-pushed the geido/fix/color-consistency-race-conditions branch from 84ea4d6 to 02c5b5d Compare November 21, 2024 16:28

chore(Dashboard): Clean up color consistency functions

d78af29

geido force-pushed the geido/fix/color-consistency-race-conditions branch from 02c5b5d to d78af29 Compare November 21, 2024 17:35

geido merged commit 90572be into master Nov 21, 2024
34 checks passed

geido mentioned this pull request Nov 22, 2024

fix(Dashboard): Ensure shared label colors are updated #31031

Merged

9 tasks

sadpandajoe pushed a commit that referenced this pull request Nov 22, 2024

fix(Dashboard): Retain colors when color scheme not set (#30646)

c467fb5

(cherry picked from commit 90572be)

geido mentioned this pull request Nov 26, 2024

fix(Dashboard): Backward compatible shared_label_colors field #31163

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(Dashboard): Retain colors when color scheme not set #30646

fix(Dashboard): Retain colors when color scheme not set #30646

geido commented Oct 17, 2024 •

edited

Loading

hanslemm commented Oct 18, 2024 •

edited

Loading

rusackas commented Oct 18, 2024

rusackas left a comment

geido commented Oct 18, 2024

fisjac left a comment

This comment was marked as outdated.

kgabryje commented Nov 13, 2024

github-actions bot commented Nov 13, 2024

fix(Dashboard): Retain colors when color scheme not set #30646

fix(Dashboard): Retain colors when color scheme not set #30646

Conversation

geido commented Oct 17, 2024 • edited Loading

SUMMARY

BEFORE

AFTER

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

hanslemm commented Oct 18, 2024 • edited Loading

rusackas commented Oct 18, 2024

rusackas left a comment

Choose a reason for hiding this comment

geido commented Oct 18, 2024

fisjac left a comment

Choose a reason for hiding this comment

This comment was marked as outdated.

kgabryje commented Nov 13, 2024

github-actions bot commented Nov 13, 2024

geido commented Oct 17, 2024 •

edited

Loading

hanslemm commented Oct 18, 2024 •

edited

Loading