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 48b23ce18d639..97cef2cae2f5c 100644 --- a/superset-frontend/cypress-base/cypress/e2e/dashboard/editmode.test.ts +++ b/superset-frontend/cypress-base/cypress/e2e/dashboard/editmode.test.ts @@ -127,7 +127,7 @@ function selectColorScheme(color: string) { } function applyChanges() { - cy.getBySel('properties-modal-apply-button').click(); + cy.getBySel('properties-modal-apply-button').click({ force: true }); } function saveChanges() { @@ -359,17 +359,17 @@ describe('Dashboard edit', () => { '[data-test-chart-name="Top 10 California Names Timeseries"] .line .nv-legend-symbol', ) .first() - .should('have.css', 'fill', 'rgb(255, 0, 0)'); + .should('have.css', 'fill', 'rgb(234, 11, 140)'); cy.get( '[data-test-chart-name="Top 10 California Names Timeseries"] .line .nv-legend-symbol', ) .eq(1) - .should('have.css', 'fill', 'rgb(157, 172, 185)'); + .should('have.css', 'fill', 'rgb(108, 131, 142)'); cy.get( '[data-test-chart-name="Top 10 California Names Timeseries"] .line .nv-legend-symbol', ) .eq(2) - .should('have.css', 'fill', 'rgb(45, 85, 132)'); + .should('have.css', 'fill', 'rgb(41, 171, 226)'); }); it('should re-apply original color after removing custom label color with no color scheme set', () => { @@ -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(31, 168, 201)'); + .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(69, 78, 124)'); + .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(90, 193, 137)'); + .should('have.css', 'fill', 'rgb(168, 104, 183)'); }); it('should show the same colors in Explore', () => { 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 f9d0056deeae2..7a59372ad3af0 100644 --- a/superset-frontend/packages/superset-ui-core/src/color/CategoricalColorScale.ts +++ b/superset-frontend/packages/superset-ui-core/src/color/CategoricalColorScale.ts @@ -77,15 +77,13 @@ class CategoricalColorScale extends ExtensibleFunction { sharedColorMap: Map, cleanedValue: string, ) { - if (isFeatureEnabled(FeatureFlag.USE_ANALAGOUS_COLORS)) { - return; - } - + // make sure we don't overwrite the origin colors const updatedRange = [...this.originColors]; // remove the color option from shared color sharedColorMap.forEach((value: string, key: string) => { if (key !== cleanedValue) { - updatedRange.splice(updatedRange.indexOf(value), 1); + const index = updatedRange.indexOf(value); + updatedRange.splice(index, 1); } }); this.range(updatedRange.length > 0 ? updatedRange : this.originColors); @@ -116,9 +114,10 @@ class CategoricalColorScale extends ExtensibleFunction { const newColor = this.scale(cleanedValue); if (!color) { color = newColor; - // make sure we don't overwrite the origin colors - this.removeSharedLabelColorFromRange(sharedColorMap, cleanedValue); - color = this.scale(cleanedValue); + if (isFeatureEnabled(FeatureFlag.AVOID_COLORS_COLLISION)) { + this.removeSharedLabelColorFromRange(sharedColorMap, cleanedValue); + color = this.scale(cleanedValue); + } } sharedLabelColor.addSlice(cleanedValue, color, sliceId); diff --git a/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts b/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts index 2cf67b080c68a..48e54d18416a8 100644 --- a/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts +++ b/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts @@ -59,6 +59,7 @@ export enum FeatureFlag { TAGGING_SYSTEM = 'TAGGING_SYSTEM', VERSIONED_EXPORT = 'VERSIONED_EXPORT', SSH_TUNNELING = 'SSH_TUNNELING', + AVOID_COLORS_COLLISION = 'AVOID_COLORS_COLLISION', } export type ScheduleQueriesProps = { JSONSCHEMA: { 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 8b39b9644f178..0a09df66091cb 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 @@ -120,6 +120,24 @@ describe('CategoricalColorScale', () => { scale.getColor('goat'); expect(scale.range()).toHaveLength(6); }); + + it('should remove shared color from range if avoid colors collision enabled', () => { + window.featureFlags = { + [FeatureFlag.AVOID_COLORS_COLLISION]: true, + }; + const scale = new CategoricalColorScale(['blue', 'red', 'green']); + const color1 = scale.getColor('a', 1); + expect(scale.range()).toHaveLength(3); + const color2 = scale.getColor('a', 2); + expect(color1).toBe(color2); + scale.getColor('b', 2); + expect(scale.range()).toHaveLength(2); + scale.getColor('c', 2); + expect(scale.range()).toHaveLength(1); + }); + window.featureFlags = { + [FeatureFlag.AVOID_COLORS_COLLISION]: false, + }; }); describe('.setColor(value, forcedColor)', () => { it('overrides default color', () => { @@ -228,31 +246,12 @@ describe('CategoricalColorScale', () => { }); describe('.removeSharedLabelColorFromRange(colorMap, cleanedValue)', () => { - it('range should be the same if analogous colors enabled', () => { - window.featureFlags = { - [FeatureFlag.USE_ANALAGOUS_COLORS]: true, - }; - const colors = ['blue', 'green', 'red']; - const scale = new CategoricalColorScale(colors); - expect(scale.range()).toEqual(colors); - - const sharedLabelColor = getSharedLabelColor(); - const colorMap = sharedLabelColor.getColorMap(); - sharedLabelColor.addSlice('cow', 'blue', 1); - sharedLabelColor.addSlice('goat', 'green', 1); - scale.removeSharedLabelColorFromRange(colorMap, 'pig'); - expect(scale.range()).toEqual(colors); - sharedLabelColor.clear(); - }); - it('should remove shared color from range', () => { - window.featureFlags = { - [FeatureFlag.USE_ANALAGOUS_COLORS]: false, - }; const scale = new CategoricalColorScale(['blue', 'green', 'red']); expect(scale.range()).toEqual(['blue', 'green', 'red']); const sharedLabelColor = getSharedLabelColor(); + sharedLabelColor.clear(); const colorMap = sharedLabelColor.getColorMap(); sharedLabelColor.addSlice('cow', 'blue', 1); scale.removeSharedLabelColorFromRange(colorMap, 'pig'); diff --git a/superset/config.py b/superset/config.py index ea602338d3aa1..998edf6b47daf 100644 --- a/superset/config.py +++ b/superset/config.py @@ -496,6 +496,7 @@ class D3Format(TypedDict, total=False): # Users must check whether the DB engine supports SSH Tunnels # otherwise enabling this flag won't have any effect on the DB. "SSH_TUNNELING": False, + "AVOID_COLORS_COLLISION": True, } # ------------------------------