fix: Color consistency (#1406)

* Replace color in scheme statically * Set color statically and re-use instance * Fix tests and clean up * Improve comments * Refactor and simplify * Update tests * Remove unnecessary ColorMapControl * Remove unnecessary const * Remove control label_colors
2026-04-18 23:55:00 +00:00 · 2021-11-02 15:16:49 +02:00
parent b6bc25ef78
commit 57e90b1c4b
30 changed files with 33 additions and 70 deletions
--- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart-controls/src/sections/sections.tsx
+++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart-controls/src/sections/sections.tsx
@@ -104,7 +104,7 @@ export const datasourceAndVizType: ControlPanelSectionConfig = {

 export const colorScheme: ControlPanelSectionConfig = {
  label: t('Color Scheme'),
-  controlSetRows: [['color_scheme', 'label_colors']],
+  controlSetRows: [['color_scheme']],
 };

 export const annotations: ControlPanelSectionConfig = {
--- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart-controls/src/shared-controls/index.tsx
+++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart-controls/src/shared-controls/index.tsx
@@ -478,19 +478,6 @@ const color_scheme: SharedControlConfig<'ColorSchemeControl'> = {
  schemes: () => categoricalSchemeRegistry.getMap(),
 };

-const label_colors: SharedControlConfig<'ColorMapControl'> = {
-  type: 'ColorMapControl',
-  label: t('Color Map'),
-  default: {},
-  renderTrigger: true,
-  mapStateToProps: ({
-    form_data: { color_namespace: colorNamespace, color_scheme: colorScheme },
-  }) => ({
-    colorNamespace,
-    colorScheme,
-  }),
-};
-
 const enableExploreDnd = isFeatureEnabled(FeatureFlag.ENABLE_EXPLORE_DRAG_AND_DROP);

 const sharedControls = {
@@ -522,7 +509,6 @@ const sharedControls = {
  x_axis_time_format,
  adhoc_filters: enableExploreDnd ? dnd_adhoc_filters : adhoc_filters,
  color_scheme,
-  label_colors,
  series_columns: enableExploreDnd ? dndColumnsControl : columnsControl,
  series_limit,
  series_limit_metric: enableExploreDnd ? dnd_sort_by : sort_by,
--- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart-controls/src/types.ts
+++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart-controls/src/types.ts
@@ -117,7 +117,6 @@ export type InternalControlType =
  | 'BoundsControl'
  | 'CheckboxControl'
  | 'CollectionControl'
-  | 'ColorMapControl'
  | 'ColorPickerControl'
  | 'ColorSchemeControl'
  | 'DatasourceControl'
--- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/src/color/CategoricalColorNamespace.ts
+++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/src/color/CategoricalColorNamespace.ts
@@ -39,14 +39,8 @@ export default class CategoricalColorNamespace {

  getScale(schemeId?: string) {
    const id = schemeId ?? getCategoricalSchemeRegistry().getDefaultKey() ?? '';
-    const scale = this.scales[id];
-    if (scale) {
-      return scale;
-    }
    const scheme = getCategoricalSchemeRegistry().get(id);
-
    const newScale = new CategoricalColorScale(scheme?.colors ?? [], this.forcedItems);
-    this.scales[id] = newScale;

    return newScale;
  }
@@ -63,6 +57,10 @@ export default class CategoricalColorNamespace {

    return this;
  }
+
+  resetColors() {
+    this.forcedItems = {};
+  }
 }

 const namespaces: {
@@ -86,6 +84,10 @@ export function getColor(value?: string, schemeId?: string, namespace?: string)
  return getNamespace(namespace).getScale(schemeId).getColor(value);
 }

+/*
+  Returns a new scale instance within the same namespace.
+  Especially useful when a chart is booting for the first time
+*/
 export function getScale(scheme?: string, namespace?: string) {
  return getNamespace(namespace).getScale(scheme);
 }
--- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/src/color/CategoricalColorScale.ts
+++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/src/color/CategoricalColorScale.ts
@@ -57,7 +57,6 @@ class CategoricalColorScale extends ExtensibleFunction {

  getColor(value?: string) {
    const cleanedValue = stringifyAndTrim(value);
-
    const parentColor = this.parentForcedColors && this.parentForcedColors[cleanedValue];
    if (parentColor) {
      return parentColor;
@@ -78,7 +77,6 @@ class CategoricalColorScale extends ExtensibleFunction {
   */
  setColor(value: string, forcedColor: string) {
    this.forcedColors[stringifyAndTrim(value)] = forcedColor;
-
    return this;
  }

--- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/test/color/CategoricalColorNameSpace.test.ts
+++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-core/test/color/CategoricalColorNameSpace.test.ts
@@ -81,15 +81,6 @@ describe('CategoricalColorNamespace', () => {
      expect(scale).toBeDefined();
      getCategoricalSchemeRegistry().setDefaultKey('testColors');
    });
-    it('returns same scale if the scale with that name already exists in this namespace', () => {
-      const namespace = getNamespace('test-get-scale2');
-      const scale1 = namespace.getScale('testColors');
-      const scale2 = namespace.getScale('testColors2');
-      const scale3 = namespace.getScale('testColors2');
-      const scale4 = namespace.getScale('testColors');
-      expect(scale1).toBe(scale4);
-      expect(scale2).toBe(scale3);
-    });
  });
  describe('.setColor()', () => {
    it('overwrites color for all CategoricalColorScales in this namespace', () => {
@@ -127,19 +118,15 @@ describe('CategoricalColorNamespace', () => {
      const scale = getScale();
      expect(scale).toBeDefined();
      const scale2 = getNamespace().getScale();
-      expect(scale).toBe(scale2);
+      expect(scale2).toBeDefined();
    });
    it('getScale(scheme) returns a CategoricalColorScale with specified scheme in default namespace', () => {
-      const scale = getScale('testColors');
+      const scale = getNamespace().getScale('testColors');
      expect(scale).toBeDefined();
-      const scale2 = getNamespace().getScale('testColors');
-      expect(scale).toBe(scale2);
    });
    it('getScale(scheme, namespace) returns a CategoricalColorScale with specified scheme in specified namespace', () => {
-      const scale = getScale('testColors', 'test-getScale');
+      const scale = getNamespace('test-getScale').getScale('testColors');
      expect(scale).toBeDefined();
-      const scale2 = getNamespace('test-getScale').getScale('testColors');
-      expect(scale).toBe(scale2);
    });
  });
  describe('static getColor()', () => {
--- a/superset-frontend/temporary_superset_ui/superset-ui/plugins/legacy-plugin-chart-chord/src/controlPanel.ts
+++ b/superset-frontend/temporary_superset_ui/superset-ui/plugins/legacy-plugin-chart-chord/src/controlPanel.ts
@@ -46,10 +46,7 @@ const config: ControlPanelConfig = {
    {
      label: t('Chart Options'),
      expanded: true,
-      controlSetRows: [
-        ['y_axis_format', null],
-        ['color_scheme', 'label_colors'],
-      ],
+      controlSetRows: [['y_axis_format', null], ['color_scheme']],
    },
  ],
  controlOverrides: {
--- a/superset-frontend/temporary_superset_ui/superset-ui/plugins/legacy-plugin-chart-histogram/src/controlPanel.ts
+++ b/superset-frontend/temporary_superset_ui/superset-ui/plugins/legacy-plugin-chart-histogram/src/controlPanel.ts
@@ -70,7 +70,7 @@ const config: ControlPanelConfig = {
      label: t('Chart Options'),
      expanded: true,
      controlSetRows: [
-        ['color_scheme', 'label_colors'],
+        ['color_scheme'],
        [
          {
            name: 'link_length',
--- a/superset-frontend/temporary_superset_ui/superset-ui/plugins/legacy-plugin-chart-partition/src/controlPanel.tsx
+++ b/superset-frontend/temporary_superset_ui/superset-ui/plugins/legacy-plugin-chart-partition/src/controlPanel.tsx
@@ -131,7 +131,7 @@ const config: ControlPanelConfig = {
      expanded: true,
      tabOverride: 'customize',
      controlSetRows: [
-        ['color_scheme', 'label_colors'],
+        ['color_scheme'],
        [
          {
            name: 'number_format',
--- a/superset-frontend/temporary_superset_ui/superset-ui/plugins/legacy-plugin-chart-rose/src/controlPanel.tsx
+++ b/superset-frontend/temporary_superset_ui/superset-ui/plugins/legacy-plugin-chart-rose/src/controlPanel.tsx
@@ -65,7 +65,7 @@ const config: ControlPanelConfig = {
      label: t('Chart Options'),
      expanded: true,
      controlSetRows: [
-        ['color_scheme', 'label_colors'],
+        ['color_scheme'],
        [
          {
            name: 'number_format',
--- a/superset-frontend/temporary_superset_ui/superset-ui/plugins/legacy-plugin-chart-sankey-loop/src/controlPanel.ts
+++ b/superset-frontend/temporary_superset_ui/superset-ui/plugins/legacy-plugin-chart-sankey-loop/src/controlPanel.ts
@@ -30,7 +30,7 @@ const config: ControlPanelConfig = {
    {
      label: t('Chart Options'),
      expanded: true,
-      controlSetRows: [['color_scheme', 'label_colors']],
+      controlSetRows: [['color_scheme']],
    },
  ],
  controlOverrides: {
--- a/superset-frontend/temporary_superset_ui/superset-ui/plugins/legacy-plugin-chart-sankey/src/controlPanel.ts
+++ b/superset-frontend/temporary_superset_ui/superset-ui/plugins/legacy-plugin-chart-sankey/src/controlPanel.ts
@@ -62,7 +62,7 @@ const config: ControlPanelConfig = {
    {
      label: t('Chart Options'),
      expanded: true,
-      controlSetRows: [['color_scheme', 'label_colors']],
+      controlSetRows: [['color_scheme']],
    },
  ],
 };
--- a/superset-frontend/temporary_superset_ui/superset-ui/plugins/legacy-plugin-chart-treemap/src/controlPanel.ts
+++ b/superset-frontend/temporary_superset_ui/superset-ui/plugins/legacy-plugin-chart-treemap/src/controlPanel.ts
@@ -54,7 +54,7 @@ const config: ControlPanelConfig = {
      expanded: true,
      tabOverride: 'customize',
      controlSetRows: [
-        ['color_scheme', 'label_colors'],
+        ['color_scheme'],
        [
          {
            name: 'treemap_ratio',
--- a/superset-frontend/temporary_superset_ui/superset-ui/plugins/legacy-preset-chart-nvd3/src/Area/controlPanel.ts
+++ b/superset-frontend/temporary_superset_ui/superset-ui/plugins/legacy-preset-chart-nvd3/src/Area/controlPanel.ts
@@ -61,7 +61,7 @@ const config: ControlPanelConfig = {
            },
          },
        ],
-        ['color_scheme', 'label_colors'],
+        ['color_scheme'],
        [richTooltip, showControls],
      ],
    },
--- a/superset-frontend/temporary_superset_ui/superset-ui/plugins/legacy-preset-chart-nvd3/src/Bar/controlPanel.ts
+++ b/superset-frontend/temporary_superset_ui/superset-ui/plugins/legacy-preset-chart-nvd3/src/Bar/controlPanel.ts
@@ -49,7 +49,6 @@ const config: ControlPanelConfig = {
      expanded: true,
      controlSetRows: [
        ['color_scheme'],
-        ['label_colors'],
        [showBrush],
        [showLegend],
        [showBarValue],
--- a/superset-frontend/temporary_superset_ui/superset-ui/plugins/legacy-preset-chart-nvd3/src/Bubble/controlPanel.ts
+++ b/superset-frontend/temporary_superset_ui/superset-ui/plugins/legacy-preset-chart-nvd3/src/Bubble/controlPanel.ts
@@ -69,10 +69,7 @@ const config: ControlPanelConfig = {
      label: t('Chart Options'),
      expanded: true,
      tabOverride: 'customize',
-      controlSetRows: [
-        ['color_scheme', 'label_colors'],
-        [showLegend, null],
-      ],
+      controlSetRows: [['color_scheme'], [showLegend, null]],
    },
    {
      label: t('X Axis'),
--- a/superset-frontend/temporary_superset_ui/superset-ui/plugins/legacy-preset-chart-nvd3/src/Compare/controlPanel.ts
+++ b/superset-frontend/temporary_superset_ui/superset-ui/plugins/legacy-preset-chart-nvd3/src/Compare/controlPanel.ts
@@ -39,7 +39,7 @@ const config: ControlPanelConfig = {
    {
      label: t('Chart Options'),
      expanded: true,
-      controlSetRows: [['color_scheme', 'label_colors']],
+      controlSetRows: [['color_scheme']],
    },
    {
      label: t('X Axis'),
--- a/superset-frontend/temporary_superset_ui/superset-ui/plugins/legacy-preset-chart-nvd3/src/DistBar/controlPanel.ts
+++ b/superset-frontend/temporary_superset_ui/superset-ui/plugins/legacy-preset-chart-nvd3/src/DistBar/controlPanel.ts
@@ -79,7 +79,6 @@ const config: ControlPanelConfig = {
      expanded: true,
      controlSetRows: [
        ['color_scheme'],
-        ['label_colors'],
        [showLegend],
        [showBarValue],
        [barStacked],
--- a/superset-frontend/temporary_superset_ui/superset-ui/plugins/legacy-preset-chart-nvd3/src/DualLine/controlPanel.ts
+++ b/superset-frontend/temporary_superset_ui/superset-ui/plugins/legacy-preset-chart-nvd3/src/DualLine/controlPanel.ts
@@ -34,7 +34,7 @@ const config: ControlPanelConfig = {
    {
      label: t('Chart Options'),
      expanded: true,
-      controlSetRows: [['color_scheme', 'label_colors'], [showLegend], [xAxisFormat]],
+      controlSetRows: [['color_scheme'], [showLegend], [xAxisFormat]],
    },
    {
      label: t('Y Axis 1'),
--- a/superset-frontend/temporary_superset_ui/superset-ui/plugins/legacy-preset-chart-nvd3/src/Line/controlPanel.ts
+++ b/superset-frontend/temporary_superset_ui/superset-ui/plugins/legacy-preset-chart-nvd3/src/Line/controlPanel.ts
@@ -46,7 +46,6 @@ const config: ControlPanelConfig = {
      expanded: true,
      controlSetRows: [
        ['color_scheme'],
-        ['label_colors'],
        [showBrush],
        [
          {
--- a/superset-frontend/temporary_superset_ui/superset-ui/plugins/legacy-preset-chart-nvd3/src/LineMulti/controlPanel.ts
+++ b/superset-frontend/temporary_superset_ui/superset-ui/plugins/legacy-preset-chart-nvd3/src/LineMulti/controlPanel.ts
@@ -55,7 +55,7 @@ const config: ControlPanelConfig = {
      tabOverride: 'customize',
      expanded: true,
      controlSetRows: [
-        ['color_scheme', 'label_colors'],
+        ['color_scheme'],
        [
          {
            name: 'prefix_metric_with_slice_name',
--- a/superset-frontend/temporary_superset_ui/superset-ui/plugins/legacy-preset-chart-nvd3/src/Pie/controlPanel.ts
+++ b/superset-frontend/temporary_superset_ui/superset-ui/plugins/legacy-preset-chart-nvd3/src/Pie/controlPanel.ts
@@ -103,7 +103,7 @@ const config: ControlPanelConfig = {
            },
          },
        ],
-        ['color_scheme', 'label_colors'],
+        ['color_scheme'],
      ],
    },
  ],
--- a/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-echarts/src/MixedTimeseries/controlPanel.tsx
+++ b/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-echarts/src/MixedTimeseries/controlPanel.tsx
@@ -265,7 +265,7 @@ const config: ControlPanelConfig = {
      label: t('Chart Options'),
      expanded: true,
      controlSetRows: [
-        ['color_scheme', 'label_colors'],
+        ['color_scheme'],
        ...createCustomizeSection(t('Query A'), ''),
        ...createCustomizeSection(t('Query B'), 'B'),
        [
--- a/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-echarts/src/Timeseries/Area/controlPanel.tsx
+++ b/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-echarts/src/Timeseries/Area/controlPanel.tsx
@@ -99,7 +99,7 @@ const config: ControlPanelConfig = {
      label: t('Chart Options'),
      expanded: true,
      controlSetRows: [
-        ['color_scheme', 'label_colors'],
+        ['color_scheme'],
        [
          {
            name: 'seriesType',
--- a/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-echarts/src/Timeseries/Regular/Scatter/controlPanel.tsx
+++ b/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-echarts/src/Timeseries/Regular/Scatter/controlPanel.tsx
@@ -76,7 +76,7 @@ const config: ControlPanelConfig = {
      label: t('Chart Options'),
      expanded: true,
      controlSetRows: [
-        ['color_scheme', 'label_colors'],
+        ['color_scheme'],
        ...showValueSection,
        [
          {
--- a/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-echarts/src/Timeseries/Regular/controlPanel.tsx
+++ b/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-echarts/src/Timeseries/Regular/controlPanel.tsx
@@ -93,7 +93,7 @@ const config: ControlPanelConfig = {
      label: t('Chart Options'),
      expanded: true,
      controlSetRows: [
-        ['color_scheme', 'label_colors'],
+        ['color_scheme'],
        ...showValueSection,
        [
          {
--- a/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-echarts/src/Timeseries/Step/controlPanel.tsx
+++ b/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-echarts/src/Timeseries/Step/controlPanel.tsx
@@ -99,7 +99,7 @@ const config: ControlPanelConfig = {
      label: t('Chart Options'),
      expanded: true,
      controlSetRows: [
-        ['color_scheme', 'label_colors'],
+        ['color_scheme'],
        [
          {
            name: 'seriesType',
--- a/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-echarts/src/Timeseries/controlPanel.tsx
+++ b/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-echarts/src/Timeseries/controlPanel.tsx
@@ -100,7 +100,7 @@ const config: ControlPanelConfig = {
      label: t('Chart Options'),
      expanded: true,
      controlSetRows: [
-        ['color_scheme', 'label_colors'],
+        ['color_scheme'],
        [
          {
            name: 'seriesType',
--- a/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-word-cloud/src/plugin/controlPanel.ts
+++ b/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-word-cloud/src/plugin/controlPanel.ts
@@ -88,7 +88,7 @@ const config: ControlPanelConfig = {
            },
          },
        ],
-        ['color_scheme', 'label_colors'],
+        ['color_scheme'],
      ],
    },
  ],
--- a/superset-frontend/temporary_superset_ui/superset-ui/plugins/preset-chart-xy/src/BoxPlot/controlPanel.ts
+++ b/superset-frontend/temporary_superset_ui/superset-ui/plugins/preset-chart-xy/src/BoxPlot/controlPanel.ts
@@ -31,7 +31,7 @@ const config: ControlPanelConfig = {
      label: t('Chart Options'),
      expanded: true,
      controlSetRows: [
-        ['color_scheme', 'label_colors'],
+        ['color_scheme'],
        [
          {
            name: 'whisker_options',