This is an automated email from the ASF dual-hosted git repository.
enzomartellucci pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 3347b9bf6c fix(table): only show increase/decrease color options when
time comparison enabled (#37362)
3347b9bf6c is described below
commit 3347b9bf6cfcf4525f2af283c668f4d36febc33f
Author: Jamile Celento <[email protected]>
AuthorDate: Wed Jan 28 09:32:30 2026 -0300
fix(table): only show increase/decrease color options when time comparison
enabled (#37362)
---
.../src/controlPanel.tsx | 33 ++--
.../test/controlPanel.test.tsx | 208 +++++++++++++++++++++
.../plugin-chart-table/src/controlPanel.tsx | 35 ++--
.../plugin-chart-table/test/controlPanel.test.tsx | 208 +++++++++++++++++++++
4 files changed, 455 insertions(+), 29 deletions(-)
diff --git
a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/controlPanel.tsx
b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/controlPanel.tsx
index 50d40f9429..1277a8f7f3 100644
--- a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/controlPanel.tsx
+++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/controlPanel.tsx
@@ -683,16 +683,6 @@ const config: ControlPanelConfig = {
type: 'ConditionalFormattingControl',
renderTrigger: true,
label: t('Custom conditional formatting'),
- extraColorChoices: [
- {
- value: ColorSchemeEnum.Green,
- label: t('Green for increase, red for decrease'),
- },
- {
- value: ColorSchemeEnum.Red,
- label: t('Red for increase, green for decrease'),
- },
- ],
description: t(
'Apply conditional color formatting to numeric columns',
),
@@ -705,6 +695,22 @@ const config: ControlPanelConfig = {
)
? (explore?.datasource as Dataset)?.verbose_map
: (explore?.datasource?.columns ?? {});
+ const timeCompareValue =
explore?.controls?.time_compare?.value;
+ const hasTimeComparison = !isEmpty(timeCompareValue);
+
+ const extraColorChoices = hasTimeComparison
+ ? [
+ {
+ value: ColorSchemeEnum.Green,
+ label: t('Green for increase, red for decrease'),
+ },
+ {
+ value: ColorSchemeEnum.Red,
+ label: t('Red for increase, green for decrease'),
+ },
+ ]
+ : [];
+
const chartStatus = chart?.chartStatus;
const { colnames, coltypes } =
chart?.queriesResponse?.[0] ?? {};
@@ -724,12 +730,10 @@ const config: ControlPanelConfig = {
colnames && coltypes[colnames?.indexOf(colname)],
}))
: [];
- const columnOptions = explore?.controls?.time_compare?.value
+ const columnOptions = hasTimeComparison
? processComparisonColumns(
numericColumns || [],
- ensureIsArray(
- explore?.controls?.time_compare?.value,
- )[0]?.toString() || '',
+ ensureIsArray(timeCompareValue)[0]?.toString() || '',
)
: numericColumns;
@@ -737,6 +741,7 @@ const config: ControlPanelConfig = {
removeIrrelevantConditions: chartStatus === 'success',
columnOptions,
verboseMap,
+ extraColorChoices,
};
},
},
diff --git
a/superset-frontend/plugins/plugin-chart-ag-grid-table/test/controlPanel.test.tsx
b/superset-frontend/plugins/plugin-chart-ag-grid-table/test/controlPanel.test.tsx
new file mode 100644
index 0000000000..70501b17bd
--- /dev/null
+++
b/superset-frontend/plugins/plugin-chart-ag-grid-table/test/controlPanel.test.tsx
@@ -0,0 +1,208 @@
+/**
+ * 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 { GenericDataType } from '@apache-superset/core/api/core';
+import { QueryFormData } from '@superset-ui/core';
+import {
+ Dataset,
+ isCustomControlItem,
+ ControlConfig,
+ ControlPanelState,
+ ControlState,
+} from '@superset-ui/chart-controls';
+import config from '../src/controlPanel';
+import { ColorSchemeEnum } from '../src/types';
+
+const findConditionalFormattingControl = (): ControlConfig | null => {
+ for (const section of config.controlPanelSections) {
+ if (!section) continue;
+ for (const row of section.controlSetRows) {
+ for (const control of row) {
+ if (
+ isCustomControlItem(control) &&
+ control.name === 'conditional_formatting'
+ ) {
+ return control.config;
+ }
+ }
+ }
+ }
+ return null;
+};
+
+const createMockControlState = (value: string[] | undefined): ControlState =>
({
+ type: 'SelectControl',
+ value,
+ label: '',
+ default: undefined,
+ renderTrigger: false,
+});
+
+const createMockExplore = (
+ timeCompareValue: string[] | undefined,
+): ControlPanelState => ({
+ slice: { slice_id: 123 },
+ datasource: {
+ verbose_map: { col1: 'Column 1', col2: 'Column 2' },
+ columns: [],
+ } as Partial<Dataset> as Dataset,
+ controls: {
+ time_compare: createMockControlState(timeCompareValue),
+ },
+ form_data: {
+ time_compare: timeCompareValue,
+ datasource: 'test',
+ viz_type: 'table',
+ } as QueryFormData,
+ common: {},
+ metadata: {},
+});
+
+const createMockChart = () => ({
+ chartStatus: 'success' as const,
+ queriesResponse: [
+ {
+ colnames: ['col1', 'col2'],
+ coltypes: [GenericDataType.Numeric, GenericDataType.Numeric],
+ },
+ ],
+});
+
+const createMockControlStateForConditionalFormatting = (): ControlState => ({
+ type: 'CollectionControl',
+ value: [],
+ label: '',
+ default: undefined,
+ renderTrigger: false,
+});
+
+test('extraColorChoices not included when time comparison is disabled', () => {
+ const controlConfig = findConditionalFormattingControl();
+ expect(controlConfig).toBeTruthy();
+ expect(controlConfig?.mapStateToProps).toBeTruthy();
+
+ const explore = createMockExplore(undefined);
+ const chart = createMockChart();
+ const result = controlConfig!.mapStateToProps!(
+ explore,
+ createMockControlStateForConditionalFormatting(),
+ chart,
+ );
+
+ expect(result.extraColorChoices).toEqual([]);
+ expect(result.columnOptions).toEqual(
+ expect.arrayContaining([
+ expect.objectContaining({ value: 'col1' }),
+ expect.objectContaining({ value: 'col2' }),
+ ]),
+ );
+});
+
+test('extraColorChoices included when time comparison is enabled', () => {
+ const controlConfig = findConditionalFormattingControl();
+ expect(controlConfig).toBeTruthy();
+
+ const explore = createMockExplore(['P1D']);
+ const chart = createMockChart();
+ const result = controlConfig!.mapStateToProps!(
+ explore,
+ createMockControlStateForConditionalFormatting(),
+ chart,
+ );
+
+ expect(result.extraColorChoices).toEqual([
+ {
+ value: ColorSchemeEnum.Green,
+ label: expect.stringContaining('Green for increase'),
+ },
+ {
+ value: ColorSchemeEnum.Red,
+ label: expect.stringContaining('Red for increase'),
+ },
+ ]);
+ expect(result.columnOptions).not.toEqual(
+ expect.arrayContaining([expect.objectContaining({ value: 'col1' })]),
+ );
+});
+
+test('extraColorChoices not included when time_compare is empty array', () => {
+ const controlConfig = findConditionalFormattingControl();
+ expect(controlConfig).toBeTruthy();
+
+ const explore = createMockExplore([]);
+ const chart = createMockChart();
+ const result = controlConfig!.mapStateToProps!(
+ explore,
+ createMockControlStateForConditionalFormatting(),
+ chart,
+ );
+
+ expect(result.extraColorChoices).toEqual([]);
+});
+
+test('consistency between extraColorChoices and columnOptions', () => {
+ const controlConfig = findConditionalFormattingControl();
+ expect(controlConfig).toBeTruthy();
+
+ const explore = createMockExplore(['P1D']);
+ const chart = createMockChart();
+ const result = controlConfig!.mapStateToProps!(
+ explore,
+ createMockControlStateForConditionalFormatting(),
+ chart,
+ );
+
+ const hasExtraColorChoices = result.extraColorChoices.length > 0;
+ const hasComparisonColumns = result.columnOptions.some(
+ (col: { value: string }) =>
+ col.value.includes('Main') ||
+ col.value.includes('#') ||
+ col.value.includes('△'),
+ );
+
+ expect(hasExtraColorChoices).toBe(true);
+ expect(hasComparisonColumns).toBe(true);
+});
+
+test('uses controls.time_compare.value not form_data.time_compare', () => {
+ const controlConfig = findConditionalFormattingControl();
+ expect(controlConfig).toBeTruthy();
+
+ const explore: ControlPanelState = {
+ ...createMockExplore(undefined),
+ form_data: {
+ ...createMockExplore(undefined).form_data,
+ time_compare: ['P1D'],
+ },
+ };
+ const chart = createMockChart();
+ const result = controlConfig!.mapStateToProps!(
+ explore,
+ createMockControlStateForConditionalFormatting(),
+ chart,
+ );
+
+ expect(result.extraColorChoices).toEqual([]);
+});
+
+test('static extraColorChoices removed from config', () => {
+ const controlConfig = findConditionalFormattingControl();
+ expect(controlConfig).toBeTruthy();
+
+ expect(controlConfig?.extraColorChoices).toBeUndefined();
+});
diff --git a/superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx
b/superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx
index cdff93eb8e..2d051cf97c 100644
--- a/superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx
+++ b/superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx
@@ -746,16 +746,6 @@ const config: ControlPanelConfig = {
type: 'ConditionalFormattingControl',
renderTrigger: true,
label: t('Custom conditional formatting'),
- extraColorChoices: [
- {
- value: ColorSchemeEnum.Green,
- label: t('Green for increase, red for decrease'),
- },
- {
- value: ColorSchemeEnum.Red,
- label: t('Red for increase, green for decrease'),
- },
- ],
description: t(
'Apply conditional color formatting to numeric columns',
),
@@ -768,6 +758,22 @@ const config: ControlPanelConfig = {
)
? (explore?.datasource as Dataset)?.verbose_map
: (explore?.datasource?.columns ?? {});
+ const timeCompareValue =
explore?.controls?.time_compare?.value;
+ const hasTimeComparison = !isEmpty(timeCompareValue);
+
+ const extraColorChoices = hasTimeComparison
+ ? [
+ {
+ value: ColorSchemeEnum.Green,
+ label: t('Green for increase, red for decrease'),
+ },
+ {
+ value: ColorSchemeEnum.Red,
+ label: t('Red for increase, green for decrease'),
+ },
+ ]
+ : [];
+
const chartStatus = chart?.chartStatus;
const value = _?.value ?? [];
if (value && Array.isArray(value)) {
@@ -796,7 +802,7 @@ const config: ControlPanelConfig = {
? colnames.reduce((acc, colname, index) => {
if (
coltypes[index] === GenericDataType.Numeric ||
- (!explore?.controls?.time_compare?.value &&
+ (!hasTimeComparison &&
(coltypes[index] === GenericDataType.String ||
coltypes[index] === GenericDataType.Boolean))
) {
@@ -811,12 +817,10 @@ const config: ControlPanelConfig = {
return acc;
}, [])
: [];
- const columnOptions = explore?.controls?.time_compare?.value
+ const columnOptions = hasTimeComparison
? processComparisonColumns(
numericColumns || [],
- ensureIsArray(
- explore?.controls?.time_compare?.value,
- )[0]?.toString() || '',
+ ensureIsArray(timeCompareValue)[0]?.toString() || '',
)
: numericColumns;
@@ -828,6 +832,7 @@ const config: ControlPanelConfig = {
toAllRowCheck: true,
toColorTextCheck: true,
},
+ extraColorChoices,
};
},
},
diff --git
a/superset-frontend/plugins/plugin-chart-table/test/controlPanel.test.tsx
b/superset-frontend/plugins/plugin-chart-table/test/controlPanel.test.tsx
new file mode 100644
index 0000000000..70501b17bd
--- /dev/null
+++ b/superset-frontend/plugins/plugin-chart-table/test/controlPanel.test.tsx
@@ -0,0 +1,208 @@
+/**
+ * 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 { GenericDataType } from '@apache-superset/core/api/core';
+import { QueryFormData } from '@superset-ui/core';
+import {
+ Dataset,
+ isCustomControlItem,
+ ControlConfig,
+ ControlPanelState,
+ ControlState,
+} from '@superset-ui/chart-controls';
+import config from '../src/controlPanel';
+import { ColorSchemeEnum } from '../src/types';
+
+const findConditionalFormattingControl = (): ControlConfig | null => {
+ for (const section of config.controlPanelSections) {
+ if (!section) continue;
+ for (const row of section.controlSetRows) {
+ for (const control of row) {
+ if (
+ isCustomControlItem(control) &&
+ control.name === 'conditional_formatting'
+ ) {
+ return control.config;
+ }
+ }
+ }
+ }
+ return null;
+};
+
+const createMockControlState = (value: string[] | undefined): ControlState =>
({
+ type: 'SelectControl',
+ value,
+ label: '',
+ default: undefined,
+ renderTrigger: false,
+});
+
+const createMockExplore = (
+ timeCompareValue: string[] | undefined,
+): ControlPanelState => ({
+ slice: { slice_id: 123 },
+ datasource: {
+ verbose_map: { col1: 'Column 1', col2: 'Column 2' },
+ columns: [],
+ } as Partial<Dataset> as Dataset,
+ controls: {
+ time_compare: createMockControlState(timeCompareValue),
+ },
+ form_data: {
+ time_compare: timeCompareValue,
+ datasource: 'test',
+ viz_type: 'table',
+ } as QueryFormData,
+ common: {},
+ metadata: {},
+});
+
+const createMockChart = () => ({
+ chartStatus: 'success' as const,
+ queriesResponse: [
+ {
+ colnames: ['col1', 'col2'],
+ coltypes: [GenericDataType.Numeric, GenericDataType.Numeric],
+ },
+ ],
+});
+
+const createMockControlStateForConditionalFormatting = (): ControlState => ({
+ type: 'CollectionControl',
+ value: [],
+ label: '',
+ default: undefined,
+ renderTrigger: false,
+});
+
+test('extraColorChoices not included when time comparison is disabled', () => {
+ const controlConfig = findConditionalFormattingControl();
+ expect(controlConfig).toBeTruthy();
+ expect(controlConfig?.mapStateToProps).toBeTruthy();
+
+ const explore = createMockExplore(undefined);
+ const chart = createMockChart();
+ const result = controlConfig!.mapStateToProps!(
+ explore,
+ createMockControlStateForConditionalFormatting(),
+ chart,
+ );
+
+ expect(result.extraColorChoices).toEqual([]);
+ expect(result.columnOptions).toEqual(
+ expect.arrayContaining([
+ expect.objectContaining({ value: 'col1' }),
+ expect.objectContaining({ value: 'col2' }),
+ ]),
+ );
+});
+
+test('extraColorChoices included when time comparison is enabled', () => {
+ const controlConfig = findConditionalFormattingControl();
+ expect(controlConfig).toBeTruthy();
+
+ const explore = createMockExplore(['P1D']);
+ const chart = createMockChart();
+ const result = controlConfig!.mapStateToProps!(
+ explore,
+ createMockControlStateForConditionalFormatting(),
+ chart,
+ );
+
+ expect(result.extraColorChoices).toEqual([
+ {
+ value: ColorSchemeEnum.Green,
+ label: expect.stringContaining('Green for increase'),
+ },
+ {
+ value: ColorSchemeEnum.Red,
+ label: expect.stringContaining('Red for increase'),
+ },
+ ]);
+ expect(result.columnOptions).not.toEqual(
+ expect.arrayContaining([expect.objectContaining({ value: 'col1' })]),
+ );
+});
+
+test('extraColorChoices not included when time_compare is empty array', () => {
+ const controlConfig = findConditionalFormattingControl();
+ expect(controlConfig).toBeTruthy();
+
+ const explore = createMockExplore([]);
+ const chart = createMockChart();
+ const result = controlConfig!.mapStateToProps!(
+ explore,
+ createMockControlStateForConditionalFormatting(),
+ chart,
+ );
+
+ expect(result.extraColorChoices).toEqual([]);
+});
+
+test('consistency between extraColorChoices and columnOptions', () => {
+ const controlConfig = findConditionalFormattingControl();
+ expect(controlConfig).toBeTruthy();
+
+ const explore = createMockExplore(['P1D']);
+ const chart = createMockChart();
+ const result = controlConfig!.mapStateToProps!(
+ explore,
+ createMockControlStateForConditionalFormatting(),
+ chart,
+ );
+
+ const hasExtraColorChoices = result.extraColorChoices.length > 0;
+ const hasComparisonColumns = result.columnOptions.some(
+ (col: { value: string }) =>
+ col.value.includes('Main') ||
+ col.value.includes('#') ||
+ col.value.includes('△'),
+ );
+
+ expect(hasExtraColorChoices).toBe(true);
+ expect(hasComparisonColumns).toBe(true);
+});
+
+test('uses controls.time_compare.value not form_data.time_compare', () => {
+ const controlConfig = findConditionalFormattingControl();
+ expect(controlConfig).toBeTruthy();
+
+ const explore: ControlPanelState = {
+ ...createMockExplore(undefined),
+ form_data: {
+ ...createMockExplore(undefined).form_data,
+ time_compare: ['P1D'],
+ },
+ };
+ const chart = createMockChart();
+ const result = controlConfig!.mapStateToProps!(
+ explore,
+ createMockControlStateForConditionalFormatting(),
+ chart,
+ );
+
+ expect(result.extraColorChoices).toEqual([]);
+});
+
+test('static extraColorChoices removed from config', () => {
+ const controlConfig = findConditionalFormattingControl();
+ expect(controlConfig).toBeTruthy();
+
+ expect(controlConfig?.extraColorChoices).toBeUndefined();
+});
