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

superset-frontend/cypress-base/cypress/e2e/dashboard/editmode.test.ts

@@ -42,15 +42,29 @@ function openProperties() {
     cy.getBySel('header-actions-menu')
       .contains('Edit properties')
       .click({ force: true });
-    cy.wait(500);
+    cy.get('.ant-modal-body').should('be.visible');
   });
 }
 
+function assertMetadata(text: string) {
+  const regex = new RegExp(text);
+  cy.get('#json_metadata')
+    .should('be.visible')
+    .then(() => {
+      const metadata = cy.$$('#json_metadata')[0];
+
+      // cypress can read this locally, but not in ci
+      // so we have to use the ace module directly to fetch the value
+      expect(ace.edit(metadata).getValue()).to.match(regex);
+    });
+}
+
 function openAdvancedProperties() {
   cy.get('.ant-modal-body')
     .contains('Advanced')
     .should('be.visible')
     .click({ force: true });
+  cy.get('#json_metadata').should('be.visible');
 }
 
 function dragComponent(
@@ -96,7 +110,7 @@ function resetTabbedDashboard(go = false) {
       ...metadata,
       color_scheme: '',
       label_colors: {},
-      shared_label_colors: {},
+      shared_label_colors: [],
     });
     cy.updateDashboard(r.id, {
       certification_details: r.certification_details,
@@ -120,10 +134,13 @@ function visitResetTabbedDashboard() {
 
 function selectColorScheme(color: string) {
   cy.get(
-    '[data-test="dashboard-edit-properties-form"] [aria-label="Select color scheme"]',
+    '[data-test="dashboard-edit-properties-form"] input[aria-label="Select color scheme"]',
   )
     .first()
-    .click();
+    .then($input => {
+      cy.wrap($input).click({ force: true });
+      cy.wrap($input).type(color.slice(0, 3), { force: true });
+    });
   cy.getBySel(color).click({ force: true });
 }
 
@@ -137,34 +154,34 @@ function saveChanges() {
   cy.wait('@update');
 }
 
-function assertMetadata(text: string) {
-  const regex = new RegExp(text);
-  cy.get('#json_metadata')
-    .should('be.visible')
-    .then(() => {
-      const metadata = cy.$$('#json_metadata')[0];
-
-      // cypress can read this locally, but not in ci
-      // so we have to use the ace module directly to fetch the value
-      expect(ace.edit(metadata).getValue()).to.match(regex);
-    });
-}
 function clearMetadata() {
   cy.get('#json_metadata').then($jsonmetadata => {
-    cy.wrap($jsonmetadata).find('.ace_content').click();
+    cy.wrap($jsonmetadata).find('.ace_content').click({ force: true });
     cy.wrap($jsonmetadata)
       .find('.ace_text-input')
-      .type('{selectall} {backspace}', { force: true });
+      .then($ace => {
+        cy.wrap($ace).focus();
+        cy.wrap($ace).should('have.focus');
+        cy.wrap($ace).type('{selectall}', { force: true });
+        cy.wrap($ace).type('{backspace}', { force: true });
+      });
   });
 }
 
 function writeMetadata(metadata: string) {
-  cy.get('#json_metadata').then($jsonmetadata =>
-    cy
-      .wrap($jsonmetadata)
+  cy.get('#json_metadata').then($jsonmetadata => {
+    cy.wrap($jsonmetadata).find('.ace_content').click({ force: true });
+    cy.wrap($jsonmetadata)
       .find('.ace_text-input')
-      .type(metadata, { parseSpecialCharSequences: false, force: true }),
-  );
+      .then($ace => {
+        cy.wrap($ace).focus();
+        cy.wrap($ace).should('have.focus');
+        cy.wrap($ace).type(metadata, {
+          parseSpecialCharSequences: false,
+          force: true,
+        });
+      });
+  });
 }
 
 function openExplore(chartName: string) {
@@ -469,7 +486,7 @@ describe('Dashboard edit', () => {
         .should('have.css', 'fill', 'rgb(255, 0, 0)');
     });
 
-    it.skip('should change color scheme multiple times', () => {
+    it('should change color scheme multiple times', () => {
       openProperties();
       selectColorScheme('blueToGreen');
       applyChanges();
@@ -487,7 +504,7 @@ describe('Dashboard edit', () => {
         '[data-test-chart-name="Top 10 California Names Timeseries"] .line .nv-legend-symbol',
       )
         .first()
-        .should('have.css', 'fill', 'rgb(234, 11, 140)');
+        .should('have.css', 'fill', 'rgb(50, 0, 167)');
 
       // open 2nd main tab
       openTab(0, 1);
@@ -496,7 +513,7 @@ describe('Dashboard edit', () => {
       // label Anthony
       cy.get('[data-test-chart-name="Trends"] .line .nv-legend-symbol')
         .eq(2)
-        .should('have.css', 'fill', 'rgb(234, 11, 140)');
+        .should('have.css', 'fill', 'rgb(50, 0, 167)');
 
       editDashboard();
       openProperties();
@@ -507,7 +524,7 @@ describe('Dashboard edit', () => {
       // label Anthony
       cy.get('[data-test-chart-name="Trends"] .line .nv-legend-symbol')
         .eq(2)
-        .should('have.css', 'fill', 'rgb(41, 105, 107)');
+        .should('have.css', 'fill', 'rgb(0, 128, 246)');
 
       // open main tab and nested tab
       openTab(0, 0);
@@ -518,10 +535,10 @@ describe('Dashboard edit', () => {
         '[data-test-chart-name="Top 10 California Names Timeseries"] .line .nv-legend-symbol',
       )
         .first()
-        .should('have.css', 'fill', 'rgb(41, 105, 107)');
+        .should('have.css', 'fill', 'rgb(0, 128, 246)');
     });
 
-    it.skip('should apply the color scheme across main tabs', () => {
+    it('should apply the color scheme across main tabs', () => {
       openProperties();
       selectColorScheme('blueToGreen');
       applyChanges();
@@ -533,10 +550,10 @@ describe('Dashboard edit', () => {
 
       cy.get('[data-test-chart-name="Trends"] .line .nv-legend-symbol')
         .first()
-        .should('have.css', 'fill', 'rgb(234, 11, 140)');
+        .should('have.css', 'fill', 'rgb(50, 0, 167)');
     });
 
-    it.skip('should apply the color scheme across main tabs for rendered charts', () => {
+    it('should apply the color scheme across main tabs for rendered charts', () => {
       waitForChartLoad({ name: 'Treemap', viz: 'treemap_v2' });
       openProperties();
       selectColorScheme('blueToGreen');
@@ -549,7 +566,7 @@ describe('Dashboard edit', () => {
 
       cy.get('[data-test-chart-name="Trends"] .line .nv-legend-symbol')
         .first()
-        .should('have.css', 'fill', 'rgb(41, 105, 107)');
+        .should('have.css', 'fill', 'rgb(50, 0, 167)');
 
       // change scheme now that charts are rendered across the main tabs
       editDashboard();
@@ -560,10 +577,10 @@ describe('Dashboard edit', () => {
 
       cy.get('[data-test-chart-name="Trends"] .line .nv-legend-symbol')
         .first()
-        .should('have.css', 'fill', 'rgb(234, 11, 140)');
+        .should('have.css', 'fill', 'rgb(0, 128, 246)');
     });
 
-    it.skip('should apply the color scheme in nested tabs', () => {
+    it('should apply the color scheme in nested tabs', () => {
       openProperties();
       selectColorScheme('blueToGreen');
       applyChanges();
@@ -579,17 +596,17 @@ describe('Dashboard edit', () => {
         '[data-test-chart-name="Top 10 California Names Timeseries"] .line .nv-legend-symbol',
       )
         .first()
-        .should('have.css', 'fill', 'rgb(234, 11, 140)');
+        .should('have.css', 'fill', 'rgb(50, 0, 167)');
 
       // open another nested tab
       openTab(2, 1);
       waitForChartLoad({ name: 'Growth Rate', viz: 'line' });
       cy.get('[data-test-chart-name="Growth Rate"] .line .nv-legend-symbol')
         .first()
-        .should('have.css', 'fill', 'rgb(234, 11, 140)');
+        .should('have.css', 'fill', 'rgb(50, 0, 167)');
     });
 
-    it.skip('should apply a valid color scheme for rendered charts in nested tabs', () => {
+    it('should apply a valid color scheme for rendered charts in nested tabs', () => {
       // open the tab first time and let chart load
       openTab(1, 1);
       waitForChartLoad({
@@ -611,7 +628,7 @@ describe('Dashboard edit', () => {
         '[data-test-chart-name="Top 10 California Names Timeseries"] .line .nv-legend-symbol',
       )
         .first()
-        .should('have.css', 'fill', 'rgb(234, 11, 140)');
+        .should('have.css', 'fill', 'rgb(50, 0, 167)');
     });
   });
 
@@ -625,7 +642,7 @@ describe('Dashboard edit', () => {
       openProperties();
     });
 
-    it.skip('should accept a valid color scheme', () => {
+    it('should accept a valid color scheme', () => {
       openAdvancedProperties();
       clearMetadata();
       writeMetadata('{"color_scheme":"lyftColors"}');
@@ -636,21 +653,21 @@ describe('Dashboard edit', () => {
       applyChanges();
     });
 
-    it.skip('should overwrite the color scheme when advanced is closed', () => {
+    it('should overwrite the color scheme when advanced is closed', () => {
       selectColorScheme('blueToGreen');
       openAdvancedProperties();
       assertMetadata('blueToGreen');
       applyChanges();
     });
 
-    it.skip('should overwrite the color scheme when advanced is open', () => {
+    it('should overwrite the color scheme when advanced is open', () => {
       openAdvancedProperties();
       selectColorScheme('modernSunset');
       assertMetadata('modernSunset');
       applyChanges();
     });
 
-    it.skip('should not accept an invalid color scheme', () => {
+    it('should not accept an invalid color scheme', () => {
       openAdvancedProperties();
       clearMetadata();
       // allow console error
@@ -716,7 +733,7 @@ describe('Dashboard edit', () => {
       visitEdit();
     });
 
-    it.skip('should add charts', () => {
+    it('should add charts', () => {
       cy.get('[role="checkbox"]').click();
       dragComponent();
       cy.getBySel('dashboard-component-chart-holder').should('have.length', 1);
@@ -765,7 +782,7 @@ describe('Dashboard edit', () => {
       visitEdit();
     });
 
-    it.skip('should save', () => {
+    it('should save', () => {
       cy.get('[role="checkbox"]').click();
       dragComponent();
       cy.getBySel('header-save-button').should('be.enabled');

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

@@ -525,12 +525,12 @@ export function addCountryNameFilter() {
 }
 
 export function openTab(tabComponentIndex: number, tabIndex: number) {
-  return cy
-    .getBySel('dashboard-component-tabs')
+  cy.getBySel('dashboard-component-tabs')
     .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,30 @@
+/**
+ * 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,
+    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';