fix label color will change color order

apache · Oct 14, 2022 · d20665a · d20665a
1 parent 5a2081f
commit d20665a
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 15 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
@@ -73,18 +73,19 @@ class CategoricalColorScale extends ExtensibleFunction {
       this.forcedColors?.[cleanedValue] ||
       sharedLabelColor.getColorMap().get(cleanedValue);
 
-    if (!color) {
-      if (isFeatureEnabled(FeatureFlag.USE_ANALAGOUS_COLORS)) {
-        const multiple = Math.floor(
-          this.domain().length / this.originColors.length,
-        );
-        if (multiple > this.multiple) {
-          this.multiple = multiple;
-          const newRange = getAnalogousColors(this.originColors, multiple);
-          this.range(this.originColors.concat(newRange));
-        }
+    if (isFeatureEnabled(FeatureFlag.USE_ANALAGOUS_COLORS)) {
+      const multiple = Math.floor(
+        this.domain().length / this.originColors.length,
+      );
+      if (multiple > this.multiple) {
+        this.multiple = multiple;
+        const newRange = getAnalogousColors(this.originColors, multiple);
+        this.range(this.originColors.concat(newRange));
       }
-      color = this.scale(cleanedValue);
+    }
+    const newColor = this.scale(cleanedValue);
+    if (!color) {
+      color = newColor;
     }
     sharedLabelColor.addSlice(cleanedValue, color, sliceId);
 

diff --git a/superset-frontend/packages/superset-ui-core/src/color/SharedLabelColorSingleton.ts b/superset-frontend/packages/superset-ui-core/src/color/SharedLabelColorSingleton.ts
@@ -64,8 +64,10 @@ export class SharedLabelColor {
     )
       return;
     const labels = this.sliceLabelMap.get(sliceId) || [];
-    labels.push(label);
-    this.sliceLabelMap.set(sliceId, labels);
+    if (!labels.includes(label)) {
+      labels.push(label);
+      this.sliceLabelMap.set(sliceId, labels);
+    }
     this.colorMap.set(label, color);
   }
 

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',
       });
     });
   });

diff --git a/superset-frontend/packages/superset-ui-core/test/color/SharedLabelColorSingleton.test.ts b/superset-frontend/packages/superset-ui-core/test/color/SharedLabelColorSingleton.test.ts
@@ -68,14 +68,29 @@ describe('SharedLabelColor', () => {
       expect(sharedLabelColor.sliceLabelMap.has(1)).toEqual(true);
       const labels = sharedLabelColor.sliceLabelMap.get(1);
       expect(labels?.includes('a')).toEqual(true);
+      const colorMap = sharedLabelColor.getColorMap();
+      expect(Object.fromEntries(colorMap)).toEqual({ a: 'red' });
     });
 
-    it('should add to sliceLabelColorMap when label exist', () => {
+    it('should add to sliceLabelColorMap when slice exist', () => {
       const sharedLabelColor = getSharedLabelColor();
       sharedLabelColor.addSlice('a', 'red', 1);
       sharedLabelColor.addSlice('b', 'blue', 1);
       const labels = sharedLabelColor.sliceLabelMap.get(1);
       expect(labels?.includes('b')).toEqual(true);
+      const colorMap = sharedLabelColor.getColorMap();
+      expect(Object.fromEntries(colorMap)).toEqual({ a: 'red', b: 'blue' });
+    });
+
+    it('should use last color if adding label repeatedly', () => {
+      const sharedLabelColor = getSharedLabelColor();
+      sharedLabelColor.addSlice('b', 'blue', 1);
+      sharedLabelColor.addSlice('b', 'green', 1);
+      const labels = sharedLabelColor.sliceLabelMap.get(1);
+      expect(labels?.includes('b')).toEqual(true);
+      expect(labels?.length).toEqual(1);
+      const colorMap = sharedLabelColor.getColorMap();
+      expect(Object.fromEntries(colorMap)).toEqual({ b: 'green' });
     });
 
     it('should do nothing when source is not dashboard', () => {