luizotavio32 commented on code in PR #44810:
URL: https://github.com/apache/superset/pull/44810#discussion_r4139794808
##########
superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx:
##########
@@ -289,12 +290,59 @@ function Echart(
const notMerge = !isDashboardRefreshing;
chartRef.current?.dispatchAction({ type: 'hideTip' });
+ // setOption(notMerge:true) replaces the dataZoom config, dropping any
+ // range the user has engaged. Preserve it across the call.
+ const previousZoom = notMerge
+ ? (
+ chartRef.current?.getOption() as {
+ dataZoom?: DataZoomComponentOption[];
+ }
+ )?.dataZoom
+ : undefined;
chartRef.current?.setOption(themedEchartOptions, {
notMerge,
replaceMerge: notMerge ? undefined : ['series'],
// lazyUpdate defers render, causing tooltip crashes on stale shapes
(#39247)
lazyUpdate: false,
});
+ if (previousZoom?.length) {
+ // Skip restore when the new option reshapes dataZoom (different count
+ // means index-based restore could land on the wrong component).
+ const newZoom = (
+ chartRef.current?.getOption() as {
+ dataZoom?: DataZoomComponentOption[];
+ }
+ )?.dataZoom;
+ if (newZoom?.length === previousZoom.length) {
+ const batch = previousZoom
+ .map((dz, dataZoomIndex) => ({
+ dataZoomIndex,
+ start: dz.start,
+ end: dz.end,
+ startValue: dz.startValue,
+ endValue: dz.endValue,
Review Comment:
Not an issue in practice: within a mounted chart the `dataZoom` array has a
fixed shape per chart type. For Timeseries (`transformProps.ts`), a zoomable
chart always emits the same three components in the same order (slider,
inside-y, inside-x); disabling zoom removes them all, which changes the length
and skips the restore. The only same-length change is flipping orientation,
which moves the slider between x and y, but in both cases it targets the
category axis, so the restored range still lands on the right axis. Changing
viz type remounts the chart, so nothing is carried over.
Matching by type/axis instead of index would be a robustness refactor, not a
behavior fix. This PR is a straight backport of #40173 and master has the
identical check, so any change there belongs in a separate PR against master.
--
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]