sadpandajoe commented on code in PR #42599:
URL: https://github.com/apache/superset/pull/42599#discussion_r3684759180
##########
superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/Bar/transformProps.test.ts:
##########
@@ -994,4 +994,56 @@ describe('Bar Chart X-axis Time Formatting', () => {
expect(grid.bottom).not.toBe(expandedPadding.bottom);
});
});
+
+ describe('Regression test for Issue #42560', () => {
+ test('custom X Axis Title is preserved verbatim, not overwritten by the
axis number/currency format ("unit")', () => {
+ const formData = {
+ ...baseFormData,
+ orientation: 'vertical',
+ xAxisTitle: 'My X Axis',
+ xAxisNumberFormat: 'SMART_NUMBER',
+ yAxisFormat: '$,.2f',
+ };
+
+ const chartProps = new ChartProps({
+ ...baseChartPropsConfig,
+ formData,
Review Comment:
Agreed. The inherited `TIMESTAMP` data selects the temporal formatter, so
`xAxisNumberFormat` is not exercised. If this test is intended to pin the
numeric-format interaction, could the fixture use numeric X-axis data?
##########
superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/Bar/transformProps.test.ts:
##########
@@ -994,4 +994,56 @@ describe('Bar Chart X-axis Time Formatting', () => {
expect(grid.bottom).not.toBe(expandedPadding.bottom);
});
});
+
+ describe('Regression test for Issue #42560', () => {
+ test('custom X Axis Title is preserved verbatim, not overwritten by the
axis number/currency format ("unit")', () => {
+ const formData = {
+ ...baseFormData,
+ orientation: 'vertical',
+ xAxisTitle: 'My X Axis',
+ xAxisNumberFormat: 'SMART_NUMBER',
+ yAxisFormat: '$,.2f',
+ };
+
+ const chartProps = new ChartProps({
+ ...baseChartPropsConfig,
+ formData,
+ });
+
+ const transformedProps = transformProps(
+ chartProps as EchartsTimeseriesChartProps,
+ );
+ const xAxis = transformedProps.echartOptions.xAxis as any;
+
+ expect(xAxis.name).toBe('My X Axis');
+ });
+
+ test('X Axis Title control maps onto the rendered category (left) axis in
horizontal orientation, not the bottom axis', () => {
+ // Documents the axis swap for horizontal bar charts: `xAxisTitle` ends
+ // up on `echartOptions.yAxis.name` (the vertical category axis) and
+ // `yAxisTitle` ends up on `echartOptions.xAxis.name` (the horizontal
+ // value axis). This is existing, intentional swap behavior, not the
+ // "unit" overwrite described in the issue.
+ const formData = {
+ ...baseFormData,
+ orientation: 'horizontal',
+ xAxisTitle: 'My X Axis',
+ yAxisTitle: 'My Y Axis',
+ };
+
+ const chartProps = new ChartProps({
+ ...baseChartPropsConfig,
+ formData,
Review Comment:
Agreed. With the inherited `TIMESTAMP` fixture, the swapped axis remains a
time axis rather than a category axis. Could this either use category data and
assert the axis type, or rename the test and comment to describe the
type-independent axis swap?
--
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]
