Copilot commented on code in PR #42713:
URL: https://github.com/apache/superset/pull/42713#discussion_r3705970011
##########
superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts:
##########
@@ -844,6 +844,8 @@ export default function transformProps(
},
scale: truncateYAxis,
name: yAxisTitleSecondary,
+ nameGap: convertInteger(yAxisTitleMargin),
+ nameLocation: yAxisTitlePosition === 'Left' ? 'middle' : 'end',
Review Comment:
Both Y-axes (and padding calculations earlier in the function) call
`convertInteger(yAxisTitleMargin)` multiple times. Consider computing a single
`const yAxisTitleMarginPx = convertInteger(yAxisTitleMargin);` and reusing it
across yAxis[0], yAxis[1], and padding computation to keep behavior consistent
and avoid repeated conversions.
##########
superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/transformProps.test.ts:
##########
@@ -1243,3 +1243,55 @@ test('regression #37921: multi-metric Query A with
groupby does not duplicate fi
expect(name).not.toMatch(/score_one,\s+score_two/);
}
});
+
+test('y-axis title position: Left sets nameLocation to middle', () => {
+ const chartProps = createEchartsTimeseriesTestChartProps<
+ EchartsMixedTimeseriesFormData,
+ EchartsMixedTimeseriesProps
+ >({
+ ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+ defaultQueriesData: queriesData,
+ formData: {
+ ...formData,
+ yAxisTitlePosition: 'Left',
+ yAxisTitleMargin: 20,
+ },
+ queriesData,
+ });
+ const transformed = transformProps(chartProps as
EchartsMixedTimeseriesProps);
+
+ expect((transformed.echartOptions.yAxis as any)[0].nameGap).toEqual(20);
+ expect((transformed.echartOptions.yAxis as any)[0].nameLocation).toEqual(
+ 'middle',
+ );
+ expect((transformed.echartOptions.yAxis as any)[1].nameGap).toEqual(20);
+ expect((transformed.echartOptions.yAxis as any)[1].nameLocation).toEqual(
+ 'middle',
+ );
Review Comment:
The repeated `(… as any)` casts reduce type-safety and make the test more
brittle. Prefer casting `transformed.echartOptions.yAxis` to a typed array
(e.g., an ECharts YAxis option type) once and then indexing into it, so
property accesses are checked and future refactors fail at compile time instead
of runtime.
--
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]
