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

apache · Oct 17, 2024 · b8db377 · b8db377
1 parent bad48d0
commit b8db377
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 14 deletions.
diff --git a/superset-frontend/src/dashboard/actions/dashboardState.js b/superset-frontend/src/dashboard/actions/dashboardState.js
@@ -716,18 +716,21 @@ export const updateDashboardLabelsColor = () => async (dispatch, getState) => {
     }
 
     // stored labels color map and applied might differ
+    // new data may appear in the map (data changes)
+    // or new slices may appear while changing tabs
     const isMapSynced = isLabelsColorMapSynced(metadata);
     if (!isMapSynced) {
       // re-apply a fresh labels color map
-      applyColors(updatedMetadata, true);
+      applyColors(updatedMetadata, false, true);
       // pull and store the just applied labels color map
       updatedMetadata.shared_label_colors = getLabelsColorMapEntries();
     }
 
     // the stored color domain registry and fresh might differ at this point
     const freshColorSchemeDomain = getColorSchemeDomain(colorScheme);
     const isRegistrySynced =
-      colorSchemeDomain.toString() !== freshColorSchemeDomain.toString();
+      colorSchemeDomain.toString() === freshColorSchemeDomain.toString();
+
     if (colorScheme && !isRegistrySynced) {
       updatedMetadata.color_scheme_domain = freshColorSchemeDomain;
     }

diff --git a/superset-frontend/src/dashboard/components/PropertiesModal/index.tsx b/superset-frontend/src/dashboard/components/PropertiesModal/index.tsx
@@ -318,7 +318,6 @@ const PropertiesModal = ({
       return;
     }
 
-    const copyMetadata = { ...metadata };
     const colorNamespace = getColorNamespace(metadata?.color_namespace);
 
     // color scheme in json metadata has precedence over selection
@@ -332,8 +331,14 @@ const PropertiesModal = ({
       delete metadata.color_scheme_domain;
     }
 
-    // only apply colors, the user has not saved yet
-    applyColors(copyMetadata, true);
+    // only apply colors as the user has not saved yet
+    applyColors(
+      {
+        ...metadata,
+        color_scheme: currentColorScheme,
+      },
+      true,
+    );
 
     currentJsonMetadata = jsonStringify(metadata);
 

diff --git a/superset-frontend/src/utils/colorScheme.ts b/superset-frontend/src/utils/colorScheme.ts
@@ -107,31 +107,53 @@ 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 = false,
+  merge = false,
+) => {
   const colorNameSpace = getColorNamespace(metadata?.color_namespace);
   const categoricalNamespace =
     CategoricalColorNamespace.getNamespace(colorNameSpace);
-  const colorScheme = metadata?.color_scheme;
+  let colorScheme = metadata?.color_scheme;
   const customLabelColors = metadata?.label_colors || {};
   // when scheme unset, update only custom label colors
   const labelsColorMap = metadata?.shared_label_colors || {};
 
-  // reset forced colors (custom labels + labels color map)
-  categoricalNamespace.resetColors();
+  if (!colorScheme) {
+    // a fallback color scheme must be set to generate shared label colors
+    const categoricalSchemes = getCategoricalSchemeRegistry();
+    colorScheme =
+      categoricalSchemes.getDefaultKey()?.toString() ?? 'supersetColors';
+  }
+
+  if (fresh) {
+    // reset forced colors (custom labels + labels color map)
+    categoricalNamespace.resetColors();
+  }
 
   // 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) {
+    // re-instantiate a fresh labels color map based on current scheme
+    // must consider also just applied customLabelColors
+    refreshLabelsColorMap(metadata?.color_namespace, colorScheme);
+  }
+
+  const currentColorMapEntries = getLabelsColorMapEntries();
 
   // get the fresh map that was just updated or existing
   const labelsColorMapEntries = fresh
-    ? getLabelsColorMapEntries()
-    : labelsColorMap;
+    ? currentColorMapEntries
+    : merge
+      ? {
+          ...currentColorMapEntries,
+          ...labelsColorMap,
+        }
+      : labelsColorMap;
 
   // apply the final color map
   Object.keys(labelsColorMapEntries).forEach(label => {