EnxDev commented on code in PR #45234:
URL: https://github.com/apache/superset/pull/45234#discussion_r4282064753
##########
superset-frontend/src/components/Chart/ChartRenderer.tsx:
##########
@@ -272,14 +272,44 @@ function ChartRendererComponent({
prevQueriesResponseRef.current = queriesResponse;
}, [queriesResponse]);
+ const currentFormData =
+ chartIsStale && latestQueryFormData ? latestQueryFormData : formData;
+ const vizType = currentFormData.viz_type || propVizType;
+ // Some charts fetch their own data and issue no top-level query (e.g.
deck.gl
+ // Multiple Layers, which renders its sub-layer charts), so an empty response
+ // is expected and shouldn't trigger the no-results state. Honor the chart's
+ // own enableNoResults metadata (defaults to true).
+ const chartEnableNoResults =
+ getChartMetadataRegistry().get(vizType)?.enableNoResults ?? true;
+
+ // A failed load leaves its errored response in the store, and the next
+ // CHART_UPDATE_STARTED clears chartAlert but not queriesResponse. Checking
+ // the response itself keeps that stale error from being drawn as chart data
+ // while the retry is in flight.
+ const hasAnyErrors = queriesResponse?.some(item => item?.error);
+ // The response of a chart that fetches its own data stays empty once it has
+ // loaded, so for those any response that landed counts as previous data.
+ const hasPreviousResponse = chartEnableNoResults
+ ? (queriesResponse?.length ?? 0) > 0
+ : Array.isArray(queriesResponse);
Review Comment:
Good catch, not intended. Keeping the chart mounted took away the remount
that used to reload the layers, so a refresh reported success and kept the
first-loaded data on screen. Fixed in 30ef2afe46.
Multiple Layers now has its own `transformProps` that adds `isRefreshing` to
the shared deck.gl props, and `DeckMulti` reloads its layers when that flag
goes from true to false, which is when the refresh lands. The reload also
fetches the sub-chart metadata again, the same as the remount did. On a refresh
`loadLayers` doesn't clear the layers or reset the viewport, so the current map
stays up until each new layer arrives. A change to `deck_slices` or to the
visibility filter still does the full reset. A failed refresh doesn't need
handling here: `chartAlert` is set, ChartRenderer returns null, and the chart
unmounts the same way as before.
Tests:
- `MultiV1.test.tsx`: no reload while `isRefreshing` is true, then one
reload when it lands, with the layer and its fitted viewport still on screen
while the new data is in flight. If I force the old reset path, the layer count
drops to 0 and the test fails.
- `transformProps.test.ts`: `isRefreshing` is passed through and the shared
props stay as they were.
--
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]
