EnxDev commented on code in PR #41859:
URL: https://github.com/apache/superset/pull/41859#discussion_r4121867997
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/Regular/Bar/controlPanel.tsx:
##########
@@ -63,6 +67,29 @@ import { StackControlsValue } from '../../../constants';
const { logAxis, minorSplitLine, truncateYAxis, yAxisBounds, orientation } =
DEFAULT_FORM_DATA;
+const barXAxisForceCategoricalControl = {
+ ...xAxisForceCategoricalControl,
+ config: {
+ ...xAxisForceCategoricalControl.config,
+ default: true,
+ description: t(
+ 'Treat values as categorical. Enabled by default for bar charts to
prevent bar overflow and intermediate tick labels. Disable to use a continuous
numeric x-axis that preserves spacing for missing values.',
Review Comment:
`babel-extract` is red because this string and the Mixed one at
`MixedTimeseries/controlPanel.tsx:104` aren't in
`superset/translations/messages.pot` yet.
Could you add both msgids and run `scripts/translations/check_pot_drift.py`
to confirm? Adding them by hand is safer than regenerating the whole file,
which tends to rewrite the header.
##########
superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/transformProps.test.ts:
##########
@@ -1092,6 +1092,51 @@ test('tooltip resolves per-metric formats for
secondary-query series', () => {
expect(html).toContain('2.5');
});
+test('xAxisForceCategorical runtime fallback enables Category axis for
unstacked numeric bars when omitted from payload', () => {
+ const ts1 = 1745784000000;
+ const ts2 = 1745870400000;
+ const epochRows = [
+ { __timestamp: ts1, metric: 10 },
+ { __timestamp: ts2, metric: 20 },
+ ];
+ const epochQueryData = createTestQueryData(epochRows, {
+ colnames: ['__timestamp', 'metric'],
+ coltypes: [GenericDataType.Numeric, GenericDataType.Numeric],
+ label_map: { __timestamp: ['__timestamp'], metric: ['metric'] },
+ });
+
+ const defaultFormDataOmittingFlag = { ...DEFAULT_FORM_DATA } as any;
+ delete defaultFormDataOmittingFlag.xAxisForceCategorical;
+
+ // For unstacked numeric bars, Value axis would be selected if
+ // `xAxisForceCategorical` defaults to false. This test simulates a
+ // legacy/external payload that omits the control entirely.
+ const chartProps = createEchartsTimeseriesTestChartProps<
+ EchartsMixedTimeseriesFormData,
+ EchartsMixedTimeseriesProps
+ >({
+ ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+ defaultFormData: defaultFormDataOmittingFlag,
+ defaultVizType: 'mixed_timeseries',
+ defaultQueriesData: [epochQueryData, epochQueryData],
+ formData: {
+ ...formData,
+ x_axis: '__timestamp',
+ metrics: ['metric'],
+ metricsB: ['metric'],
+ groupby: [],
+ groupbyB: [],
+ stack: false,
+ stackB: false,
+ },
+ queriesData: [epochQueryData, epochQueryData],
+ });
+
+ const { echartOptions } = transformProps(chartProps);
+ const xAxis = echartOptions.xAxis as { type: string };
+ expect(xAxis.type).toBe(AxisType.Category);
Review Comment:
This covers the omitted-flag fallback, but nothing checks the opt-out, which
is the point of the PR: a numeric bar with `xAxisForceCategorical: false` set
explicitly should render a Value axis.
Worth adding that case next to this one, so a later change to the fallback
can't quietly swallow an explicit `false`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
