From 0328dd270467e71260bfa85078beb7b38a87877b Mon Sep 17 00:00:00 2001
From: Lily Kuang <lily@preset.io>
Date: Fri, 14 Jul 2023 12:13:49 -0700
Subject: [PATCH] fix: color collision in dashboard with tabs (#24670)

---
 .../cypress/e2e/dashboard/editmode.test.ts       | 10 +++++-----
 .../src/color/CategoricalColorScale.ts           | 13 +++++++++----
 .../test/color/CategoricalColorScale.test.ts     | 16 ++++++++++++++++
 3 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/superset-frontend/cypress-base/cypress/e2e/dashboard/editmode.test.ts b/superset-frontend/cypress-base/cypress/e2e/dashboard/editmode.test.ts
index 416af35182288..b35105a7b5911 100644
--- a/superset-frontend/cypress-base/cypress/e2e/dashboard/editmode.test.ts
+++ b/superset-frontend/cypress-base/cypress/e2e/dashboard/editmode.test.ts
@@ -422,17 +422,17 @@ describe('Dashboard edit', () => {
         '[data-test-chart-name="Top 10 California Names Timeseries"] .line .nv-legend-symbol',
       )
         .first()
-        .should('have.css', 'fill', 'rgb(252, 199, 0)');
+        .should('have.css', 'fill', 'rgb(69, 78, 124)');
       cy.get(
         '[data-test-chart-name="Top 10 California Names Timeseries"] .line .nv-legend-symbol',
       )
         .eq(1)
-        .should('have.css', 'fill', 'rgb(143, 211, 228)');
+        .should('have.css', 'fill', 'rgb(224, 67, 85)');
       cy.get(
         '[data-test-chart-name="Top 10 California Names Timeseries"] .line .nv-legend-symbol',
       )
         .eq(2)
-        .should('have.css', 'fill', 'rgb(172, 225, 196)');
+        .should('have.css', 'fill', 'rgb(163, 143, 121)');
     });
 
     it('should show the same colors in Explore', () => {
@@ -463,7 +463,7 @@ describe('Dashboard edit', () => {
         '[data-test-chart-name="Top 10 California Names Timeseries"] .line .nv-legend-symbol',
       )
         .eq(1)
-        .should('have.css', 'fill', 'rgb(51, 61, 71)');
+        .should('have.css', 'fill', 'rgb(172, 32, 119)');
 
       openExplore('Top 10 California Names Timeseries');
 
@@ -474,7 +474,7 @@ describe('Dashboard edit', () => {
       // label Christopher
       cy.get('[data-test="chart-container"] .line .nv-legend-symbol')
         .eq(1)
-        .should('have.css', 'fill', 'rgb(108, 131, 142)');
+        .should('have.css', 'fill', 'rgb(172, 32, 119)');
     });
 
     it('should change color scheme multiple times', () => {
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 7a59372ad3af0..5004dce2dd7cc 100644
--- a/superset-frontend/packages/superset-ui-core/src/color/CategoricalColorScale.ts
+++ b/superset-frontend/packages/superset-ui-core/src/color/CategoricalColorScale.ts
@@ -78,15 +78,20 @@ class CategoricalColorScale extends ExtensibleFunction {
     cleanedValue: string,
   ) {
     // make sure we don't overwrite the origin colors
-    const updatedRange = [...this.originColors];
+    const updatedRange = new Set(this.originColors);
     // remove the color option from shared color
     sharedColorMap.forEach((value: string, key: string) => {
       if (key !== cleanedValue) {
-        const index = updatedRange.indexOf(value);
-        updatedRange.splice(index, 1);
+        updatedRange.delete(value);
       }
     });
-    this.range(updatedRange.length > 0 ? updatedRange : this.originColors);
+    // remove the color option from forced colors
+    Object.entries(this.parentForcedColors).forEach(([key, value]) => {
+      if (key !== cleanedValue) {
+        updatedRange.delete(value);
+      }
+    });
+    this.range(updatedRange.size > 0 ? [...updatedRange] : this.originColors);
   }
 
   getColor(value?: string, sliceId?: number) {
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 0a09df66091cb..0d98198de13b7 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
@@ -273,5 +273,21 @@ describe('CategoricalColorScale', () => {
       expect(scale.range()).toEqual(['blue', 'green', 'red']);
       sharedLabelColor.clear();
     });
+
+    it('should remove parentForcedColors from range', () => {
+      const parentForcedColors = { house: 'blue', cow: 'red' };
+      const scale = new CategoricalColorScale(
+        ['blue', 'red', 'green'],
+        parentForcedColors,
+      );
+      const sharedLabelColor = getSharedLabelColor();
+      sharedLabelColor.clear();
+      const colorMap = sharedLabelColor.getColorMap();
+      scale.removeSharedLabelColorFromRange(colorMap, 'pig');
+      expect(scale.range()).toEqual(['green']);
+      scale.removeSharedLabelColorFromRange(colorMap, 'cow');
+      expect(scale.range()).toEqual(['red', 'green']);
+      sharedLabelColor.clear();
+    });
   });
 });