Draw chart skeletons instead of spinners while a chart loads

jest/setup.ts

@@ -357,6 +357,9 @@ jest.mock('victory-native', () => ({
     CartesianChart: jest.fn(
         ({children}: {children?: (args: Record<string, unknown>) => ReactNode}) => children?.({points: {y: []}, chartBounds: {left: 0, right: 0, top: 0, bottom: 0}}) ?? null,
     ),
+    Line: jest.fn(() => null),
+    Pie: {Chart: jest.fn(() => null)},
+    PolarChart: jest.fn(() => null),
 }));
 
 // Provide a default global fetch mock for tests that do not explicitly set it up

patches/@shopify/react-native-skia/@shopify+react-native-skia+2.11.2+005+emit-first-draw-event.patch

@@ -0,0 +1,72 @@
+diff --git a/node_modules/@shopify/react-native-skia/lib/commonjs/views/SkiaPictureView.web.js b/node_modules/@shopify/react-native-skia/lib/commonjs/views/SkiaPictureView.web.js
+index d5d5806..efa6d1e 100644
+--- a/node_modules/@shopify/react-native-skia/lib/commonjs/views/SkiaPictureView.web.js
++++ b/node_modules/@shopify/react-native-skia/lib/commonjs/views/SkiaPictureView.web.js
+@@ -23,6 +23,15 @@ const dp2Pixel = (pd, rect) => {
+     height: rect.height * pd
+   };
+ };
++const notifyFirstDraw = renderer => {
++  if (renderer.hasDrawn) {
++    return;
++  }
++  renderer.hasDrawn = true;
++  renderer.canvas.dispatchEvent(new CustomEvent("skia-first-draw", {
++    bubbles: true
++  }));
++};
+ class WebGLRenderer {
+   constructor(canvas) {
+     this.canvas = canvas;
+@@ -136,6 +145,7 @@ class WebGLRenderer {
+       canvas.drawPicture(picture);
+       canvas.restore();
+       this.surface.ref.flush();
++      notifyFirstDraw(this);
+     }
+   }
+   dispose() {
+@@ -269,6 +279,7 @@ class StaticWebGLRenderer {
+     // Draw the tempCanvas scaled down to the display size
+     ctx2d.drawImage(tempCanvas, 0, 0, tempCanvas.width, tempCanvas.height, 0, 0, this.canvas.clientWidth * this.pd, this.canvas.clientHeight * this.pd);
+     this.cleanupRenderResult(renderResult);
++    notifyFirstDraw(this);
+   }
+   makeImageSnapshot(picture, rect) {
+     if (!this.cachedImage) {
+diff --git a/node_modules/@shopify/react-native-skia/lib/module/views/SkiaPictureView.web.js b/node_modules/@shopify/react-native-skia/lib/module/views/SkiaPictureView.web.js
+index f6e4d6d..4ac9acf 100644
+--- a/node_modules/@shopify/react-native-skia/lib/module/views/SkiaPictureView.web.js
++++ b/node_modules/@shopify/react-native-skia/lib/module/views/SkiaPictureView.web.js
+@@ -17,6 +17,15 @@ const dp2Pixel = (pd, rect) => {
+     height: rect.height * pd
+   };
+ };
++const notifyFirstDraw = renderer => {
++  if (renderer.hasDrawn) {
++    return;
++  }
++  renderer.hasDrawn = true;
++  renderer.canvas.dispatchEvent(new CustomEvent("skia-first-draw", {
++    bubbles: true
++  }));
++};
+ class WebGLRenderer {
+   constructor(canvas) {
+     this.canvas = canvas;
+@@ -130,6 +139,7 @@ class WebGLRenderer {
+       canvas.drawPicture(picture);
+       canvas.restore();
+       this.surface.ref.flush();
++      notifyFirstDraw(this);
+     }
+   }
+   dispose() {
+@@ -263,6 +273,7 @@ class StaticWebGLRenderer {
+     // Draw the tempCanvas scaled down to the display size
+     ctx2d.drawImage(tempCanvas, 0, 0, tempCanvas.width, tempCanvas.height, 0, 0, this.canvas.clientWidth * this.pd, this.canvas.clientHeight * this.pd);
+     this.cleanupRenderResult(renderResult);
++    notifyFirstDraw(this);
+   }
+   makeImageSnapshot(picture, rect) {
+     if (!this.cachedImage) {

patches/@shopify/react-native-skia/details.md

@@ -159,3 +159,28 @@
 - Upstream PR/issue:
 - E/App issue: https://github.com/Expensify/App/issues/95221
 - PR introducing patch:
+
+### [@shopify+react-native-skia+2.11.2+005+emit-first-draw-event.patch](@shopify+react-native-skia+2.11.2+005+emit-first-draw-event.patch)
+
+- Reason:
+
+    ```
+    Lets a chart's loading placeholder stay on screen until the chart is actually visible on web, so a
+    chart goes straight from its skeleton to the drawn chart, with no blank frame in between and no frame
+    showing both.
+
+    On web, SkiaPictureView creates its renderer only once its canvas reports a layout, and draws the
+    first picture after that. React Native Web delivers that layout from a ResizeObserver through a
+    setTimeout, so the canvas is still blank for at least one painted frame after the chart component
+    mounts, for as long as the browser takes. Skia exposes no public signal for when that first draw has
+    happened: CanvasProps omits onLayout, onSize changes before the renderer exists, and victory-native
+    forwards neither to the canvas it renders.
+
+    Fix: after a renderer first draws a picture onto its canvas, it dispatches a bubbling
+    "skia-first-draw" CustomEvent on that canvas. SkiaWebChart keeps the chart hidden under its
+    placeholder until the event reaches its container, then shows the chart and removes the placeholder.
+    ```
+
+- Upstream PR/issue: 🛑 TODO. No upstream issue or PR exists yet; Skia has no public API reporting when a web canvas is first drawn, still true on 2.13.0, so one has to be proposed. Without this patch, 2.13.0 still shows a blank frame between the placeholder and the drawn chart on web (verified manually).
+- E/App issue: 🛑 TODO
+- PR introducing patch: https://github.com/Expensify/App/pull/100610

server/victory-chart-renderer/tests/plotBoundsDrift.test.ts

@@ -0,0 +1,77 @@
+import {beforeAll, describe, expect, test} from 'bun:test';
+
+import createRnStubPlugin from '@server/plugins/rnStubPlugin';
+import {spawnSync} from 'node:child_process';
+import {mkdtempSync} from 'node:fs';
+import {tmpdir} from 'node:os';
+import {basename, join, resolve} from 'node:path';
+import {fileURLToPath} from 'node:url';
+
+import {packageRoot} from './testUtils';
+
+type PlotBounds = {left: number; right: number};
+
+type ProbeOutput = {
+    predicted: PlotBounds;
+    reported: PlotBounds;
+};
+
+const runDir = mkdtempSync(join(tmpdir(), 'vcr-plot-bounds-'));
+const bundlePath = join(runDir, 'plotBoundsProbe.js');
+
+function hasPlotBounds(value: unknown): value is PlotBounds {
+    return typeof value === 'object' && value !== null && 'left' in value && typeof value.left === 'number' && 'right' in value && typeof value.right === 'number';
+}
+
+function parseProbeOutput(raw: string): ProbeOutput {
+    const parsed: unknown = JSON.parse(raw);
+
+    if (typeof parsed !== 'object' || parsed === null || !('predicted' in parsed) || !('reported' in parsed) || !hasPlotBounds(parsed.predicted) || !hasPlotBounds(parsed.reported)) {
+        throw new Error(`Plot bounds probe printed unexpected output:\n${raw}`);
+    }
+
+    return {predicted: parsed.predicted, reported: parsed.reported};
+}
+
+let probeOutput: ProbeOutput;
+
+describe('cartesian plot bounds', () => {
+    beforeAll(async () => {
+        const buildResult = await Bun.build({
+            entrypoints: [fileURLToPath(import.meta.resolve('./probes/plotBoundsProbe.tsx'))],
+            target: 'bun',
+            packages: 'bundle',
+            conditions: ['react-native'],
+            tsconfig: join(packageRoot, 'tsconfig.json'),
+            plugins: [createRnStubPlugin(resolve(packageRoot, '../stubs'))],
+        });
+
+        if (!buildResult.success) {
+            throw new Error(`Failed to bundle the plot bounds probe:\n${buildResult.logs.map(String).join('\n')}`);
+        }
+
+        if (buildResult.outputs.length === 0) {
+            throw new Error('Bundled plot bounds probe output is missing');
+        }
+
+        // The bundle loads CanvasKit's wasm from beside itself, so every output has to land in the run directory.
+        for (const output of buildResult.outputs) {
+            await Bun.write(output.kind === 'entry-point' ? bundlePath : join(runDir, basename(output.path)), output);
+        }
+
+        const runResult = spawnSync(process.execPath, [bundlePath], {
+            cwd: resolve(packageRoot, '../..'),
+            encoding: 'utf8',
+        });
+
+        if (runResult.status !== 0) {
+            throw new Error(`Plot bounds probe failed:\n${runResult.stderr}\n${runResult.stdout}`);
+        }
+
+        probeOutput = parseProbeOutput(runResult.stdout);
+    }, 120_000);
+
+    test('getCartesianPlotBounds matches the bounds victory-native reports', () => {
+        expect(probeOutput.predicted).toEqual(probeOutput.reported);
+    });
+});

server/victory-chart-renderer/tests/probes/plotBoundsProbe.tsx

@@ -0,0 +1,12 @@
+/*
+ * Renders one CartesianChart in a headless surface and prints, as JSON, both the plot bounds victory-native reports
+ * and the ones getCartesianPlotBounds predicts for the same width.
+ *
+ * It runs as a bundled child process rather than inside the test process because victory-native resolves
+ * react-native, which only loads through the stubs rnStubPlugin substitutes at bundle time.
+ */
+import initSkiaForCli from '../../src/initSkiaForCli';
+
+// Skia must be initialized before the chart module and its transitive imports are evaluated.
+await initSkiaForCli();
+await import('./plotBoundsProbeMain');

server/victory-chart-renderer/tests/probes/plotBoundsProbeMain.tsx

@@ -0,0 +1,68 @@
+import {getCartesianPlotBounds} from '@components/Charts/utils';
+import VictoryTheme from '@components/Charts/VictoryTheme';
+
+import type {ChartBounds} from 'victory-native';
+
+import {drawOffscreen, makeOffscreenSurface} from '@shopify/react-native-skia/lib/module/headless';
+import React from 'react';
+import {CartesianChart} from 'victory-native';
+
+const CHART_WIDTH = 400;
+const CHART_HEIGHT = 250;
+
+/** Stands in for the space a chart leaves for its own y-axis labels. */
+const CHART_PADDING_LEFT = 37;
+
+const DATA = [
+    {x: 0, y: 120},
+    {x: 1, y: 80},
+    {x: 2, y: 45},
+];
+
+const captured: {bounds: ChartBounds | null} = {bounds: null};
+
+const chartElement = (
+    <CartesianChart
+        explicitSize={{width: CHART_WIDTH, height: CHART_HEIGHT}}
+        headless
+        xKey="x"
+        yKeys={['y']}
+        padding={{...VictoryTheme.axis.padding, left: CHART_PADDING_LEFT}}
+        domainPadding={{top: 16, bottom: 16, left: 8, right: 8}}
+        xAxis={{
+            tickCount: DATA.length,
+            lineWidth: VictoryTheme.axis.xLineWidth,
+        }}
+        yAxis={[
+            {
+                tickCount: VictoryTheme.axis.tickCount,
+                lineWidth: VictoryTheme.axis.yLineWidth,
+                labelOffset: VictoryTheme.axis.labelGap,
+            },
+        ]}
+        frame={{lineWidth: 0}}
+        data={DATA}
+    >
+        {({chartBounds}) => {
+            // The bounds reach onChartBoundsChange from an effect, which the offscreen draw never flushes, so they
+            // are read here instead. Both come from the same value the chart computes for its own layout.
+            captured.bounds = chartBounds;
+            return null;
+        }}
+    </CartesianChart>
+);
+
+using surface = makeOffscreenSurface(CHART_WIDTH, CHART_HEIGHT);
+using image = await drawOffscreen(surface, chartElement);
+
+if (!image) {
+    throw new Error('Skia failed to draw the probe chart');
+}
+
+if (!captured.bounds) {
+    throw new Error('CartesianChart never reported its bounds');
+}
+
+const predicted = getCartesianPlotBounds(CHART_WIDTH, CHART_PADDING_LEFT);
+
+console.log(JSON.stringify({predicted, reported: {left: captured.bounds.left, right: captured.bounds.right}}));

src/components/Charts/BarChart/HorizontalBarChartContent.tsx

@@ -1,4 +1,4 @@
-import ActivityIndicator from '@components/ActivityIndicator';
+import ChartSkeleton from '@components/Charts/ChartSkeleton';
 import ChartTooltipLayer from '@components/Charts/components/ChartTooltipLayer';
 import ChartYAxisLabels from '@components/Charts/components/ChartYAxisLabels';
 import type {HitTestArgs, ResolveTargetIndexArgs} from '@components/Charts/hooks';
@@ -13,6 +13,8 @@ import useThemeStyles from '@hooks/useThemeStyles';
 import colors from '@styles/theme/colors';
 import variables from '@styles/variables';
 
+import CONST from '@src/CONST';
+
 import type {NonUniformRRect, SkTypefaceFontProvider} from '@shopify/react-native-skia';
 import type {LayoutChangeEvent} from 'react-native';
 import type {CartesianChartRenderArg, ChartBounds, Scale} from 'victory-native';
@@ -391,8 +393,8 @@ function HorizontalBarChartContentBody({data, isLoading, yAxisUnit, yAxisUnitPos
 
     if (isLoading || !fontManager) {
         return (
-            <View style={styles.chartActivityIndicator}>
-                <ActivityIndicator size="large" />
+            <View style={[styles.chartContent, dynamicChartStyle]}>
+                <ChartSkeleton view={CONST.SEARCH.VIEW.BAR} />
             </View>
         );
     }