fix(Dashboard): Retain colors when color scheme not set (apache#30646)

superset-frontend/cypress-base/cypress/e2e/dashboard/shared_dashboard_functions.ts

@@ -91,7 +91,7 @@ export function prepareDashboardFilters(
         refresh_frequency: 0,
         color_scheme: '',
         label_colors: {},
-        shared_label_colors: {},
+        shared_label_colors: [],
         color_scheme_domain: [],
         cross_filters_enabled: false,
         positions: {

superset-frontend/cypress-base/cypress/e2e/dashboard/utils.ts

@@ -125,7 +125,7 @@ export const valueNativeFilterOptions = [
 ];
 
 export function interceptGet() {
-  cy.intercept('/api/v1/dashboard/*').as('get');
+  cy.intercept('GET', '/api/v1/dashboard/*').as('get');
 }
 
 export function interceptFiltering() {
@@ -144,6 +144,10 @@ export function interceptUpdate() {
   cy.intercept('PUT', `/api/v1/dashboard/*`).as('update');
 }
 
+export function interceptExploreUpdate() {
+  cy.intercept('PUT', `/api/v1/chart/*`).as('chartUpdate');
+}
+
 export function interceptPost() {
   cy.intercept('POST', `/api/v1/dashboard/`).as('post');
 }
@@ -524,13 +528,17 @@ export function addCountryNameFilter() {
   );
 }
 
-export function openTab(tabComponentIndex: number, tabIndex: number) {
-  return cy
-    .getBySel('dashboard-component-tabs')
+export function openTab(
+  tabComponentIndex: number,
+  tabIndex: number,
+  target = 'dashboard-component-tabs',
+) {
+  cy.getBySel(target)
     .eq(tabComponentIndex)
     .find('[role="tab"]')
     .eq(tabIndex)
     .click();
+  cy.wait(500);
 }
 
 export const openTopLevelTab = (tabName: string) => {

superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/sharedControls.tsx

@@ -50,6 +50,7 @@ import {
 import {
   formatSelectOptions,
   displayTimeRelatedControls,
+  getColorControlsProps,
   D3_FORMAT_OPTIONS,
   D3_FORMAT_DOCS,
   D3_TIME_FORMAT_OPTIONS,
@@ -142,9 +143,7 @@ const linear_color_scheme: SharedControlConfig<'ColorSchemeControl'> = {
   renderTrigger: true,
   schemes: () => sequentialSchemeRegistry.getMap(),
   isLinear: true,
-  mapStateToProps: state => ({
-    dashboardId: state?.form_data?.dashboardId,
-  }),
+  mapStateToProps: state => getColorControlsProps(state),
 };
 
 const granularity: SharedControlConfig<'SelectControl'> = {
@@ -333,9 +332,7 @@ const color_scheme: SharedControlConfig<'ColorSchemeControl'> = {
   choices: () => categoricalSchemeRegistry.keys().map(s => [s, s]),
   description: t('The color scheme for rendering chart'),
   schemes: () => categoricalSchemeRegistry.getMap(),
-  mapStateToProps: state => ({
-    dashboardId: state?.form_data?.dashboardId,
-  }),
+  mapStateToProps: state => getColorControlsProps(state),
 };
 
 const time_shift_color: SharedControlConfig<'CheckboxControl'> = {

superset-frontend/packages/superset-ui-chart-controls/src/types.ts

@@ -87,6 +87,9 @@ export interface Dataset {
 }
 
 export interface ControlPanelState {
+  slice: {
+    slice_id: number;
+  };
   form_data: QueryFormData;
   datasource: Dataset | QueryResponse | null;
   controls: ControlStateMapping;

superset-frontend/packages/superset-ui-chart-controls/src/utils/colorControls.ts

@@ -0,0 +1,32 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+export const getColorControlsProps = (state: Record<string, any>) => {
+  const dashboardId = state?.form_data?.dashboardId;
+  return {
+    chartId: state?.slice?.slice_id,
+    dashboardId,
+    hasDashboardColorScheme:
+      !!dashboardId && !!state?.form_data?.dashboard_color_scheme,
+    hasCustomLabelsColor:
+      Object.keys(state?.form_data?.label_colors || {}).length > 0,
+    colorNamespace: state?.form_data?.color_namespace,
+    mapLabelsColors: state?.form_data?.map_label_colors || {},
+    sharedLabelsColors: state?.form_data?.shared_label_colors || [],
+  };
+};

superset-frontend/packages/superset-ui-chart-controls/src/utils/index.ts

@@ -27,3 +27,4 @@ export * from './defineSavedMetrics';
 export * from './getStandardizedControls';
 export * from './getTemporalColumns';
 export { default as displayTimeRelatedControls } from './displayTimeRelatedControls';
+export * from './colorControls';

superset-frontend/packages/superset-ui-chart-controls/test/utils/colorControls.test.ts

@@ -0,0 +1,112 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { getColorControlsProps } from '../../src';
+
+describe('getColorControlsProps', () => {
+  it('should return default values when state is empty', () => {
+    const state = {};
+    const result = getColorControlsProps(state);
+    expect(result).toEqual({
+      chartId: undefined,
+      dashboardId: undefined,
+      hasDashboardColorScheme: false,
+      hasCustomLabelsColor: false,
+      colorNamespace: undefined,
+      mapLabelsColors: {},
+      sharedLabelsColors: [],
+    });
+  });
+
+  it('should return correct values when state has form_data with dashboardId and color scheme', () => {
+    const state = {
+      form_data: {
+        dashboardId: 123,
+        dashboard_color_scheme: 'blueScheme',
+        label_colors: {},
+      },
+      slice: { slice_id: 456 },
+    };
+    const result = getColorControlsProps(state);
+    expect(result).toEqual({
+      chartId: 456,
+      dashboardId: 123,
+      hasDashboardColorScheme: true,
+      hasCustomLabelsColor: false,
+      colorNamespace: undefined,
+      mapLabelsColors: {},
+      sharedLabelsColors: [],
+    });
+  });
+
+  it('should detect custom label colors correctly', () => {
+    const state = {
+      form_data: {
+        dashboardId: 123,
+        label_colors: { label1: '#000000' },
+      },
+      slice: { slice_id: 456 },
+    };
+    const result = getColorControlsProps(state);
+    expect(result).toEqual({
+      chartId: 456,
+      dashboardId: 123,
+      hasDashboardColorScheme: false,
+      hasCustomLabelsColor: true,
+      colorNamespace: undefined,
+      mapLabelsColors: {},
+      sharedLabelsColors: [],
+    });
+  });
+
+  it('should return shared label colors when available', () => {
+    const state = {
+      form_data: {
+        shared_label_colors: ['#FF5733', '#33FF57'],
+      },
+    };
+    const result = getColorControlsProps(state);
+    expect(result).toEqual({
+      chartId: undefined,
+      dashboardId: undefined,
+      hasDashboardColorScheme: false,
+      hasCustomLabelsColor: false,
+      sharedLabelsColors: ['#FF5733', '#33FF57'],
+      colorNamespace: undefined,
+      mapLabelsColors: {},
+    });
+  });
+
+  it('should handle missing form_data and slice properties', () => {
+    const state = {
+      form_data: {
+        dashboardId: 789,
+      },
+    };
+    const result = getColorControlsProps(state);
+    expect(result).toEqual({
+      chartId: undefined,
+      dashboardId: 789,
+      hasDashboardColorScheme: false,
+      hasCustomLabelsColor: false,
+      colorNamespace: undefined,
+      mapLabelsColors: {},
+      sharedLabelsColors: [],
+    });
+  });
+});

superset-frontend/packages/superset-ui-core/src/color/CategoricalColorNamespace.ts

@@ -17,6 +17,7 @@
  * under the License.
  */
 
+import { cloneDeep } from 'lodash';
 import CategoricalColorScale from './CategoricalColorScale';
 import { ColorsLookup } from './types';
 import getCategoricalSchemeRegistry from './CategoricalSchemeRegistrySingleton';
@@ -37,10 +38,21 @@ export default class CategoricalColorNamespace {
     this.forcedItems = {};
   }
 
-  getScale(schemeId?: string) {
-    const id = schemeId ?? getCategoricalSchemeRegistry().getDefaultKey() ?? '';
+  /**
+   * A new CategoricalColorScale instance is created for each chart.
+   *
+   * @param colorScheme - the color scheme to use
+   * @returns a new instance of a color scale
+   */
+  getScale(colorScheme?: string) {
+    const id =
+      colorScheme ?? getCategoricalSchemeRegistry().getDefaultKey() ?? '';
     const scheme = getCategoricalSchemeRegistry().get(id);
-    return new CategoricalColorScale(scheme?.colors ?? [], this.forcedItems);
+    return new CategoricalColorScale(
+      scheme?.colors ?? [],
+      this.forcedItems,
+      colorScheme,
+    );
   }
 
   /**
@@ -59,6 +71,17 @@ export default class CategoricalColorNamespace {
   resetColors() {
     this.forcedItems = {};
   }
+
+  resetColorsForLabels(labels: string[] = []) {
+    const updatedForcedItems = cloneDeep(this.forcedItems);
+    labels.forEach(label => {
+      if (updatedForcedItems.hasOwnProperty(label)) {
+        delete updatedForcedItems[label];
+      }
+    });
+
+    this.forcedItems = { ...updatedForcedItems };
+  }
 }
 
 const namespaces: {
@@ -80,16 +103,19 @@ export function getNamespace(name: string = DEFAULT_NAMESPACE) {
 
 export function getColor(
   value?: string,
-  schemeId?: string,
+  colorScheme?: string,
   namespace?: string,
 ) {
-  return getNamespace(namespace).getScale(schemeId).getColor(value);
+  return getNamespace(namespace).getScale(colorScheme).getColor(value);
 }
 
 /*
   Returns a new scale instance within the same namespace.
   Especially useful when a chart is booting for the first time
+
+  @param scheme - the applied color scheme
+  @param namespace - the namespace
 */
-export function getScale(scheme?: string, namespace?: string) {
-  return getNamespace(namespace).getScale(scheme);
+export function getScale(colorScheme?: string, namespace?: string) {
+  return getNamespace(namespace).getScale(colorScheme);
 }