EnxDev commented on code in PR #45234:
URL: https://github.com/apache/superset/pull/45234#discussion_r4297675463
##########
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:
Both confirmed. Fixed in 7b9f12e3d3 with a refit in each plugin rather than
narrowing which charts stay on screen.
- Point Cluster Map: `computeFitBounds` reads the current `bounds` instead
of the ones captured at mount, and a new effect refits when `bounds` changes by
value. The `viewport_*` overrides still win, and a refresh whose bounds are
unchanged keeps the viewer's pan.
- Cartodiagram: in `FIT_DATA` mode the map calls `fitMapToCharts` when
`currentChartConfigs` changes. That state is already deep-compared with
`isChartConfigEqual` on the serialized sub-chart props, so a refresh with the
same data doesn't refit. A zoom in Explore re-runs `transformProps` with the
same data, so it doesn't refit either. A `CUSTOM` map view keeps its fixed
center.
Before choosing between the two options, I went through the other plugins
for state derived from the data only at mount. No other plugin had it:
- legacy reactify plugins: reactify calls the render function on every
update, and each of them clears its container first
- ECharts: `setOption` runs on every `echartOptions` change
- Table, AG Grid, Pivot Table, Handlebars, Horizon, Word Cloud, Paired
t-test and TimeTable: their memos and effects all depend on the data
Limiting the behaviour to an opt-in list would bring the blank chart back
for all of those to cover the two that needed a fix.
Tests, each refit test fails without its fix:
- `MapLibre.test.tsx`: refits to the bounds of a refreshed response, keeps
the viewport overrides when the bounds move, and keeps a panned map when the
refreshed bounds are equal
- new `OlChartMap.test.tsx`, on a real OpenLayers map: `FIT_DATA` refits
from San Francisco to London on a refresh, and `CUSTOM` stays on its fixed
center
--
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]
