diff --git a/src/components/Charts/components/ChartFrame.tsx b/src/components/Charts/components/ChartFrame.tsx index c58f7838fdd0..b1705b844ec1 100644 --- a/src/components/Charts/components/ChartFrame.tsx +++ b/src/components/Charts/components/ChartFrame.tsx @@ -29,6 +29,7 @@ function ChartFrame({isLoading = false, hasData, getLoadingHeight, children}: Ch {(chartWidth) => ( diff --git a/src/components/Charts/components/ChartReveal.tsx b/src/components/Charts/components/ChartReveal.tsx index a12074237c26..666f12c9c40c 100644 --- a/src/components/Charts/components/ChartReveal.tsx +++ b/src/components/Charts/components/ChartReveal.tsx @@ -29,7 +29,7 @@ const ChartLoadingContext = createContext<((loadState: ChartLoadState) => void) function useReportChartLoading(isLoading: boolean, isDrawnBySkia = true) { const setLoadState = useContext(ChartLoadingContext); - // Before paint, so no frame shows the chart and the in-flow spinner stacked, or neither of them + // Before paint, so no frame shows the chart and the in-flow spinner stacked useLayoutEffect(() => { setLoadState?.({isLoading, isDrawnBySkia}); }, [setLoadState, isLoading, isDrawnBySkia]); @@ -40,6 +40,9 @@ function useIsInsideChartReveal() { } type ChartRevealProps = { + /** Brings the spinner back in the same render, since the chart's own report lands a commit later and leaves the card empty until then. */ + isLoading?: boolean; + /** Must match the loaded chart's height, or the card jumps at the reveal. */ loadingHeight: number; @@ -50,7 +53,7 @@ type ChartRevealProps = { children: ReactNode; }; -function ChartReveal({loadingHeight, isCardMeasured, children}: ChartRevealProps) { +function ChartReveal({isLoading = false, loadingHeight, isCardMeasured, children}: ChartRevealProps) { const styles = useThemeStyles(); const StyleUtils = useStyleUtils(); const [loadState, setLoadState] = useState(null); @@ -65,7 +68,8 @@ function ChartReveal({loadingHeight, isCardMeasured, children}: ChartRevealProps // Written on the UI thread, which counts the hold, so that the cleanup there can cancel the hold's next frame const holdFrameID = useSharedValue(0); - const isReady = !!loadState && !loadState.isLoading && isCardMeasured; + const isChartLoading = isLoading || !!loadState?.isLoading; + const isReady = !!loadState && !isChartLoading && isCardMeasured; // A chart that mounts with everything in memory waits only for its card's first layout to report the width, // so it shows no spinner and fades in at once. Only a chart that has shown the spinner holds it until Skia has drawn. @@ -73,7 +77,7 @@ function ChartReveal({loadingHeight, isCardMeasured, children}: ChartRevealProps const shouldHold = hasReportedLoading && !!loadState?.isDrawnBySkia; // Adjusted during render, not in the effect below, so the spinner changes in the same commit as what the chart reports - if (loadState?.isLoading && !hasReportedLoading) { + if (isChartLoading && !hasReportedLoading) { setHasReportedLoading(true); } if (!isReady && isRevealed) { diff --git a/tests/unit/components/Charts/chartReveal.test.tsx b/tests/unit/components/Charts/chartReveal.test.tsx index 426a38ec2fb4..f96d81fbae87 100644 --- a/tests/unit/components/Charts/chartReveal.test.tsx +++ b/tests/unit/components/Charts/chartReveal.test.tsx @@ -156,6 +156,30 @@ describe('ChartReveal', () => { expect(screen.UNSAFE_queryByType(ActivityIndicator)).toBeNull(); }); + it('should bring the spinner back in the same render when a revealed chart loads again', () => { + // Given a chart that has been revealed after loading + const {rerender} = render(renderChart(true)); + rerender(renderChart(false)); + runFrames(HOLD_FRAMES); + + // When the card starts loading again, before the chart has reported it + rerender( + + + , + ); + + // Then the spinner is back already, because an empty card for even one commit shrinks the page under the user + expect(screen.UNSAFE_queryByType(ActivityIndicator)).not.toBeNull(); + }); + it('should hold when a chart that faded in without a spinner reloads', () => { // Given a chart that faded in as soon as its card was measured const {rerender} = render(renderChart(false, 'chart', false));