msyavuz commented on code in PR #42473:
URL: https://github.com/apache/superset/pull/42473#discussion_r3665258069
##########
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:
Checked, real for a chart A to B switch, so `updateHistory` is now cancelled
when Explore unmounts.
##########
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;
+ }
+ restoringFromHistory.current = true;
+ props.actions.setExploreControls(chartState);
+ props.actions.triggerQuery(true, props.chart.id);
Review Comment:
`form_data` is derived from the controls in `mapStateToProps`, so the query
already posts the restored state.
##########
superset-frontend/src/hooks/useUnsavedChangesPrompt/index.ts:
##########
@@ -88,6 +95,10 @@ export const useUnsavedChangesPrompt = ({
return undefined;
}
+ if (isInPlaceTransition?.(state)) {
+ return undefined;
+ }
Review Comment:
The flag is consumed by the save own navigation (REPLACE already returns
before it), so the in-place check does not strand 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:
Fixed, both now resolve the target through `selectRestoreTarget`, with a
regression test for the POP after a dataset swap.
##########
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))
Review Comment:
Both sides come from the same controls round-trip now, and a restore no
longer pushes at all, so a normalized value cannot create an entry.
--
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]
