rebenitez1802 commented on code in PR #45954:
URL: https://github.com/apache/superset/pull/45954#discussion_r4443783529
##########
superset-frontend/src/components/Chart/ChartRenderer.tsx:
##########
@@ -516,6 +517,12 @@ function ChartRendererComponent({
dataMaskSourceRef.current = latestQueryFormData ?? formData;
const setDataMaskCallback = useCallback(
(dataMask: DataMask) => {
+ if (
+ dataMask.filterState !== undefined ||
+ dataMask.extraFormData !== undefined
+ ) {
+ releaseFilterKeyPublish();
Review Comment:
🟡 Medium (pre-existing, non-blocking) - Filter change during an in-flight
"Try again" is overwritten
Not introduced by this PR, but chart clicks now make it easier to hit. The
release listener in `DashboardPage.tsx:478` only clears the notice and never
aborts `retryController`. If the viewer clicks **Try again** and then
cross-filters a chart before the retry resolves, `retrySharedState`
(`DashboardPage.tsx:483-504`) still calls `hydrateWithStateRef.current(...)`
and applies the saved state over their change, which contradicts the comment at
`:476`. FilterBar Apply and the cross-filter chip already hit this on master.
`DashboardPage.tsx` isn't in this diff, so this can't be an applyable
suggestion. Fix (validated with a repro test: fall back, click Try again with a
pending fetch, release, resolve, then assert no hydrate with the saved mask):
```tsx
useEffect(
() =>
subscribeFilterKeyPublishRelease(() => {
retryController.current?.abort();
setIsRetryingSharedState(false);
setSharedStateNotice(null);
}),
[],
);
```
--
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]
