[CP Staging] Fix: bring chart spinner back in the same render on reload

src/components/Charts/components/ChartFrame.tsx

@@ -29,6 +29,7 @@ function ChartFrame({isLoading = false, hasData, getLoadingHeight, children}: Ch
         <ChartWidthBox>
             {(chartWidth) => (
                 <ChartReveal
+                    isLoading={isLoading}
                     loadingHeight={getLoadingHeight(chartWidth)}
                     isCardMeasured={chartWidth !== null}
                 >

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<ChartLoadState | null>(null);
@@ -65,15 +68,16 @@ 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.
     const isSpinnerShown = !loadState || hasReportedLoading;
     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) {

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(
+            <ChartReveal
+                isLoading
+                loadingHeight={LOADING_HEIGHT}
+                isCardMeasured
+            >
+                <ChartBody
+                    isLoading={false}
+                    label="chart"
+                />
+            </ChartReveal>,
+        );
+
+        // 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));