sadpandajoe commented on code in PR #45234:
URL: https://github.com/apache/superset/pull/45234#discussion_r4296636536
##########
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);
+ const hasValidPreviousData = hasPreviousResponse && !hasAnyErrors;
+
+ // An auto-refresh keeps the chart on screen while its query is in flight.
+ // The store holds the previous response until the new one lands, so that
+ // response is what the chart renders in the meantime.
+ const isRefreshing =
+ Boolean(restProps.suppressLoadingSpinner) && chartStatus === 'loading';
+ const hasRenderableResults =
Review Comment:
Keeping every chart with a previous response mounted also removes the
remount that Point Cluster Map and Cartodiagram relied on to refit to new data.
Point Cluster Map computes its fit bounds once (`computeFitBounds` has empty
deps and the viewport effect only re-runs on the viewport override props), and
Cartodiagram only calls `fitMapToCharts` in its mount effect and on a
`map_view` mode change. If an auto-refresh moves the rows, say from San
Francisco to London, both maps stay framed on the old area and look empty where
the old remount refit them. The deck.gl charts got an explicit refit for this;
should these two get one as well, or should the kept-on-screen behaviour be
limited to plugins that handle new data in place?
--
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]
