Skip to content

Commit fde4c47

Browse files
alex241728VincentTrung
authored andcommitted
feat: Floating Point Formatting for Scatter Point Chart (apache#35915)
Co-authored-by: Vincent <vincent.trung4@gmail.com>
1 parent 0be143c commit fde4c47

File tree

7 files changed

+418
-1
lines changed

7 files changed

+418
-1
lines changed

superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/sharedControls.tsx

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,31 @@ const x_axis_time_format: SharedControlConfig<
342342
option.label.includes(search) || option.value.includes(search),
343343
};
344344

345+
const x_axis_number_format: SharedControlConfig<
346+
'SelectControl',
347+
SelectDefaultOption
348+
> = {
349+
type: 'SelectControl',
350+
freeForm: true,
351+
label: t('X Axis Number Format'),
352+
renderTrigger: true,
353+
default: DEFAULT_NUMBER_FORMAT,
354+
choices: D3_FORMAT_OPTIONS,
355+
description: D3_FORMAT_DOCS,
356+
tokenSeparators: ['\n', '\t', ';'],
357+
filterOption: ({ data: option }, search) =>
358+
option.label.includes(search) || option.value.includes(search),
359+
mapStateToProps: state => {
360+
const isPercentage =
361+
state.controls?.comparison_type?.value === ComparisonType.Percentage;
362+
return {
363+
choices: isPercentage
364+
? D3_FORMAT_OPTIONS.filter(option => option[0].includes('%'))
365+
: D3_FORMAT_OPTIONS,
366+
};
367+
},
368+
};
369+
345370
const color_scheme: SharedControlConfig<'ColorSchemeControl'> = {
346371
type: 'ColorSchemeControl',
347372
label: t('Color Scheme'),
@@ -456,6 +481,7 @@ const sharedControls: Record<string, SharedControlConfig<any>> = {
456481
size: dndSizeControl,
457482
y_axis_format,
458483
x_axis_time_format,
484+
x_axis_number_format,
459485
adhoc_filters: dndAdhocFilterControl,
460486
color_scheme,
461487
time_shift_color,

superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/Regular/Scatter/controlPanel.tsx

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,53 @@ const config: ControlPanelConfig = {
112112
...sharedControls.x_axis_time_format,
113113
default: 'smart_date',
114114
description: `${D3_TIME_FORMAT_DOCS}. ${TIME_SERIES_DESCRIPTION_TEXT}`,
115+
visibility: ({ controls }: ControlPanelsContainerProps) => {
116+
// check if x axis is a time column
117+
const xAxisColumn = controls?.x_axis?.value;
118+
const xAxisOptions = controls?.x_axis?.options;
119+
120+
if (!xAxisColumn || !Array.isArray(xAxisOptions)) {
121+
return false;
122+
}
123+
124+
const xAxisType = xAxisOptions.find(
125+
option => option.column_name === xAxisColumn,
126+
)?.type;
127+
128+
return (
129+
typeof xAxisType === 'string' &&
130+
xAxisType.toUpperCase().includes('TIME')
131+
);
132+
},
133+
},
134+
},
135+
{
136+
name: 'x_axis_number_format',
137+
config: {
138+
...sharedControls.x_axis_number_format,
139+
visibility: ({ controls }: ControlPanelsContainerProps) => {
140+
// check if x axis is a floating-point column
141+
const xAxisColumn = controls?.x_axis?.value;
142+
const xAxisOptions = controls?.x_axis?.options;
143+
144+
if (!xAxisColumn || !Array.isArray(xAxisOptions)) {
145+
return false;
146+
}
147+
148+
const xAxisType = xAxisOptions.find(
149+
option => option.column_name === xAxisColumn,
150+
)?.type;
151+
152+
if (typeof xAxisType !== 'string') {
153+
return false;
154+
}
155+
156+
const typeUpper = xAxisType.toUpperCase();
157+
158+
return ['FLOAT', 'DOUBLE', 'REAL', 'NUMERIC', 'DECIMAL'].some(
159+
t => typeUpper.includes(t),
160+
);
161+
},
115162
},
116163
},
117164
],

superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/constants.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ export const DEFAULT_FORM_DATA: EchartsTimeseriesFormData = {
7272
stack: false,
7373
tooltipTimeFormat: 'smart_date',
7474
xAxisTimeFormat: 'smart_date',
75+
xAxisNumberFormat: 'SMART_NUMBER',
7576
truncateXAxis: true,
7677
truncateYAxis: false,
7778
yAxisBounds: [null, null],

superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,7 @@ export default function transformProps(
189189
xAxisSort,
190190
xAxisSortAsc,
191191
xAxisTimeFormat,
192+
xAxisNumberFormat,
192193
xAxisTitle,
193194
xAxisTitleMargin,
194195
yAxisBounds,
@@ -485,7 +486,9 @@ export default function transformProps(
485486
const xAxisFormatter =
486487
xAxisDataType === GenericDataType.Temporal
487488
? getXAxisFormatter(xAxisTimeFormat)
488-
: String;
489+
: xAxisDataType === GenericDataType.Numeric
490+
? getNumberFormatter(xAxisNumberFormat)
491+
: String;
489492

490493
const {
491494
setDataMask = () => {},

superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ export type EchartsTimeseriesFormData = QueryFormData & {
8484
yAxisFormat?: string;
8585
xAxisForceCategorical?: boolean;
8686
xAxisTimeFormat?: string;
87+
xAxisNumberFormat?: string;
8788
timeGrainSqla?: TimeGranularity;
8889
forceMaxInterval?: boolean;
8990
xAxisBounds: [number | undefined | null, number | undefined | null];
Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
/**
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
import { ControlPanelsContainerProps } from '@superset-ui/chart-controls/types';
20+
import controlPanel from '../../../src/Timeseries/Regular/Scatter/controlPanel';
21+
22+
const config = controlPanel;
23+
24+
const getControl = (controlName: string) => {
25+
for (const section of config.controlPanelSections) {
26+
if (section && section.controlSetRows) {
27+
for (const row of section.controlSetRows) {
28+
for (const control of row) {
29+
if (
30+
typeof control === 'object' &&
31+
control !== null &&
32+
'name' in control &&
33+
control.name === controlName
34+
) {
35+
return control;
36+
}
37+
}
38+
}
39+
}
40+
}
41+
42+
return null;
43+
};
44+
45+
const mockControls = (
46+
xAxisColumn: string | null,
47+
xAxisType: string | null,
48+
): ControlPanelsContainerProps => {
49+
const options = xAxisType
50+
? [{ column_name: xAxisColumn, type: xAxisType }]
51+
: [];
52+
53+
return {
54+
controls: {
55+
// @ts-ignore
56+
x_axis: {
57+
value: xAxisColumn,
58+
options: options,
59+
},
60+
},
61+
};
62+
};
63+
64+
// tests for x_axis_time_format control
65+
const timeFormatControl: any = getControl('x_axis_time_format');
66+
67+
test('scatter chart control panel should include x_axis_time_format control in the panel', () => {
68+
expect(timeFormatControl).toBeDefined();
69+
});
70+
71+
test('scatter chart control panel should have correct default value for x_axis_time_format', () => {
72+
expect(timeFormatControl).toBeDefined();
73+
expect(timeFormatControl.config).toBeDefined();
74+
expect(timeFormatControl.config.default).toBe('smart_date');
75+
});
76+
77+
test('scatter chart control panel should have visibility function for x_axis_time_format', () => {
78+
expect(timeFormatControl).toBeDefined();
79+
expect(timeFormatControl.config.visibility).toBeDefined();
80+
expect(typeof timeFormatControl.config.visibility).toBe('function');
81+
82+
// The visibility function exists - the exact logic is tested implicitly through UI behavior
83+
// The important part is that the control has proper visibility configuration
84+
});
85+
86+
const isTimeVisible = (
87+
xAxisColumn: string | null,
88+
xAxisType: string | null,
89+
): boolean => {
90+
const props = mockControls(xAxisColumn, xAxisType);
91+
const visibilityFn = timeFormatControl?.config?.visibility;
92+
return visibilityFn ? visibilityFn(props) : false;
93+
};
94+
95+
test('x_axis_time_format control should be visible for any data types include TIME', () => {
96+
expect(isTimeVisible('time_column', 'TIME')).toBe(true);
97+
expect(isTimeVisible('time_column', 'TIME WITH TIME ZONE')).toBe(true);
98+
expect(isTimeVisible('time_column', 'TIMESTAMP WITH TIME ZONE')).toBe(true);
99+
expect(isTimeVisible('time_column', 'TIMESTAMP WITHOUT TIME ZONE')).toBe(
100+
true,
101+
);
102+
});
103+
104+
test('x_axis_time_format control should be hidden for data types that do NOT include TIME', () => {
105+
expect(isTimeVisible('null', 'null')).toBe(false);
106+
expect(isTimeVisible(null, null)).toBe(false);
107+
expect(isTimeVisible('float_column', 'FLOAT')).toBe(false);
108+
});
109+
110+
// tests for x_axis_number_format control
111+
const numberFormatControl: any = getControl('x_axis_number_format');
112+
113+
test('scatter chart control panel should include x_axis_number_format control in the panel', () => {
114+
expect(numberFormatControl).toBeDefined();
115+
});
116+
117+
test('scatter chart control panel should have correct default value for x_axis_number_format', () => {
118+
expect(numberFormatControl).toBeDefined();
119+
expect(numberFormatControl.config).toBeDefined();
120+
expect(numberFormatControl.config.default).toBe('SMART_NUMBER');
121+
});
122+
123+
test('scatter chart control panel should have visibility function for x_axis_number_format', () => {
124+
expect(numberFormatControl).toBeDefined();
125+
expect(numberFormatControl.config.visibility).toBeDefined();
126+
expect(typeof numberFormatControl.config.visibility).toBe('function');
127+
128+
// The visibility function exists - the exact logic is tested implicitly through UI behavior
129+
// The important part is that the control has proper visibility configuration
130+
});
131+
132+
const isNumberVisible = (
133+
xAxisColumn: string | null,
134+
xAxisType: string | null,
135+
): boolean => {
136+
const props = mockControls(xAxisColumn, xAxisType);
137+
const visibilityFn = numberFormatControl?.config?.visibility;
138+
return visibilityFn ? visibilityFn(props) : false;
139+
};
140+
141+
test('x_axis_number_format control should be visible for any floating-point data types', () => {
142+
expect(isNumberVisible('float_column', 'FLOAT')).toBe(true);
143+
expect(isNumberVisible('double_column', 'DOUBLE')).toBe(true);
144+
expect(isNumberVisible('real_column', 'REAL')).toBe(true);
145+
expect(isNumberVisible('numeric_column', 'NUMERIC')).toBe(true);
146+
expect(isNumberVisible('decimal_column', 'DECIMAL')).toBe(true);
147+
});
148+
149+
test('x_axis_number_format control should be hidden for any non-floating-point data types', () => {
150+
expect(isNumberVisible('string_column', 'VARCHAR')).toBe(false);
151+
expect(isNumberVisible('null', 'null')).toBe(false);
152+
expect(isNumberVisible(null, null)).toBe(false);
153+
expect(isNumberVisible('time_column', 'TIMESTAMP WITHOUT TIME ZONE')).toBe(
154+
false,
155+
);
156+
});

0 commit comments

Comments
 (0)