fix: color collision in dashboard with tabs (#24670)

apache · Jul 14, 2023 · 0328dd2 · 0328dd2
1 parent 65fb8e1
commit 0328dd2
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 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
@@ -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
@@ -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
@@ -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();
+    });
   });
 });