feat(color): color consistency enhancements (#21507)

apache · Oct 17, 2022 · 7a7181a · 7a7181a
1 parent 7ec136f
commit 7a7181a
Show file tree

Hide file tree

Showing 21 changed files with 229 additions and 237 deletions.
diff --git a/superset-frontend/packages/superset-ui-core/src/color/CategoricalColorScale.ts b/superset-frontend/packages/superset-ui-core/src/color/CategoricalColorScale.ts
@@ -51,7 +51,7 @@ class CategoricalColorScale extends ExtensibleFunction {
    * @param {*} parentForcedColors optional parameter that comes from parent
    * (usually CategoricalColorNamespace) and supersede this.forcedColors
    */
-  constructor(colors: string[], parentForcedColors?: ColorsLookup) {
+  constructor(colors: string[], parentForcedColors: ColorsLookup = {}) {
     super((value: string, sliceId?: number) => this.getColor(value, sliceId));
 
     this.originColors = colors;
@@ -67,17 +67,11 @@ class CategoricalColorScale extends ExtensibleFunction {
     const cleanedValue = stringifyAndTrim(value);
     const sharedLabelColor = getSharedLabelColor();
 
-    const parentColor = this.parentForcedColors?.[cleanedValue];
-    if (parentColor) {
-      sharedLabelColor.addSlice(cleanedValue, parentColor, sliceId);
-      return parentColor;
-    }
-
-    const forcedColor = this.forcedColors[cleanedValue];
-    if (forcedColor) {
-      sharedLabelColor.addSlice(cleanedValue, forcedColor, sliceId);
-      return forcedColor;
-    }
+    // priority: parentForcedColors > forcedColors > labelColors
+    let color =
+      this.parentForcedColors?.[cleanedValue] ||
+      this.forcedColors?.[cleanedValue] ||
+      sharedLabelColor.getColorMap().get(cleanedValue);
 
     if (isFeatureEnabled(FeatureFlag.USE_ANALAGOUS_COLORS)) {
       const multiple = Math.floor(
@@ -89,8 +83,10 @@ class CategoricalColorScale extends ExtensibleFunction {
         this.range(this.originColors.concat(newRange));
       }
     }
-
-    const color = this.scale(cleanedValue);
+    const newColor = this.scale(cleanedValue);
+    if (!color) {
+      color = newColor;
+    }
     sharedLabelColor.addSlice(cleanedValue, color, sliceId);
 
     return color;

diff --git a/superset-frontend/packages/superset-ui-core/src/color/SharedLabelColorSingleton.ts b/superset-frontend/packages/superset-ui-core/src/color/SharedLabelColorSingleton.ts
@@ -18,113 +18,81 @@
  */
 
 import { CategoricalColorNamespace } from '.';
-import { FeatureFlag, isFeatureEnabled, makeSingleton } from '../utils';
-import { getAnalogousColors } from './utils';
+import { makeSingleton } from '../utils';
 
+export enum SharedLabelColorSource {
+  dashboard,
+  explore,
+}
 export class SharedLabelColor {
-  sliceLabelColorMap: Record<number, Record<string, string | undefined>>;
+  sliceLabelMap: Map<number, string[]>;
 
-  constructor() {
-    // { sliceId1: { label1: color1 }, sliceId2: { label2: color2 } }
-    this.sliceLabelColorMap = {};
-  }
+  colorMap: Map<string, string>;
 
-  getColorMap(
-    colorNamespace?: string,
-    colorScheme?: string,
-    updateColorScheme?: boolean,
-  ) {
-    if (colorScheme) {
-      const categoricalNamespace =
-        CategoricalColorNamespace.getNamespace(colorNamespace);
-      const sharedLabels = this.getSharedLabels();
-      let generatedColors: string[] = [];
-      let sharedLabelMap;
+  source: SharedLabelColorSource;
 
-      if (sharedLabels.length) {
-        const colorScale = categoricalNamespace.getScale(colorScheme);
-        const colors = colorScale.range();
-        if (isFeatureEnabled(FeatureFlag.USE_ANALAGOUS_COLORS)) {
-          const multiple = Math.ceil(sharedLabels.length / colors.length);
-          generatedColors = getAnalogousColors(colors, multiple);
-          sharedLabelMap = sharedLabels.reduce(
-            (res, label, index) => ({
-              ...res,
-              [label.toString()]: generatedColors[index],
-            }),
-            {},
-          );
-        } else {
-          // reverse colors to reduce color conflicts
-          colorScale.range(colors.reverse());
-          sharedLabelMap = sharedLabels.reduce(
-            (res, label) => ({
-              ...res,
-              [label.toString()]: colorScale(label),
-            }),
-            {},
-          );
-        }
-      }
+  constructor() {
+    // { sliceId1: [label1, label2, ...], sliceId2: [label1, label2, ...] }
+    this.sliceLabelMap = new Map();
+    this.colorMap = new Map();
+    this.source = SharedLabelColorSource.dashboard;
+  }
 
-      const labelMap = Object.keys(this.sliceLabelColorMap).reduce(
-        (res, sliceId) => {
-          // get new color scale instance
-          const colorScale = categoricalNamespace.getScale(colorScheme);
-          return {
-            ...res,
-            ...Object.keys(this.sliceLabelColorMap[sliceId]).reduce(
-              (res, label) => ({
-                ...res,
-                [label]: updateColorScheme
-                  ? colorScale(label)
-                  : this.sliceLabelColorMap[sliceId][label],
-              }),
-              {},
-            ),
-          };
-        },
-        {},
-      );
+  updateColorMap(colorNamespace?: string, colorScheme?: string) {
+    const categoricalNamespace =
+      CategoricalColorNamespace.getNamespace(colorNamespace);
+    const newColorMap = new Map();
+    this.colorMap.clear();
+    this.sliceLabelMap.forEach(labels => {
+      const colorScale = categoricalNamespace.getScale(colorScheme);
+      labels.forEach(label => {
+        const newColor = colorScale(label);
+        newColorMap.set(label, newColor);
+      });
+    });
+    this.colorMap = newColorMap;
+  }
 
-      return {
-        ...labelMap,
-        ...sharedLabelMap,
-      };
-    }
-    return undefined;
+  getColorMap() {
+    return this.colorMap;
   }
 
   addSlice(label: string, color: string, sliceId?: number) {
-    if (!sliceId) return;
-    this.sliceLabelColorMap[sliceId] = {
-      ...this.sliceLabelColorMap[sliceId],
-      [label]: color,
-    };
+    if (
+      this.source !== SharedLabelColorSource.dashboard ||
+      sliceId === undefined
+    )
+      return;
+    const labels = this.sliceLabelMap.get(sliceId) || [];
+    if (!labels.includes(label)) {
+      labels.push(label);
+      this.sliceLabelMap.set(sliceId, labels);
+    }
+    this.colorMap.set(label, color);
   }
 
   removeSlice(sliceId: number) {
-    delete this.sliceLabelColorMap[sliceId];
+    if (this.source !== SharedLabelColorSource.dashboard) return;
+    this.sliceLabelMap.delete(sliceId);
+    const newColorMap = new Map();
+    this.sliceLabelMap.forEach(labels => {
+      labels.forEach(label => {
+        newColorMap.set(label, this.colorMap.get(label));
+      });
+    });
+    this.colorMap = newColorMap;
   }
 
-  clear() {
-    this.sliceLabelColorMap = {};
+  reset() {
+    const copyColorMap = new Map(this.colorMap);
+    copyColorMap.forEach((_, label) => {
+      this.colorMap.set(label, '');
+    });
   }
 
-  getSharedLabels() {
-    const tempLabels = new Set<string>();
-    const result = new Set<string>();
-    Object.keys(this.sliceLabelColorMap).forEach(sliceId => {
-      const colorMap = this.sliceLabelColorMap[sliceId];
-      Object.keys(colorMap).forEach(label => {
-        if (tempLabels.has(label) && !result.has(label)) {
-          result.add(label);
-        } else {
-          tempLabels.add(label);
-        }
-      });
-    });
-    return [...result];
+  clear() {
+    this.sliceLabelMap.clear();
+    this.colorMap.clear();
   }
 }
 

diff --git a/superset-frontend/packages/superset-ui-core/src/color/index.ts b/superset-frontend/packages/superset-ui-core/src/color/index.ts
@@ -35,6 +35,7 @@ export * from './utils';
 export {
   default as getSharedLabelColor,
   SharedLabelColor,
+  SharedLabelColorSource,
 } from './SharedLabelColorSingleton';
 
 export const BRAND_COLOR = '#00A699';
diff --git a/superset-frontend/packages/superset-ui-core/test/color/CategoricalColorScale.test.ts b/superset-frontend/packages/superset-ui-core/test/color/CategoricalColorScale.test.ts
@@ -140,7 +140,7 @@ describe('CategoricalColorScale', () => {
       expect(scale2.getColorMap()).toEqual({
         cow: 'black',
         pig: 'pink',
-        horse: 'blue',
+        horse: 'green',
       });
     });
   });