From e5e2c2a5dd84d407cbfd34bde8a36c9ef1e4a5e6 Mon Sep 17 00:00:00 2001 From: Alex Yang <82569366+alex241728@users.noreply.github.com> Date: Fri, 14 Nov 2025 14:37:44 -0500 Subject: [PATCH] feat: Floating Point Formatting for Scatter Point Chart (#35915) Co-authored-by: Vincent (cherry picked from commit 001b6cb801ab25bd81a397e8f9786fd44857151b) --- .../src/shared-controls/sharedControls.tsx | 26 +++ .../Regular/Scatter/controlPanel.tsx | 47 +++++ .../src/Timeseries/constants.ts | 1 + .../src/Timeseries/transformProps.ts | 5 +- .../src/Timeseries/types.ts | 1 + .../Timeseries/Scatter/controlPanel.test.ts | 156 +++++++++++++++ .../Timeseries/Scatter/transformProps.test.ts | 181 ++++++++++++++++++ 7 files changed, 416 insertions(+), 1 deletion(-) create mode 100644 superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/Scatter/controlPanel.test.ts create mode 100644 superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/Scatter/transformProps.test.ts diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/sharedControls.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/sharedControls.tsx index 30714f0abf5..30e7f9fcffe 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/sharedControls.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/sharedControls.tsx @@ -345,6 +345,31 @@ const x_axis_time_format: SharedControlConfig< option.label.includes(search) || option.value.includes(search), }; +const x_axis_number_format: SharedControlConfig< + 'SelectControl', + SelectDefaultOption +> = { + type: 'SelectControl', + freeForm: true, + label: t('X Axis Number Format'), + renderTrigger: true, + default: DEFAULT_NUMBER_FORMAT, + choices: D3_FORMAT_OPTIONS, + description: D3_FORMAT_DOCS, + tokenSeparators: ['\n', '\t', ';'], + filterOption: ({ data: option }, search) => + option.label.includes(search) || option.value.includes(search), + mapStateToProps: state => { + const isPercentage = + state.controls?.comparison_type?.value === ComparisonType.Percentage; + return { + choices: isPercentage + ? D3_FORMAT_OPTIONS.filter(option => option[0].includes('%')) + : D3_FORMAT_OPTIONS, + }; + }, +}; + const color_scheme: SharedControlConfig<'ColorSchemeControl'> = { type: 'ColorSchemeControl', label: t('Color Scheme'), @@ -459,6 +484,7 @@ const sharedControls: Record> = { size: dndSizeControl, y_axis_format, x_axis_time_format, + x_axis_number_format, adhoc_filters: dndAdhocFilterControl, color_scheme, time_shift_color, diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/Regular/Scatter/controlPanel.tsx b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/Regular/Scatter/controlPanel.tsx index bf23eadd61f..a081b850360 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/Regular/Scatter/controlPanel.tsx +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/Regular/Scatter/controlPanel.tsx @@ -112,6 +112,53 @@ const config: ControlPanelConfig = { ...sharedControls.x_axis_time_format, default: 'smart_date', description: `${D3_TIME_FORMAT_DOCS}. ${TIME_SERIES_DESCRIPTION_TEXT}`, + visibility: ({ controls }: ControlPanelsContainerProps) => { + // check if x axis is a time column + const xAxisColumn = controls?.x_axis?.value; + const xAxisOptions = controls?.x_axis?.options; + + if (!xAxisColumn || !Array.isArray(xAxisOptions)) { + return false; + } + + const xAxisType = xAxisOptions.find( + option => option.column_name === xAxisColumn, + )?.type; + + return ( + typeof xAxisType === 'string' && + xAxisType.toUpperCase().includes('TIME') + ); + }, + }, + }, + { + name: 'x_axis_number_format', + config: { + ...sharedControls.x_axis_number_format, + visibility: ({ controls }: ControlPanelsContainerProps) => { + // check if x axis is a floating-point column + const xAxisColumn = controls?.x_axis?.value; + const xAxisOptions = controls?.x_axis?.options; + + if (!xAxisColumn || !Array.isArray(xAxisOptions)) { + return false; + } + + const xAxisType = xAxisOptions.find( + option => option.column_name === xAxisColumn, + )?.type; + + if (typeof xAxisType !== 'string') { + return false; + } + + const typeUpper = xAxisType.toUpperCase(); + + return ['FLOAT', 'DOUBLE', 'REAL', 'NUMERIC', 'DECIMAL'].some( + t => typeUpper.includes(t), + ); + }, }, }, ], diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/constants.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/constants.ts index 6378da8f2ea..5aac81dc7f1 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/constants.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/constants.ts @@ -72,6 +72,7 @@ export const DEFAULT_FORM_DATA: EchartsTimeseriesFormData = { stack: false, tooltipTimeFormat: 'smart_date', xAxisTimeFormat: 'smart_date', + xAxisNumberFormat: 'SMART_NUMBER', truncateXAxis: true, truncateYAxis: false, yAxisBounds: [null, null], diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts index 0bbfedaab0e..67162d28a1f 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts @@ -196,6 +196,7 @@ export default function transformProps( xAxisSort, xAxisSortAsc, xAxisTimeFormat, + xAxisNumberFormat, xAxisTitle, xAxisTitleMargin, yAxisBounds, @@ -560,7 +561,9 @@ export default function transformProps( const xAxisFormatter = xAxisDataType === GenericDataType.Temporal ? getXAxisFormatter(xAxisTimeFormat) - : String; + : xAxisDataType === GenericDataType.Numeric + ? getNumberFormatter(xAxisNumberFormat) + : String; const { setDataMask = () => {}, diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/types.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/types.ts index 72ea97019ef..49b764e6a03 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/types.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/types.ts @@ -84,6 +84,7 @@ export type EchartsTimeseriesFormData = QueryFormData & { yAxisFormat?: string; xAxisForceCategorical?: boolean; xAxisTimeFormat?: string; + xAxisNumberFormat?: string; timeGrainSqla?: TimeGranularity; forceMaxInterval?: boolean; xAxisBounds: [number | undefined | null, number | undefined | null]; diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/Scatter/controlPanel.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/Scatter/controlPanel.test.ts new file mode 100644 index 00000000000..6dd0fc91c80 --- /dev/null +++ b/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/Scatter/controlPanel.test.ts @@ -0,0 +1,156 @@ +/** + * 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 { ControlPanelsContainerProps } from '@superset-ui/chart-controls'; +import controlPanel from '../../../src/Timeseries/Regular/Scatter/controlPanel'; + +const config = controlPanel; + +const getControl = (controlName: string) => { + for (const section of config.controlPanelSections) { + if (section && section.controlSetRows) { + for (const row of section.controlSetRows) { + for (const control of row) { + if ( + typeof control === 'object' && + control !== null && + 'name' in control && + control.name === controlName + ) { + return control; + } + } + } + } + } + + return null; +}; + +const mockControls = ( + xAxisColumn: string | null, + xAxisType: string | null, +): ControlPanelsContainerProps => { + const options = xAxisType + ? [{ column_name: xAxisColumn, type: xAxisType }] + : []; + + return { + controls: { + // @ts-ignore + x_axis: { + value: xAxisColumn, + options, + }, + }, + }; +}; + +// tests for x_axis_time_format control +const timeFormatControl: any = getControl('x_axis_time_format'); + +test('scatter chart control panel should include x_axis_time_format control in the panel', () => { + expect(timeFormatControl).toBeDefined(); +}); + +test('scatter chart control panel should have correct default value for x_axis_time_format', () => { + expect(timeFormatControl).toBeDefined(); + expect(timeFormatControl.config).toBeDefined(); + expect(timeFormatControl.config.default).toBe('smart_date'); +}); + +test('scatter chart control panel should have visibility function for x_axis_time_format', () => { + expect(timeFormatControl).toBeDefined(); + expect(timeFormatControl.config.visibility).toBeDefined(); + expect(typeof timeFormatControl.config.visibility).toBe('function'); + + // The visibility function exists - the exact logic is tested implicitly through UI behavior + // The important part is that the control has proper visibility configuration +}); + +const isTimeVisible = ( + xAxisColumn: string | null, + xAxisType: string | null, +): boolean => { + const props = mockControls(xAxisColumn, xAxisType); + const visibilityFn = timeFormatControl?.config?.visibility; + return visibilityFn ? visibilityFn(props) : false; +}; + +test('x_axis_time_format control should be visible for any data types include TIME', () => { + expect(isTimeVisible('time_column', 'TIME')).toBe(true); + expect(isTimeVisible('time_column', 'TIME WITH TIME ZONE')).toBe(true); + expect(isTimeVisible('time_column', 'TIMESTAMP WITH TIME ZONE')).toBe(true); + expect(isTimeVisible('time_column', 'TIMESTAMP WITHOUT TIME ZONE')).toBe( + true, + ); +}); + +test('x_axis_time_format control should be hidden for data types that do NOT include TIME', () => { + expect(isTimeVisible('null', 'null')).toBe(false); + expect(isTimeVisible(null, null)).toBe(false); + expect(isTimeVisible('float_column', 'FLOAT')).toBe(false); +}); + +// tests for x_axis_number_format control +const numberFormatControl: any = getControl('x_axis_number_format'); + +test('scatter chart control panel should include x_axis_number_format control in the panel', () => { + expect(numberFormatControl).toBeDefined(); +}); + +test('scatter chart control panel should have correct default value for x_axis_number_format', () => { + expect(numberFormatControl).toBeDefined(); + expect(numberFormatControl.config).toBeDefined(); + expect(numberFormatControl.config.default).toBe('SMART_NUMBER'); +}); + +test('scatter chart control panel should have visibility function for x_axis_number_format', () => { + expect(numberFormatControl).toBeDefined(); + expect(numberFormatControl.config.visibility).toBeDefined(); + expect(typeof numberFormatControl.config.visibility).toBe('function'); + + // The visibility function exists - the exact logic is tested implicitly through UI behavior + // The important part is that the control has proper visibility configuration +}); + +const isNumberVisible = ( + xAxisColumn: string | null, + xAxisType: string | null, +): boolean => { + const props = mockControls(xAxisColumn, xAxisType); + const visibilityFn = numberFormatControl?.config?.visibility; + return visibilityFn ? visibilityFn(props) : false; +}; + +test('x_axis_number_format control should be visible for any floating-point data types', () => { + expect(isNumberVisible('float_column', 'FLOAT')).toBe(true); + expect(isNumberVisible('double_column', 'DOUBLE')).toBe(true); + expect(isNumberVisible('real_column', 'REAL')).toBe(true); + expect(isNumberVisible('numeric_column', 'NUMERIC')).toBe(true); + expect(isNumberVisible('decimal_column', 'DECIMAL')).toBe(true); +}); + +test('x_axis_number_format control should be hidden for any non-floating-point data types', () => { + expect(isNumberVisible('string_column', 'VARCHAR')).toBe(false); + expect(isNumberVisible('null', 'null')).toBe(false); + expect(isNumberVisible(null, null)).toBe(false); + expect(isNumberVisible('time_column', 'TIMESTAMP WITHOUT TIME ZONE')).toBe( + false, + ); +}); diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/Scatter/transformProps.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/Scatter/transformProps.test.ts new file mode 100644 index 00000000000..8aa22f49b9a --- /dev/null +++ b/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/Scatter/transformProps.test.ts @@ -0,0 +1,181 @@ +/** + * 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 { + ChartProps, + SMART_DATE_ID, + GenericDataType, + supersetTheme, +} from '@superset-ui/core'; +import { + D3_FORMAT_OPTIONS, + D3_TIME_FORMAT_OPTIONS, +} from '@superset-ui/chart-controls'; +import transformProps from '../../../src/Timeseries/transformProps'; +import { DEFAULT_FORM_DATA } from '../../../src/Timeseries/constants'; +import { + EchartsTimeseriesSeriesType, + EchartsTimeseriesFormData, + EchartsTimeseriesChartProps, +} from '../../../src/Timeseries/types'; + +describe('Scatter Chart X-axis Time Formatting', () => { + const baseFormData: EchartsTimeseriesFormData = { + ...DEFAULT_FORM_DATA, + colorScheme: 'supersetColors', + datasource: '1__table', + granularity_sqla: '__timestamp', + metric: ['column 1'], + groupby: [], + viz_type: 'echarts_timeseries_scatter', + seriesType: EchartsTimeseriesSeriesType.Scatter, + }; + + const timeseriesData = [ + { + data: [ + { column_1: 0.72099, __timestamp: 1609459200000 }, + { column_1: 0.77954, __timestamp: 1612137600000 }, + { column_1: 2.83434, __timestamp: 1614556800000 }, + ], + colnames: ['column_1', '__timestamp'], + coltypes: [GenericDataType.Numeric, GenericDataType.Temporal], + }, + ]; + + const baseChartPropsConfig = { + width: 800, + height: 600, + queriesData: timeseriesData, + theme: supersetTheme, + }; + + test('xAxisTimeFormat has no default formatter', () => { + const chartProps = new ChartProps({ + ...baseChartPropsConfig, + formData: baseFormData, + }); + + const transformedProps = transformProps( + // @ts-ignore + chartProps as EchartsTimeseriesChartProps, + ); + + expect(transformedProps.echartOptions.xAxis).toHaveProperty('axisLabel'); + const xAxis = transformedProps.echartOptions.xAxis as any; + expect(xAxis.axisLabel).toHaveProperty('formatter'); + expect(xAxis.axisLabel.formatter).toBeUndefined(); + }); + + test.each( + D3_TIME_FORMAT_OPTIONS.map(([id]) => id).filter(id => id !== SMART_DATE_ID), + )('should handle %s format', format => { + const chartProps = new ChartProps({ + ...baseChartPropsConfig, + formData: { + ...baseFormData, + xAxisTimeFormat: format, + }, + }); + + const transformedProps = transformProps( + // @ts-ignore + chartProps as EchartsTimeseriesChartProps, + ); + + const xAxis = transformedProps.echartOptions.xAxis as any; + expect(xAxis.axisLabel).toHaveProperty('formatter'); + expect(typeof xAxis.axisLabel.formatter).toBe('function'); + expect(xAxis.axisLabel.formatter.id).toBe(format); + }); +}); + +describe('Scatter Chart X-axis Number Formatting', () => { + const baseFormData: EchartsTimeseriesFormData = { + ...DEFAULT_FORM_DATA, + colorScheme: 'supersetColors', + datasource: '1__table', + metric: ['column_1'], + x_axis: 'column_2', + groupby: [], + viz_type: 'echarts_timeseries_scatter', + seriesType: EchartsTimeseriesSeriesType.Scatter, + }; + + const timeseriesData = [ + { + data: [ + { column_1: 0.72099, column_2: 3.01699 }, + { column_1: 0.77954, column_2: 3.44802 }, + { column_1: 2.83434, column_2: 3.58095 }, + ], + colnames: ['column_1', 'column_2'], + coltypes: [GenericDataType.Numeric, GenericDataType.Numeric], + }, + ]; + + const baseChartPropsConfig = { + width: 800, + height: 600, + queriesData: timeseriesData, + theme: supersetTheme, + }; + + test('should use SMART_NUMBER as default xAxisNumberFormat', () => { + const chartProps = new ChartProps({ + ...baseChartPropsConfig, + formData: baseFormData, + }); + + const transformedProps = transformProps( + // @ts-ignore + chartProps as EchartsTimeseriesChartProps, + ); + + expect(transformedProps.echartOptions.xAxis).toHaveProperty('axisLabel'); + const xAxis = transformedProps.echartOptions.xAxis as any; + expect(xAxis.axisLabel).toHaveProperty('formatter'); + expect(typeof xAxis.axisLabel.formatter).toBe('function'); + expect(xAxis.axisLabel.formatter.id).toBe('SMART_NUMBER'); + }); + + test.each(D3_FORMAT_OPTIONS.map(([id]) => id))( + 'should handle %s format', + format => { + const chartProps = new ChartProps({ + ...baseChartPropsConfig, + formData: { + ...baseFormData, + xAxisNumberFormat: format, + }, + }); + + const transformedProps = transformProps( + // @ts-ignore + chartProps as EchartsTimeseriesChartProps, + ); + + expect(transformedProps.echartOptions.xAxis).toHaveProperty('axisLabel'); + const xAxis = transformedProps.echartOptions.xAxis as any; + expect(xAxis.axisLabel).toHaveProperty('formatter'); + expect(typeof xAxis.axisLabel.formatter).toBe('function'); + expect(xAxis.axisLabel.formatter.id).toBe(format); + }, + ); +});