From c01c9406cb4b2f5ba8e600ee60b6442397e9522a Mon Sep 17 00:00:00 2001 From: geido Date: Wed, 8 Sep 2021 20:39:51 +0300 Subject: [PATCH 1/6] Update label colors on the fly --- superset-frontend/src/chart/ChartRenderer.jsx | 1 + .../src/dashboard/actions/dashboardInfo.ts | 19 ++++++++++++++++++- .../src/dashboard/containers/Chart.jsx | 11 ++++++++++- .../charts/getFormDataWithExtraFilters.ts | 6 ++---- 4 files changed, 31 insertions(+), 6 deletions(-) diff --git a/superset-frontend/src/chart/ChartRenderer.jsx b/superset-frontend/src/chart/ChartRenderer.jsx index bbc4213817c30..cbd0970f91951 100644 --- a/superset-frontend/src/chart/ChartRenderer.jsx +++ b/superset-frontend/src/chart/ChartRenderer.jsx @@ -100,6 +100,7 @@ class ChartRenderer extends React.Component { nextProps.height !== this.props.height || nextProps.width !== this.props.width || nextProps.triggerRender || + nextProps.formData.label_colors !== this.props.formData.label_colors || nextProps.formData.color_scheme !== this.props.formData.color_scheme || nextProps.cacheBusterProp !== this.props.cacheBusterProp ); diff --git a/superset-frontend/src/dashboard/actions/dashboardInfo.ts b/superset-frontend/src/dashboard/actions/dashboardInfo.ts index 3aa30936dcedd..b539e8e39748f 100644 --- a/superset-frontend/src/dashboard/actions/dashboardInfo.ts +++ b/superset-frontend/src/dashboard/actions/dashboardInfo.ts @@ -17,13 +17,30 @@ * 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; + + 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; + Object.keys(colorMap).forEach(label => { + CategoricalColorNamespace.getScale(scheme, namespace).setColor( + label, + colorMap[label], + ); + }); + } + return { type: DASHBOARD_INFO_UPDATED, newInfo }; } export const SET_CHART_CONFIG_BEGIN = 'SET_CHART_CONFIG_BEGIN'; diff --git a/superset-frontend/src/dashboard/containers/Chart.jsx b/superset-frontend/src/dashboard/containers/Chart.jsx index c629209d42541..b3e9c368c4f2b 100644 --- a/superset-frontend/src/dashboard/containers/Chart.jsx +++ b/superset-frontend/src/dashboard/containers/Chart.jsx @@ -19,6 +19,7 @@ import { bindActionCreators } from 'redux'; import { connect } from 'react-redux'; +import { CategoricalColorNamespace } from '@superset-ui/core'; import { toggleExpandSlice, setFocusedFilterField, @@ -59,7 +60,8 @@ function mapStateToProps( (chart && chart.form_data && datasources[chart.form_data.datasource]) || PLACEHOLDER_DATASOURCE; const { colorScheme, colorNamespace } = dashboardState; - + const scale = CategoricalColorNamespace.getScale(colorScheme, colorNamespace); + const scaleLabelColors = scale.getColorMap(); // note: this method caches filters if possible to prevent render cascades const formData = getFormDataWithExtraFilters({ layout: dashboardLayout.present, @@ -73,8 +75,15 @@ function mapStateToProps( sliceId: id, nativeFilters, dataMask, + labelColors: + Object.keys(dashboardInfo.metadata.label_colors).length > 0 + ? dashboardInfo.metadata.label_colors + : scaleLabelColors, }); + console.log('dashboardInfo', dashboardInfo); + console.log('formData', formData); + formData.dashboardId = dashboardInfo.id; return { diff --git a/superset-frontend/src/dashboard/util/charts/getFormDataWithExtraFilters.ts b/superset-frontend/src/dashboard/util/charts/getFormDataWithExtraFilters.ts index ea5ef7b3aa0ea..e90dcbde75c7e 100644 --- a/superset-frontend/src/dashboard/util/charts/getFormDataWithExtraFilters.ts +++ b/superset-frontend/src/dashboard/util/charts/getFormDataWithExtraFilters.ts @@ -45,6 +45,7 @@ export interface GetFormDataWithExtraFiltersArguments { sliceId: number; dataMask: DataMaskStateWithId; nativeFilters: NativeFiltersState; + labelColors: Record; } // this function merge chart's formData with dashboard filters value, @@ -61,11 +62,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 ( From df8d585cdbd5bf69d2720687e49168f37ee6d54f Mon Sep 17 00:00:00 2001 From: geido Date: Wed, 8 Sep 2021 20:42:28 +0300 Subject: [PATCH 2/6] Clean up --- superset-frontend/src/dashboard/containers/Chart.jsx | 3 --- 1 file changed, 3 deletions(-) diff --git a/superset-frontend/src/dashboard/containers/Chart.jsx b/superset-frontend/src/dashboard/containers/Chart.jsx index b3e9c368c4f2b..38800f067fca9 100644 --- a/superset-frontend/src/dashboard/containers/Chart.jsx +++ b/superset-frontend/src/dashboard/containers/Chart.jsx @@ -81,9 +81,6 @@ function mapStateToProps( : scaleLabelColors, }); - console.log('dashboardInfo', dashboardInfo); - console.log('formData', formData); - formData.dashboardId = dashboardInfo.id; return { From 8b607229e161fb4826533dd73d54c7b24d0f49af Mon Sep 17 00:00:00 2001 From: geido Date: Wed, 8 Sep 2021 20:48:35 +0300 Subject: [PATCH 3/6] Improve getFormDataWithExtraFilters --- superset-frontend/src/dashboard/containers/Chart.jsx | 6 ++---- .../dashboard/util/charts/getFormDataWithExtraFilters.ts | 6 ++++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/superset-frontend/src/dashboard/containers/Chart.jsx b/superset-frontend/src/dashboard/containers/Chart.jsx index 38800f067fca9..837afef923e55 100644 --- a/superset-frontend/src/dashboard/containers/Chart.jsx +++ b/superset-frontend/src/dashboard/containers/Chart.jsx @@ -19,7 +19,6 @@ import { bindActionCreators } from 'redux'; import { connect } from 'react-redux'; -import { CategoricalColorNamespace } from '@superset-ui/core'; import { toggleExpandSlice, setFocusedFilterField, @@ -60,8 +59,6 @@ function mapStateToProps( (chart && chart.form_data && datasources[chart.form_data.datasource]) || PLACEHOLDER_DATASOURCE; const { colorScheme, colorNamespace } = dashboardState; - const scale = CategoricalColorNamespace.getScale(colorScheme, colorNamespace); - const scaleLabelColors = scale.getColorMap(); // note: this method caches filters if possible to prevent render cascades const formData = getFormDataWithExtraFilters({ layout: dashboardLayout.present, @@ -76,9 +73,10 @@ function mapStateToProps( nativeFilters, dataMask, labelColors: + dashboardInfo?.metadata?.label_colors && Object.keys(dashboardInfo.metadata.label_colors).length > 0 ? dashboardInfo.metadata.label_colors - : scaleLabelColors, + : undefined, }); formData.dashboardId = dashboardInfo.id; diff --git a/superset-frontend/src/dashboard/util/charts/getFormDataWithExtraFilters.ts b/superset-frontend/src/dashboard/util/charts/getFormDataWithExtraFilters.ts index e90dcbde75c7e..97ed851cc7079 100644 --- a/superset-frontend/src/dashboard/util/charts/getFormDataWithExtraFilters.ts +++ b/superset-frontend/src/dashboard/util/charts/getFormDataWithExtraFilters.ts @@ -45,7 +45,7 @@ export interface GetFormDataWithExtraFiltersArguments { sliceId: number; dataMask: DataMaskStateWithId; nativeFilters: NativeFiltersState; - labelColors: Record; + labelColors?: Record; } // this function merge chart's formData with dashboard filters value, @@ -64,6 +64,8 @@ export default function getFormDataWithExtraFilters({ dataMask, labelColors, }: GetFormDataWithExtraFiltersArguments) { + const scale = CategoricalColorNamespace.getScale(colorScheme, colorNamespace); + const scaleLabelColors = labelColors || scale.getColorMap(); // if dashboard metadata + filters have not changed, use cache if possible const cachedFormData = cachedFormdataByChart[sliceId]; if ( @@ -108,7 +110,7 @@ export default function getFormDataWithExtraFilters({ const formData = { ...chart.formData, ...(colorScheme && { color_scheme: colorScheme }), - label_colors: labelColors, + label_colors: scaleLabelColors, extra_filters: getEffectiveExtraFilters(filters), ...extraData, }; From f284259e62e3984551877aca52055094e34ea60b Mon Sep 17 00:00:00 2001 From: geido Date: Thu, 7 Oct 2021 12:02:19 +0000 Subject: [PATCH 4/6] Improve code structure --- .../src/dashboard/actions/dashboardInfo.ts | 13 ++++++++----- .../src/dashboard/containers/Chart.jsx | 8 +++----- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/superset-frontend/src/dashboard/actions/dashboardInfo.ts b/superset-frontend/src/dashboard/actions/dashboardInfo.ts index b539e8e39748f..51ab7c6ceaa8c 100644 --- a/superset-frontend/src/dashboard/actions/dashboardInfo.ts +++ b/superset-frontend/src/dashboard/actions/dashboardInfo.ts @@ -28,11 +28,14 @@ export function dashboardInfoChanged(newInfo: { metadata: any }) { const { metadata } = newInfo; 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 { + color_scheme: scheme, + color_namespace: namespace, + label_colors: labelColors, + } = metadata; + const colorMap = isString(labelColors) + ? JSON.parse(labelColors) + : labelColors; Object.keys(colorMap).forEach(label => { CategoricalColorNamespace.getScale(scheme, namespace).setColor( label, diff --git a/superset-frontend/src/dashboard/containers/Chart.jsx b/superset-frontend/src/dashboard/containers/Chart.jsx index f6c6a8d8bd280..cb692225ea6d7 100644 --- a/superset-frontend/src/dashboard/containers/Chart.jsx +++ b/superset-frontend/src/dashboard/containers/Chart.jsx @@ -61,6 +61,8 @@ function mapStateToProps( (chart && chart.form_data && datasources[chart.form_data.datasource]) || PLACEHOLDER_DATASOURCE; const { colorScheme, colorNamespace } = dashboardState; + const { metadata } = dashboardInfo; + const labelColors = metadata?.label_colors; // note: this method caches filters if possible to prevent render cascades const formData = getFormDataWithExtraFilters({ layout: dashboardLayout.present, @@ -74,11 +76,7 @@ function mapStateToProps( sliceId: id, nativeFilters, dataMask, - labelColors: - dashboardInfo?.metadata?.label_colors && - Object.keys(dashboardInfo.metadata.label_colors).length > 0 - ? dashboardInfo.metadata.label_colors - : undefined, + labelColors: labelColors && Object.keys(labelColors).length && labelColors, }); formData.dashboardId = dashboardInfo.id; From 69b7b62a7e239808655c5517029a4f6f784460ff Mon Sep 17 00:00:00 2001 From: geido Date: Thu, 7 Oct 2021 12:59:55 +0000 Subject: [PATCH 5/6] Remove labelColors from formData --- superset-frontend/src/chart/Chart.jsx | 1 + superset-frontend/src/chart/ChartRenderer.jsx | 3 ++- .../dashboard/components/gridComponents/Chart.jsx | 3 +++ .../src/dashboard/containers/Chart.jsx | 2 +- .../util/charts/getFormDataWithExtraFilters.ts | 13 +------------ 5 files changed, 8 insertions(+), 14 deletions(-) diff --git a/superset-frontend/src/chart/Chart.jsx b/superset-frontend/src/chart/Chart.jsx index e5cfd72585ed2..106cfcdcfaae6 100644 --- a/superset-frontend/src/chart/Chart.jsx +++ b/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, diff --git a/superset-frontend/src/chart/ChartRenderer.jsx b/superset-frontend/src/chart/ChartRenderer.jsx index cbd0970f91951..ac8b8db2ddeea 100644 --- a/superset-frontend/src/chart/ChartRenderer.jsx +++ b/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,7 +101,7 @@ class ChartRenderer extends React.Component { nextProps.height !== this.props.height || nextProps.width !== this.props.width || nextProps.triggerRender || - nextProps.formData.label_colors !== this.props.formData.label_colors || + nextProps.labelColors !== this.props.labelColors || nextProps.formData.color_scheme !== this.props.formData.color_scheme || nextProps.cacheBusterProp !== this.props.cacheBusterProp ); diff --git a/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx b/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx index 60cbab7295303..2f3c556bf5d66 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx +++ b/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} diff --git a/superset-frontend/src/dashboard/containers/Chart.jsx b/superset-frontend/src/dashboard/containers/Chart.jsx index cb692225ea6d7..830f30638de7d 100644 --- a/superset-frontend/src/dashboard/containers/Chart.jsx +++ b/superset-frontend/src/dashboard/containers/Chart.jsx @@ -76,7 +76,6 @@ function mapStateToProps( sliceId: id, nativeFilters, dataMask, - labelColors: labelColors && Object.keys(labelColors).length && labelColors, }); formData.dashboardId = dashboardInfo.id; @@ -84,6 +83,7 @@ function mapStateToProps( return { chart, datasource, + labelColors: labelColors && Object.keys(labelColors).length && labelColors, slice: sliceEntities.slices[id], timeout: dashboardInfo.common.conf.SUPERSET_WEBSERVER_TIMEOUT, filters: getActiveFilters() || EMPTY_OBJECT, diff --git a/superset-frontend/src/dashboard/util/charts/getFormDataWithExtraFilters.ts b/superset-frontend/src/dashboard/util/charts/getFormDataWithExtraFilters.ts index 97ed851cc7079..cace8ddedf35d 100644 --- a/superset-frontend/src/dashboard/util/charts/getFormDataWithExtraFilters.ts +++ b/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'; @@ -62,10 +58,7 @@ export default function getFormDataWithExtraFilters({ sliceId, layout, dataMask, - labelColors, }: GetFormDataWithExtraFiltersArguments) { - const scale = CategoricalColorNamespace.getScale(colorScheme, colorNamespace); - const scaleLabelColors = labelColors || scale.getColorMap(); // if dashboard metadata + filters have not changed, use cache if possible const cachedFormData = cachedFormdataByChart[sliceId]; if ( @@ -76,9 +69,6 @@ export default function getFormDataWithExtraFilters({ areObjectsEqual(cachedFormData?.color_namespace, colorNamespace, { ignoreUndefined: true, }) && - areObjectsEqual(cachedFormData?.label_colors, labelColors, { - ignoreUndefined: true, - }) && !!cachedFormData && areObjectsEqual(cachedFormData?.dataMask, dataMask, { ignoreUndefined: true, @@ -110,7 +100,6 @@ export default function getFormDataWithExtraFilters({ const formData = { ...chart.formData, ...(colorScheme && { color_scheme: colorScheme }), - label_colors: scaleLabelColors, extra_filters: getEffectiveExtraFilters(filters), ...extraData, }; From 3f9ea8456327475b5f0bfc275d87115f11530e00 Mon Sep 17 00:00:00 2001 From: geido Date: Fri, 8 Oct 2021 16:29:34 +0000 Subject: [PATCH 6/6] Exclude label_colors from URL --- .../src/dashboard/containers/Chart.jsx | 1 + .../util/charts/getFormDataWithExtraFilters.ts | 14 +++++++++++++- .../src/explore/exploreUtils/index.js | 9 +++++++++ 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/superset-frontend/src/dashboard/containers/Chart.jsx b/superset-frontend/src/dashboard/containers/Chart.jsx index 830f30638de7d..ec80323bb2761 100644 --- a/superset-frontend/src/dashboard/containers/Chart.jsx +++ b/superset-frontend/src/dashboard/containers/Chart.jsx @@ -76,6 +76,7 @@ function mapStateToProps( sliceId: id, nativeFilters, dataMask, + labelColors, }); formData.dashboardId = dashboardInfo.id; diff --git a/superset-frontend/src/dashboard/util/charts/getFormDataWithExtraFilters.ts b/superset-frontend/src/dashboard/util/charts/getFormDataWithExtraFilters.ts index cace8ddedf35d..3e705ce90a25c 100644 --- a/superset-frontend/src/dashboard/util/charts/getFormDataWithExtraFilters.ts +++ b/superset-frontend/src/dashboard/util/charts/getFormDataWithExtraFilters.ts @@ -16,7 +16,11 @@ * specific language governing permissions and limitations * under the License. */ -import { DataRecordFilters, JsonObject } from '@superset-ui/core'; +import { + CategoricalColorNamespace, + 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'; @@ -58,7 +62,10 @@ export default function getFormDataWithExtraFilters({ sliceId, layout, dataMask, + labelColors, }: GetFormDataWithExtraFiltersArguments) { + const scale = CategoricalColorNamespace.getScale(colorScheme, colorNamespace); + const scaleLabelColors = labelColors || scale.getColorMap(); // if dashboard metadata + filters have not changed, use cache if possible const cachedFormData = cachedFormdataByChart[sliceId]; if ( @@ -69,6 +76,9 @@ export default function getFormDataWithExtraFilters({ areObjectsEqual(cachedFormData?.color_namespace, colorNamespace, { ignoreUndefined: true, }) && + areObjectsEqual(cachedFormData?.label_colors, scaleLabelColors, { + ignoreUndefined: true, + }) && !!cachedFormData && areObjectsEqual(cachedFormData?.dataMask, dataMask, { ignoreUndefined: true, @@ -99,10 +109,12 @@ export default function getFormDataWithExtraFilters({ const formData = { ...chart.formData, + label_colors: scaleLabelColors, ...(colorScheme && { color_scheme: colorScheme }), extra_filters: getEffectiveExtraFilters(filters), ...extraData, }; + cachedFiltersByChart[sliceId] = filters; cachedFormdataByChart[sliceId] = { ...formData, dataMask }; diff --git a/superset-frontend/src/explore/exploreUtils/index.js b/superset-frontend/src/explore/exploreUtils/index.js index ddcd95b82a437..d15ff372c9f4a 100644 --- a/superset-frontend/src/explore/exploreUtils/index.js +++ b/superset-frontend/src/explore/exploreUtils/index.js @@ -103,6 +103,10 @@ export function getExploreLongUrl( return null; } + // label_colors should not pollute the URL + // eslint-disable-next-line no-param-reassign + delete formData.label_colors; + const uri = new URI('/'); const directory = getURIDirectory(endpointType); const search = uri.search(true); @@ -159,6 +163,11 @@ export function getExploreUrl({ if (!formData.datasource) { return null; } + + // label_colors should not pollute the URL + // eslint-disable-next-line no-param-reassign + delete formData.label_colors; + let uri = getChartDataUri({ path: '/', allowDomainSharding }); if (curUrl) { uri = URI(URI(curUrl).search());