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.

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

fix: Color consistency #17089

Merged
merged 31 commits into from
Nov 3, 2021
Merged

fix: Color consistency #17089

merged 31 commits into from
Nov 3, 2021

Conversation

geido
Copy link
Member

@geido geido commented Oct 13, 2021

SUMMARY

This PR refactors the color scheme implementation and fixes the following issues:

Fixes: #11677
Fixes: #16970
Fixes: #16891
Fixes: #16961
Fixes: #16980
Fixes: #16979

It adds the ability to port custom label colors from the Dashboard to Explore.
It makes the custom label colors persistent and independent from the color scheme change in Dashboard.

KNOWN ISSUES

There are several known issues related to the color consistency project currently affecting master which will be tackled in separate PRs, as follows:

REQUIRED SUPERSET-UI PR

This PR requires the change in the related Superset-ui PR to be merged in first apache-superset/superset-ui#1406

AFTER

Change a color scheme in Explore. Then refresh.
1.explore_change_and_reload.mp4
The chart should use the colors in order and restart from the first color in the scheme when running out of available colors.
2.explore_more_series_than_colors.mp4
Chart should inherit the color scheme of the dashboard
3.dashboard_chart_inherits_color.mp4
Charts in the dashboard should use custom label color, show the change immediately, on save and on refresh
5.dashboard_custom_label.mp4
Explore should use the same color scheme and custom label colors of the originating dashboard
6.explore_inherits_dashboard_custom_labels.mp4

TESTING INSTRUCTIONS

Please follow each test case as shown in the videos above

ADDITIONAL INFORMATION

@codecov
Copy link

codecov bot commented Oct 13, 2021

Codecov Report

Merging #17089 (2cc1ad8) into master (93bafa0) will decrease coverage by 0.03%.
The diff coverage is 65.21%.

❗ Current head 2cc1ad8 differs from pull request most recent head acacba5. Consider uploading reports for the commit acacba5 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master   #17089      +/-   ##
==========================================
- Coverage   77.10%   77.06%   -0.04%     
==========================================
  Files        1037     1037              
  Lines       55659    55659              
  Branches     7612     7612              
==========================================
- Hits        42914    42894      -20     
- Misses      12495    12510      +15     
- Partials      250      255       +5     
Flag Coverage Δ
hive ?
mysql 82.00% <ø> (+0.02%) ⬆️
postgres 82.01% <ø> (+0.02%) ⬆️
presto 81.88% <ø> (?)
python 82.27% <ø> (-0.07%) ⬇️
sqlite 81.68% <ø> (+0.02%) ⬆️

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

Impacted Files Coverage Δ
superset-frontend/src/chart/Chart.jsx 43.33% <ø> (ø)
superset-frontend/src/chart/ChartRenderer.jsx 37.17% <0.00%> (ø)
superset-frontend/src/dashboard/actions/hydrate.js 1.70% <0.00%> (ø)
.../src/dashboard/components/gridComponents/Chart.jsx 80.37% <ø> (ø)
...perset-frontend/src/dashboard/containers/Chart.jsx 92.30% <ø> (ø)
...perset-frontend/src/explore/components/Control.tsx 76.47% <0.00%> (ø)
...ntend/src/explore/components/ExploreChartPanel.jsx 14.28% <ø> (ø)
...nd/src/explore/components/ExploreViewContainer.jsx 2.11% <0.00%> (ø)
...-frontend/src/explore/components/controls/index.js 100.00% <ø> (ø)
...et-frontend/src/explore/controlPanels/sections.tsx 100.00% <ø> (ø)
... and 42 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 93bafa0...acacba5. Read the comment docs.

@apache apache deleted a comment from github-actions bot Oct 14, 2021
@apache apache deleted a comment from github-actions bot Oct 14, 2021
@geido
Copy link
Member Author

geido commented Nov 3, 2021

/testenv up

@github-actions
Copy link
Contributor

github-actions bot commented Nov 3, 2021

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

Copy link
Member

@kgabryje kgabryje left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@geido geido merged commit 59a6502 into apache:master Nov 3, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Nov 3, 2021

Ephemeral environment shutdown and build artifacts deleted.

@sadpandajoe
Copy link
Contributor

🏷️ 2021.44

sadpandajoe pushed a commit to preset-io/superset that referenced this pull request Nov 3, 2021
* Update label colors on the fly

* Clean up

* Improve getFormDataWithExtraFilters

* Improve code structure

* Remove labelColors from formData

* Exclude label_colors from URL

* Refactor color scheme implementation

* Clean up

* Refactor and simplify

* Fix lint

* Remove unnecessary ColorMapControl

* Lint

* Give json color scheme precedence

* Add label_colors prop in metadata

* Separate owners and dashboard meta requests

* Remove label_colors control

* bump superset-ui 0.18.19

* Fix end of file

* Update tests

* Fix lint

* Update Cypress

* Update setColorScheme method

* Use Antd modal body

(cherry picked from commit 59a6502)
eschutho pushed a commit that referenced this pull request Nov 22, 2021
* Update label colors on the fly

* Clean up

* Improve getFormDataWithExtraFilters

* Improve code structure

* Remove labelColors from formData

* Exclude label_colors from URL

* Refactor color scheme implementation

* Clean up

* Refactor and simplify

* Fix lint

* Remove unnecessary ColorMapControl

* Lint

* Give json color scheme precedence

* Add label_colors prop in metadata

* Separate owners and dashboard meta requests

* Remove label_colors control

* bump superset-ui 0.18.19

* Fix end of file

* Update tests

* Fix lint

* Update Cypress

* Update setColorScheme method

* Use Antd modal body
AAfghahi pushed a commit that referenced this pull request Jan 10, 2022
* Update label colors on the fly

* Clean up

* Improve getFormDataWithExtraFilters

* Improve code structure

* Remove labelColors from formData

* Exclude label_colors from URL

* Refactor color scheme implementation

* Clean up

* Refactor and simplify

* Fix lint

* Remove unnecessary ColorMapControl

* Lint

* Give json color scheme precedence

* Add label_colors prop in metadata

* Separate owners and dashboard meta requests

* Remove label_colors control

* bump superset-ui 0.18.19

* Fix end of file

* Update tests

* Fix lint

* Update Cypress

* Update setColorScheme method

* Use Antd modal body
@mistercrunch mistercrunch added 🍒 1.4.0 🍒 1.4.1 🍒 1.4.2 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.5.0 labels Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels preset:2021.44 size/L v1.4 🍒 1.4.0 🍒 1.4.1 🍒 1.4.2 🚢 1.5.0
Projects
None yet
6 participants