EnxDev commented on code in PR #42473:
URL: https://github.com/apache/superset/pull/42473#discussion_r3664441874
##########
superset-frontend/src/explore/components/ExploreViewContainer/index.tsx:
##########
@@ -227,7 +235,20 @@ const updateHistory = debounce(
force,
false,
);
- history.replace(url, payload);
+ const previousChartState = getChartStateFromHistoryState(
+ history.location.state,
+ );
+ const state = toChartStateHistoryState(payload, sliceId);
+ if (
+ isReplace ||
+ !previousChartState ||
+ isEqual(previousChartState, getChartStateFromHistoryState(state))
+ ) {
+ history.replace(url, state);
+ } else {
+ // one entry per chart state is what makes the Back button undo it
+ history.push(url, state);
+ }
Review Comment:
@msyavuz would make it sense to check it?
##########
superset-frontend/src/explore/components/ExploreViewContainer/index.tsx:
##########
@@ -611,6 +639,32 @@ function ExploreViewContainer(props:
ExploreViewContainerProps) {
],
);
+ // Explore's own entries carry the chart state they were pushed with, so a
POP
+ // into one is an undo/redo: apply it and re-query in place, which is also
why
+ // ExplorePage skips its reload for them.
+ const sliceId = props.form_data.slice_id ?? props.slice?.slice_id;
+ const { datasource: datasourceUid } = props.form_data;
+ useEffect(
+ () =>
+ history.listen((loc: Location, action: Action) => {
+ const chartState = getChartStateFromHistoryState(loc.state);
+ if (
+ action !== 'POP' ||
+ !chartState ||
+ !isSameChartState(chartState, {
+ slice_id: sliceId,
+ datasource: datasourceUid,
+ })
+ ) {
+ return;
+ }
Review Comment:
The two `popstate` (other one inside Chart/index.ts) handlers use different
baselines to decide whether to restore or reload.
After changing the dataset in Explore, `loadedFormData.current` is never
updated, so one handler thinks the entry matches while the other sees a
datasource mismatch.
As a result, pressing Back updates the URL but neither restores the controls
nor reloads the chart.
Could we make both checks use the same source of truth so the flow always
resolves to either a restore or a reload?
--
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]
