fix: Color consistency

superset-frontend/src/chart/Chart.jsx

@@ -44,6 +44,7 @@ const propTypes = {
   // formData contains chart's own filter parameter
   // and merged with extra filter that current dashboard applying
   formData: PropTypes.object.isRequired,
+  labelColors: PropTypes.object,
   width: PropTypes.number,
   height: PropTypes.number,
   setControlValue: PropTypes.func,

superset-frontend/src/chart/ChartRenderer.jsx

@@ -29,6 +29,7 @@ const propTypes = {
   datasource: PropTypes.object,
   initialValues: PropTypes.object,
   formData: PropTypes.object.isRequired,
+  labelColors: PropTypes.object,
   height: PropTypes.number,
   width: PropTypes.number,
   setControlValue: PropTypes.func,
@@ -100,6 +101,7 @@ class ChartRenderer extends React.Component {
         nextProps.height !== this.props.height ||
         nextProps.width !== this.props.width ||
         nextProps.triggerRender ||
+        nextProps.labelColors !== this.props.labelColors ||
         nextProps.formData.color_scheme !== this.props.formData.color_scheme ||
         nextProps.cacheBusterProp !== this.props.cacheBusterProp
       );

superset-frontend/src/dashboard/actions/dashboardInfo.ts

@@ -17,13 +17,32 @@
  * under the License.
  */
 import { Dispatch } from 'redux';
-import { makeApi } from '@superset-ui/core';
+import { makeApi, CategoricalColorNamespace } from '@superset-ui/core';
+import { isString } from 'lodash';
 import { ChartConfiguration, DashboardInfo } from '../reducers/types';
 
 export const DASHBOARD_INFO_UPDATED = 'DASHBOARD_INFO_UPDATED';
 
 // updates partially changed dashboard info
 export function dashboardInfoChanged(newInfo: { metadata: any }) {
+  const { metadata } = newInfo;
+  const { color_namespace: namespace, label_colors: labelColors } = metadata;
+
+  const categoricalNamespace = CategoricalColorNamespace.getNamespace(
+    namespace,
+  );
+
+  categoricalNamespace.resetColors();
+
+  if (metadata?.label_colors) {
+    const colorMap = isString(labelColors)
+      ? JSON.parse(labelColors)
+      : labelColors;
+    Object.keys(colorMap).forEach(label => {
+      categoricalNamespace.setColor(label, colorMap[label]);
+    });
+  }
+
   return { type: DASHBOARD_INFO_UPDATED, newInfo };
 }
 export const SET_CHART_CONFIG_BEGIN = 'SET_CHART_CONFIG_BEGIN';

superset-frontend/src/dashboard/actions/hydrate.js

@@ -88,16 +88,16 @@ export const hydrateDashboard = (dashboardData, chartData) => (
   // Priming the color palette with user's label-color mapping provided in
   // the dashboard's JSON metadata
   if (metadata?.label_colors) {
-    const scheme = metadata.color_scheme;
     const namespace = metadata.color_namespace;
     const colorMap = isString(metadata.label_colors)
       ? JSON.parse(metadata.label_colors)
       : metadata.label_colors;
+    const categoricalNamespace = CategoricalColorNamespace.getNamespace(
+      namespace,
+    );
+
     Object.keys(colorMap).forEach(label => {
-      CategoricalColorNamespace.getScale(scheme, namespace).setColor(
-        label,
-        colorMap[label],
-      );
+      categoricalNamespace.setColor(label, colorMap[label]);
     });
   }
 

superset-frontend/src/dashboard/components/Header/index.jsx

@@ -20,7 +20,7 @@
 import moment from 'moment';
 import React from 'react';
 import PropTypes from 'prop-types';
-import { styled, CategoricalColorNamespace, t } from '@superset-ui/core';
+import { styled, t } from '@superset-ui/core';
 import ButtonGroup from 'src/components/ButtonGroup';
 
 import {
@@ -341,19 +341,10 @@ class Header extends React.PureComponent {
       lastModifiedTime,
     } = this.props;
 
-    const scale = CategoricalColorNamespace.getScale(
-      colorScheme,
-      colorNamespace,
-    );
-
-    // use the colorScheme for default labels
-    let labelColors = colorScheme ? scale.getColorMap() : {};
-    // but allow metadata to overwrite if it exists
-    // eslint-disable-next-line camelcase
-    const metadataLabelColors = dashboardInfo.metadata?.label_colors;
-    if (metadataLabelColors) {
-      labelColors = { ...labelColors, ...metadataLabelColors };
-    }
+    const labelColors =
+      colorScheme && dashboardInfo?.metadata?.label_colors
+        ? dashboardInfo.metadata.label_colors
+        : {};
 
     // check refresh frequency is for current session or persist
     const refreshFrequency = shouldPersistRefreshFrequency

superset-frontend/src/dashboard/components/PropertiesModal/index.jsx

@@ -29,7 +29,6 @@ import {
   t,
   SupersetClient,
   getCategoricalSchemeRegistry,
-  CategoricalColorNamespace,
 } from '@superset-ui/core';
 
 import Modal from 'src/components/Modal';
@@ -140,14 +139,14 @@ class PropertiesModal extends React.PureComponent {
     JsonEditor.preload();
   }
 
-  onColorSchemeChange(value, { updateMetadata = true } = {}) {
+  onColorSchemeChange(colorScheme, { updateMetadata = true } = {}) {
     // check that color_scheme is valid
     const colorChoices = getCategoricalSchemeRegistry().keys();
     const { json_metadata: jsonMetadata } = this.state.values;
     const jsonMetadataObj = jsonMetadata?.length
       ? JSON.parse(jsonMetadata)
       : {};
-    if (!colorChoices.includes(value)) {
+    if (!colorChoices.includes(colorScheme)) {
       Modal.error({
         title: 'Error',
         content: t('A valid color scheme is required'),
@@ -161,20 +160,12 @@ class PropertiesModal extends React.PureComponent {
       updateMetadata &&
       Object.keys(jsonMetadataObj).includes('color_scheme')
     ) {
-      jsonMetadataObj.color_scheme = value;
-      jsonMetadataObj.label_colors = Object.keys(
-        jsonMetadataObj.label_colors ?? {},
-      ).reduce(
-        (prev, next) => ({
-          ...prev,
-          [next]: CategoricalColorNamespace.getScale(value)(next),
-        }),
-        {},
-      );
+      jsonMetadataObj.color_scheme = colorScheme;
+
       this.onMetadataChange(jsonStringify(jsonMetadataObj));
     }
 
-    this.updateFormState('colorScheme', value);
+    this.updateFormState('colorScheme', colorScheme);
   }
 
   onOwnersChange(value) {

superset-frontend/src/dashboard/components/SaveModal.tsx

@@ -21,7 +21,7 @@ import React from 'react';
 import { Radio } from 'src/components/Radio';
 import { RadioChangeEvent, Input } from 'src/common/components';
 import Button from 'src/components/Button';
-import { t, CategoricalColorNamespace, JsonResponse } from '@superset-ui/core';
+import { t, JsonResponse } from '@superset-ui/core';
 
 import ModalTrigger from 'src/components/ModalTrigger';
 import Checkbox from 'src/components/Checkbox';
@@ -131,11 +131,11 @@ class SaveModal extends React.PureComponent<SaveModalProps, SaveModalState> {
       lastModifiedTime,
     } = this.props;
 
-    const scale = CategoricalColorNamespace.getScale(
-      colorScheme,
-      colorNamespace,
-    );
-    const labelColors = colorScheme ? scale.getColorMap() : {};
+    const labelColors =
+      colorScheme && dashboardInfo?.metadata?.label_colors
+        ? dashboardInfo.metadata.label_colors
+        : {};
+
     // check refresh frequency is for current session or persist
     const refreshFrequency = shouldPersistRefreshFrequency
       ? currentRefreshFrequency

superset-frontend/src/dashboard/components/gridComponents/Chart.jsx

@@ -52,6 +52,7 @@ const propTypes = {
   // from redux
   chart: chartPropShape.isRequired,
   formData: PropTypes.object.isRequired,
+  labelColors: PropTypes.object,
   datasource: PropTypes.object,
   slice: slicePropShape.isRequired,
   sliceName: PropTypes.string.isRequired,
@@ -274,6 +275,7 @@ export default class Chart extends React.Component {
       editMode,
       filters,
       formData,
+      labelColors,
       updateSliceName,
       sliceName,
       toggleExpandSlice,
@@ -396,6 +398,7 @@ export default class Chart extends React.Component {
             dashboardId={dashboardId}
             initialValues={initialValues}
             formData={formData}
+            labelColors={labelColors}
             ownState={ownState}
             filterState={filterState}
             queriesResponse={chart.queriesResponse}

superset-frontend/src/dashboard/containers/Chart.jsx

@@ -61,7 +61,7 @@ function mapStateToProps(
     (chart && chart.form_data && datasources[chart.form_data.datasource]) ||
     PLACEHOLDER_DATASOURCE;
   const { colorScheme, colorNamespace } = dashboardState;
-
+  const labelColors = dashboardInfo?.metadata?.label_colors || {};
   // note: this method caches filters if possible to prevent render cascades
   const formData = getFormDataWithExtraFilters({
     layout: dashboardLayout.present,
@@ -75,13 +75,15 @@ function mapStateToProps(
     sliceId: id,
     nativeFilters,
     dataMask,
+    labelColors,
   });
 
   formData.dashboardId = dashboardInfo.id;
 
   return {
     chart,
     datasource,
+    labelColors,
     slice: sliceEntities.slices[id],
     timeout: dashboardInfo.common.conf.SUPERSET_WEBSERVER_TIMEOUT,
     filters: getActiveFilters() || EMPTY_OBJECT,

superset-frontend/src/dashboard/util/charts/getFormDataWithExtraFilters.ts

@@ -16,11 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import {
-  CategoricalColorNamespace,
-  DataRecordFilters,
-  JsonObject,
-} from '@superset-ui/core';
+import { DataRecordFilters, JsonObject } from '@superset-ui/core';
 import { ChartQueryPayload, Charts, LayoutItem } from 'src/dashboard/types';
 import { getExtraFormData } from 'src/dashboard/components/nativeFilters/utils';
 import { DataMaskStateWithId } from 'src/dataMask/types';
@@ -45,6 +41,7 @@ export interface GetFormDataWithExtraFiltersArguments {
   sliceId: number;
   dataMask: DataMaskStateWithId;
   nativeFilters: NativeFiltersState;
+  labelColors?: Record<string, string>;
 }
 
 // this function merge chart's formData with dashboard filters value,
@@ -61,11 +58,8 @@ export default function getFormDataWithExtraFilters({
   sliceId,
   layout,
   dataMask,
+  labelColors,
 }: GetFormDataWithExtraFiltersArguments) {
-  // Propagate color mapping to chart
-  const scale = CategoricalColorNamespace.getScale(colorScheme, colorNamespace);
-  const labelColors = scale.getColorMap();
-
   // if dashboard metadata + filters have not changed, use cache if possible
   const cachedFormData = cachedFormdataByChart[sliceId];
   if (
@@ -109,11 +103,12 @@ export default function getFormDataWithExtraFilters({
 
   const formData = {
     ...chart.formData,
-    ...(colorScheme && { color_scheme: colorScheme }),
     label_colors: labelColors,
+    ...(colorScheme && { color_scheme: colorScheme }),
     extra_filters: getEffectiveExtraFilters(filters),
     ...extraData,
   };
+
   cachedFiltersByChart[sliceId] = filters;
   cachedFormdataByChart[sliceId] = { ...formData, dataMask };
 

superset-frontend/src/explore/components/Control.tsx

@@ -69,7 +69,9 @@ export default function Control(props: ControlProps) {
 
   const ControlComponent = typeof type === 'string' ? controlMap[type] : type;
   if (!ControlComponent) {
-    return <>Unknown controlType: {type}</>;
+    // eslint-disable-next-line no-console
+    console.warn(`Unknown controlType: ${type}`);
+    return null;
   }
 
   return (

superset-frontend/src/explore/components/ExploreChartHeader.jsx

@@ -21,7 +21,12 @@ import { connect } from 'react-redux';
 import { bindActionCreators } from 'redux';
 import PropTypes from 'prop-types';
 import Icons from 'src/components/Icons';
-import { styled, t } from '@superset-ui/core';
+import {
+  CategoricalColorNamespace,
+  SupersetClient,
+  styled,
+  t,
+} from '@superset-ui/core';
 import { Tooltip } from 'src/components/Tooltip';
 import ReportModal from 'src/components/ReportModal';
 import {
@@ -31,6 +36,7 @@ import {
 } from 'src/reports/actions/reports';
 import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags';
 import HeaderReportActionsDropdown from 'src/components/ReportModal/HeaderReportActionsDropdown';
+import { getClientErrorObject } from 'src/utils/getClientErrorObject';
 import { chartPropShape } from '../../dashboard/util/propShapes';
 import ExploreActionButtons from './ExploreActionButtons';
 import RowCountLabel from './RowCountLabel';
@@ -53,6 +59,7 @@ const propTypes = {
   addHistory: PropTypes.func,
   can_overwrite: PropTypes.bool.isRequired,
   can_download: PropTypes.bool.isRequired,
+  dashboardId: PropTypes.number,
   isStarred: PropTypes.bool.isRequired,
   slice: PropTypes.object,
   sliceName: PropTypes.string,
@@ -108,12 +115,15 @@ export class ExploreChartHeader extends React.PureComponent {
     this.state = {
       isPropertiesModalOpen: false,
       showingReportModal: false,
+      existingOwners: [],
+      permissionsError: null,
     };
     this.openPropertiesModal = this.openPropertiesModal.bind(this);
     this.closePropertiesModal = this.closePropertiesModal.bind(this);
     this.showReportModal = this.showReportModal.bind(this);
     this.hideReportModal = this.hideReportModal.bind(this);
     this.renderReportModal = this.renderReportModal.bind(this);
+    this.fetchChartData = this.fetchChartData.bind(this);
   }
 
   componentDidMount() {
@@ -127,6 +137,47 @@ export class ExploreChartHeader extends React.PureComponent {
         chart.id,
       );
     }
+    this.fetchChartData();
+  }
+
+  async fetchChartData() {
+    try {
+      const { dashboardId, slice } = this.props;
+      const response = await SupersetClient.get({
+        endpoint: `/api/v1/chart/${slice.slice_id}`,
+      });
+      const chart = response.json.result;
+      const dashboards = chart.dashboards || [];
+      const dashboard =
+        dashboardId &&
+        dashboards.length &&
+        dashboards.find(d => d.id === dashboardId);
+
+      if (dashboard && dashboard.json_metadata) {
+        // setting the chart to use the dashboard custom label colors if any
+        const labelColors =
+          JSON.parse(dashboard.json_metadata).label_colors || {};
+        const categoricalNamespace = CategoricalColorNamespace.getNamespace();
+
+        Object.keys(labelColors).forEach(label => {
+          categoricalNamespace.setColor(label, labelColors[label]);
+        });
+      }
+
+      const existingOwners = chart.owners.map(owner => ({
+        value: owner.id,
+        label: `${owner.first_name} ${owner.last_name}`,
+      }));
+
+      this.setState({
+        existingOwners,
+      });
+    } catch (response) {
+      const clientError = await getClientErrorObject(response);
+      this.setState({
+        permissionsError: clientError,
+      });
+    }
   }
 
   getSliceName() {
@@ -244,6 +295,8 @@ export class ExploreChartHeader extends React.PureComponent {
                 onHide={this.closePropertiesModal}
                 onSave={this.props.sliceUpdated}
                 slice={this.props.slice}
+                error={this.state.permissionsError}
+                existingOwners={this.state.existingOwners}
               />
               <Tooltip
                 id="edit-desc-tooltip"