sadpandajoe commented on code in PR #45541:
URL: https://github.com/apache/superset/pull/45541#discussion_r4380557393
##########
superset-frontend/src/components/Chart/ChartRenderer.tsx:
##########
@@ -591,7 +671,7 @@ function ChartRendererComponent({
<ChartContextMenu
ref={contextMenuRef}
id={chartId}
- formData={currentFormData as QueryFormData}
+ formData={renderedFormData as QueryFormData}
Review Comment:
Both renderer suites mock `ChartContextMenu` without reading its props, so
switching this back to `currentFormData` keeps them green. Then a right-click
drill during a dashboard refresh would be built from the new grouping while the
chart on screen still shows the previous one. Could a test render
country-grouped data, start a refresh with region-grouped `formData` and the
same response, capture the menu's `formData` and assert it is still
country-grouped, then region-grouped once the new response lands?
##########
superset-frontend/src/components/Chart/ChartRenderer.tsx:
##########
@@ -434,9 +508,15 @@ function ChartRendererComponent({
[handleOnContextMenu, state.inContextMenu],
);
+ // Read through a ref so the callback, and the hooks passed to the chart,
+ // keep their identity when the form data changes. A new hooks object makes
+ // the plugin transform and redraw the chart, which a filter change would
+ // otherwise do under the spinner as soon as its query starts.
+ const dataMaskSourceRef = useRef(latestQueryFormData ?? formData);
+ dataMaskSourceRef.current = latestQueryFormData ?? formData;
Review Comment:
Deleting this assignment leaves every current test green: the semantic-mask
test emits once right after the first mount, and the refresh tests never call
`setDataMask`. Without it, a cross-filter emitted after a filter change keeps
the selection sources of the chart's first query. Could a `ChartRenderer` test
mount with one set of `semantic_selection_sources`, rerender with a different
`latestQueryFormData`, call `hooks.setDataMask`, and assert `updateDataMask`
carries the new sources and not the old ones?
--
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]
