[GitHub] [superset] geido commented on a change in pull request #17792: feat: support color consistency

[GitBox <gi...@apache.org>]

geido commented on a change in pull request #17792:
URL: https://github.com/apache/superset/pull/17792#discussion_r772412386



##########
File path: superset-frontend/packages/superset-ui-core/src/color/CategoricalColorNamespace.ts
##########
@@ -39,8 +39,19 @@ export default class CategoricalColorNamespace {
 
   getScale(schemeId?: string) {
     const id = schemeId ?? getCategoricalSchemeRegistry().getDefaultKey() ?? '';
+    const scale = this.scales[id];
+    if (scale) {
+      return scale;
+    }

Review comment:
       I believe this might bring a few problems back. For instance, by reusing an existing scale like this in a dashboard, the charts would change colors based on the number of charts and their rendering order. We need to avoid that. Each chart should always use its own color scale and never share an existing color scale with any other chart within the same namespace and scheme. 
   
   This is what makes the whole implementation much harder and the core issue I was mentioning. As discussed, one way to solve this problem might be to move the logic of the color generation in the Dashboard, by pulling all the labels of all the charts and only re-apply different colors to those labels that are shared across multiple charts, rather than sharing their scale.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org