sfirke commented on code in PR #45505:
URL: https://github.com/apache/superset/pull/45505#discussion_r4351097173
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -1767,14 +1729,11 @@ export default function transformProps(
}
const seriesForKey = series.find(s => s.name === key);
const symbolForSeries = (seriesForKey as any)?.symbol || 'circle';
- const marker = value.color
- ? getSymbolMarker(symbolForSeries, value.color)
- : value.marker;
const row = formatForecastTooltipSeries({
...value,
seriesName: key,
formatter: getSeriesFormatter(key),
- marker,
+ marker: tooltipMarker(value.color, symbolForSeries),
Review Comment:
Non-blocking, but worth considering while you're here: after this PR the
ECharts marker is unusable (the sanitizer blanks it), yet it still flows
through `extractForecastValuesFromTooltipParams` → `ForecastValue.marker` →
`formatForecastTooltipSeries` in `utils/forecast.ts`, and both remaining
callers spread `...value` and then overwrite `marker` here. The next
`tooltipHtml` consumer that forgets that override gets the invisible span
back.
Since `formatForecastTooltipSeries` already receives `color`, it could build
the
marker itself (taking an optional `symbol`), which lets `marker` drop out of
`ForecastValue` along with the `echarts/types/src/util/format` import. If you
go that way, this caller could also hoist the per-row
`series.find(s => s.name === key)` into a `Map<name, symbol>` built once
outside the formatter; it runs on every mousemove and is quadratic in series
count today.
---
🤖 _Drafted by Claude Code, reviewed and approved by @sfirke._
--
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]
