geido commented on code in PR #30646:
URL: https://github.com/apache/superset/pull/30646#discussion_r1835271362


##########
superset-frontend/src/utils/colorScheme.ts:
##########
@@ -107,31 +140,67 @@ export const refreshLabelsColorMap = (
  *
  * @param metadata - the dashboard metadata object
  */
-export const applyColors = (metadata: Record<string, any>, fresh = false) => {
+export const applyColors = (
+  metadata: Record<string, any>,
+  // fresh is used when changing / removing a color scheme
+  fresh = false,
+  // merge is used to catch new labels in the color map
+  merge = false,
+  // shared is used to apply only shared labels colors from a dashboard
+  // used in Explore with a dashboard context without color scheme
+  // or when loading a dashboard with no color scheme set
+  shared = false,
+) => {
   const colorNameSpace = getColorNamespace(metadata?.color_namespace);
   const categoricalNamespace =
     CategoricalColorNamespace.getNamespace(colorNameSpace);
   const colorScheme = metadata?.color_scheme;
   const customLabelColors = metadata?.label_colors || {};
-  // when scheme unset, update only custom label colors
-  const labelsColorMap = metadata?.shared_label_colors || {};
+  const sharedLabels = Array.isArray(metadata?.shared_label_colors)
+    ? metadata.shared_label_colors
+    : getSharedLabels();
+  const fullLabelsColor = metadata?.map_label_colors || {};
+  // filter the fullLabelsColor object to only shared labels
+  const sharedLabelsColors = Object.fromEntries(
+    Object.entries(fullLabelsColor).filter(([label]) =>
+      sharedLabels.includes(label),
+    ),
+  );
 
-  // reset forced colors (custom labels + labels color map)
-  categoricalNamespace.resetColors();
+  const labelsColorMap = shared
+    ? sharedLabelsColors
+    : metadata?.map_label_colors || {};
 
-  // apply custom label colors first
+  if (fresh) {
+    // reset custom label colors
+    categoricalNamespace.resetColors();
+  }
+
+  // re-apply custom label colors first
   Object.keys(customLabelColors).forEach(label => {
     categoricalNamespace.setColor(label, customLabelColors[label]);
   });
 
-  // re-instantiate a fresh labels color map based on current scheme
-  // will consider also just applied custom label colors
-  refreshLabelsColorMap(metadata?.color_namespace, colorScheme);
+  if (fresh || merge) {
+    // re-instantiate a fresh labels color map based on current scheme
+    // must consider also just applied customLabelColors if present
+    // will merge with existing colorMap only new labels when merge is true
+    refreshLabelsColorMap(metadata?.color_namespace, colorScheme, merge);
+  }
+
+  const currentColorMapEntries = shared
+    ? getSharedLabelsColorMapEntries(fullLabelsColor)
+    : getLabelsColorMapEntries();
 
   // get the fresh map that was just updated or existing
   const labelsColorMapEntries = fresh
-    ? getLabelsColorMapEntries()
-    : labelsColorMap;
+    ? currentColorMapEntries
+    : merge
+      ? {
+          ...currentColorMapEntries,
+          ...labelsColorMap,

Review Comment:
   `currentColorMapEntries` is the freshest,  `labelsColorMap` is what we have 
stored. The idea is that if new labels appear in `currentColorMapEntries` they 
should just get added but existing label colors of `labelsColorMap` should be 
kept.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to