EnxDev commented on code in PR #38675:
URL: https://github.com/apache/superset/pull/38675#discussion_r2981157304
##########
superset-frontend/plugins/plugin-chart-echarts/src/Bubble/transformProps.ts:
##########
@@ -172,6 +176,22 @@ export default function transformProps(chartProps:
EchartsBubbleChartProps) {
const xAxisFormatter = getNumberFormatter(xAxisFormat);
const yAxisFormatter = getNumberFormatter(yAxisFormat);
const tooltipSizeFormatter = getNumberFormatter(tooltipSizeFormat);
+ const legendData = Array.from(legends).sort((a: string, b: string) => {
+ if (!legendSort) return 0;
+ return legendSort === 'asc' ? a.localeCompare(b) : b.localeCompare(a);
+ });
+ const legendLayout = getLegendLayoutResult({
+ chartHeight: height,
+ chartWidth: width,
+ legendItems: legendData,
+ legendMargin,
+ orientation: legendOrientation,
+ show: showLegend,
+ theme,
+ type: legendType,
+ });
+ const effectiveLegendMargin = legendLayout.effectiveMargin ?? legendMargin;
Review Comment:
The ?? legendMargin fallback implies effectiveMargin can be undefined, yet
the function is presumably always called with a margin, what do you think about
that?
##########
superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts:
##########
@@ -583,13 +586,59 @@ export default function transformProps(
convertInteger(yAxisTitleMargin) !== 0;
const addXAxisTitleOffset =
!!xAxisTitle && convertInteger(xAxisTitleMargin) !== 0;
+ const baseChartPadding = getPadding(
+ showLegend,
+ legendOrientation,
+ addYAxisTitleOffset,
+ zoomable,
+ legendMargin,
+ addXAxisTitleOffset,
+ yAxisTitlePosition,
+ convertInteger(yAxisTitleMargin),
+ convertInteger(xAxisTitleMargin),
+ );
+ const legendData = series
+ .filter(
+ entry =>
+ extractForecastSeriesContext((entry.name || '') as string).type ===
+ ForecastSeriesEnum.Observation,
+ )
+ .map(entry => entry.name)
+ .filter((name): name is string => Boolean(name))
+ .concat(extractAnnotationLabels(annotationLayers))
+ .sort((a: string, b: string) => {
Review Comment:
I'm wondering if removing entry.id could silently mismatch the legend.data
keys with the actual series name if ECharts uses id for legend matching in any
code path
--
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]
