From 9aa5863770054f4e1db26cf58795ce982adf9a81 Mon Sep 17 00:00:00 2001 From: stephenLYZ <750188453@qq.com> Date: Mon, 9 May 2022 00:22:59 +0800 Subject: [PATCH 1/2] feat(superset-ui-core): add feature flag for the color analogous generator --- .../src/color/CategoricalColorScale.ts | 17 +++++---- .../src/color/SharedLabelColorSingleton.ts | 36 +++++++++++++------ .../src/utils/featureFlags.ts | 1 + superset/config.py | 1 + 4 files changed, 37 insertions(+), 18 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 c6f37e4ff771f..2a6a4d63a0473 100644 --- a/superset-frontend/packages/superset-ui-core/src/color/CategoricalColorScale.ts +++ b/superset-frontend/packages/superset-ui-core/src/color/CategoricalColorScale.ts @@ -24,6 +24,7 @@ import { ColorsLookup } from './types'; import stringifyAndTrim from './stringifyAndTrim'; import getSharedLabelColor from './SharedLabelColorSingleton'; import { getAnalogousColors } from './utils'; +import { FeatureFlag, isFeatureEnabled } from '../utils'; // Use type augmentation to correct the fact that // an instance of CategoricalScale is also a function @@ -79,13 +80,15 @@ class CategoricalColorScale extends ExtensibleFunction { return forcedColor; } - 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)); + } } const color = this.scale(cleanedValue); 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 d2a59ac7c292a..10a14df075910 100644 --- a/superset-frontend/packages/superset-ui-core/src/color/SharedLabelColorSingleton.ts +++ b/superset-frontend/packages/superset-ui-core/src/color/SharedLabelColorSingleton.ts @@ -18,7 +18,7 @@ */ import { CategoricalColorNamespace } from '.'; -import makeSingleton from '../utils/makeSingleton'; +import { FeatureFlag, isFeatureEnabled, makeSingleton } from '../utils'; import { getAnalogousColors } from './utils'; export class SharedLabelColor { @@ -37,25 +37,39 @@ export class SharedLabelColor { if (colorScheme) { const categoricalNamespace = CategoricalColorNamespace.getNamespace(colorNamespace); - const colors = categoricalNamespace.getScale(colorScheme).range(); const sharedLabels = this.getSharedLabels(); let generatedColors: string[] = []; let sharedLabelMap; if (sharedLabels.length) { - const multiple = Math.ceil(sharedLabels.length / colors.length); - generatedColors = getAnalogousColors(colors, multiple); - sharedLabelMap = sharedLabels.reduce( - (res, label, index) => ({ - ...res, - [label.toString()]: generatedColors[index], - }), - {}, - ); + const colorScale = categoricalNamespace.getScale(colorScheme); + const colors = colorScale.range(); + if (isFeatureEnabled(FeatureFlag.USE_ANALAGOUS_COLORS)) { + const multiple = Math.ceil(sharedLabels.length / colors.length); + generatedColors = getAnalogousColors(colors, multiple); + sharedLabelMap = sharedLabels.reduce( + (res, label, index) => ({ + ...res, + [label.toString()]: generatedColors[index], + }), + {}, + ); + } else { + // reverse colors to reduce color conflicts + colorScale.range(colors.reverse()); + sharedLabelMap = sharedLabels.reduce( + (res, label) => ({ + ...res, + [label.toString()]: colorScale(label), + }), + {}, + ); + } } const labelMap = Object.keys(this.sliceLabelColorMap).reduce( (res, sliceId) => { + // get new color scale instance const colorScale = categoricalNamespace.getScale(colorScheme); return { ...res, 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 03d44078344f7..c9e75b96f2cfb 100644 --- a/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts +++ b/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts @@ -53,6 +53,7 @@ export enum FeatureFlag { ALLOW_FULL_CSV_EXPORT = 'ALLOW_FULL_CSV_EXPORT', UX_BETA = 'UX_BETA', GENERIC_CHART_AXES = 'GENERIC_CHART_AXES', + USE_ANALAGOUS_COLORS = 'USE_ANALAGOUS_COLORS', } export type ScheduleQueriesProps = { JSONSCHEMA: { diff --git a/superset/config.py b/superset/config.py index 62198c484c64a..bd2af320f7d4c 100644 --- a/superset/config.py +++ b/superset/config.py @@ -422,6 +422,7 @@ def _try_json_readsha(filepath: str, length: int) -> Optional[str]: "UX_BETA": False, "GENERIC_CHART_AXES": False, "ALLOW_ADHOC_SUBQUERY": False, + "USE_ANALAGOUS_COLORS": True, } # Feature flags may also be set via 'SUPERSET_FEATURE_' prefixed environment vars. From 91d40464ac5436676c6c39165f1006fd0b7cca41 Mon Sep 17 00:00:00 2001 From: stephenLYZ <750188453@qq.com> Date: Tue, 24 May 2022 16:51:14 +0800 Subject: [PATCH 2/2] fix: test --- .../test/color/CategoricalColorScale.test.ts | 31 ++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) 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 1d47cf760e326..91a8f4a3185a7 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 @@ -18,7 +18,7 @@ */ import { ScaleOrdinal } from 'd3-scale'; -import { CategoricalColorScale } from '@superset-ui/core'; +import { CategoricalColorScale, FeatureFlag } from '@superset-ui/core'; describe('CategoricalColorScale', () => { it('exists', () => { @@ -62,7 +62,36 @@ describe('CategoricalColorScale', () => { expect(c2).not.toBe(c3); expect(c3).not.toBe(c1); }); + it('recycles colors when number of items exceed available colors', () => { + window.featureFlags = { + [FeatureFlag.USE_ANALAGOUS_COLORS]: false, + }; + const colorSet: { [key: string]: number } = {}; + const scale = new CategoricalColorScale(['blue', 'red', 'green']); + const colors = [ + scale.getColor('pig'), + scale.getColor('horse'), + scale.getColor('cat'), + scale.getColor('cow'), + scale.getColor('donkey'), + scale.getColor('goat'), + ]; + colors.forEach(color => { + if (colorSet[color]) { + colorSet[color] += 1; + } else { + colorSet[color] = 1; + } + }); + expect(Object.keys(colorSet)).toHaveLength(3); + ['blue', 'red', 'green'].forEach(color => { + expect(colorSet[color]).toBe(2); + }); + }); it('get analogous colors when number of items exceed available colors', () => { + window.featureFlags = { + [FeatureFlag.USE_ANALAGOUS_COLORS]: true, + }; const scale = new CategoricalColorScale(['blue', 'red', 'green']); scale.getColor('pig'); scale.getColor('horse');