From d20665a1879692db892b22523cf481b1e574b33b Mon Sep 17 00:00:00 2001
From: stephenLYZ <750188453@qq.com>
Date: Fri, 14 Oct 2022 13:25:26 +0800
Subject: [PATCH] fix label color will change color order

---
 .../src/color/CategoricalColorScale.ts        | 23 ++++++++++---------
 .../src/color/SharedLabelColorSingleton.ts    |  6 +++--
 .../test/color/CategoricalColorScale.test.ts  |  2 +-
 .../color/SharedLabelColorSingleton.test.ts   | 17 +++++++++++++-
 4 files changed, 33 insertions(+), 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
index c03c525198faf..5f29ad4775996 100644
--- 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
index c4dbf459542a6..bc417e6036a51 100644
--- 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
index 91a8f4a3185a7..9e83aaba9a871 100644
--- 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
index 2441f886f3ac4..88610874dbd74 100644
--- 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', () => {