diff --git a/jest/setup.ts b/jest/setup.ts index 44fbb6e59862..0bd03034ae95 100644 --- a/jest/setup.ts +++ b/jest/setup.ts @@ -357,6 +357,9 @@ jest.mock('victory-native', () => ({ CartesianChart: jest.fn( ({children}: {children?: (args: Record) => 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 diff --git a/patches/@shopify/react-native-skia/@shopify+react-native-skia+2.11.2+005+emit-first-draw-event.patch b/patches/@shopify/react-native-skia/@shopify+react-native-skia+2.11.2+005+emit-first-draw-event.patch new file mode 100644 index 000000000000..818fe0e95e8c --- /dev/null +++ b/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) { diff --git a/patches/@shopify/react-native-skia/details.md b/patches/@shopify/react-native-skia/details.md index 22f5616f461a..2611fe9ce9ce 100644 --- a/patches/@shopify/react-native-skia/details.md +++ b/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 diff --git a/server/victory-chart-renderer/tests/plotBoundsDrift.test.ts b/server/victory-chart-renderer/tests/plotBoundsDrift.test.ts new file mode 100644 index 000000000000..012a17802d77 --- /dev/null +++ b/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); + }); +}); diff --git a/server/victory-chart-renderer/tests/probes/plotBoundsProbe.tsx b/server/victory-chart-renderer/tests/probes/plotBoundsProbe.tsx new file mode 100644 index 000000000000..c4c83831a9d2 --- /dev/null +++ b/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'); diff --git a/server/victory-chart-renderer/tests/probes/plotBoundsProbeMain.tsx b/server/victory-chart-renderer/tests/probes/plotBoundsProbeMain.tsx new file mode 100644 index 000000000000..f14413e0e19a --- /dev/null +++ b/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 = ( + + {({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; + }} + +); + +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}})); diff --git a/src/components/Charts/BarChart/HorizontalBarChartContent.tsx b/src/components/Charts/BarChart/HorizontalBarChartContent.tsx index eae09d67fd3c..a3b650ad66a7 100644 --- a/src/components/Charts/BarChart/HorizontalBarChartContent.tsx +++ b/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 ( - - + + ); } diff --git a/src/components/Charts/BarChart/VerticalBarChartContent.tsx b/src/components/Charts/BarChart/VerticalBarChartContent.tsx index 0fa24cdd9c87..810da940ed93 100644 --- a/src/components/Charts/BarChart/VerticalBarChartContent.tsx +++ b/src/components/Charts/BarChart/VerticalBarChartContent.tsx @@ -1,5 +1,5 @@ -import ActivityIndicator from '@components/ActivityIndicator'; import BAR_INNER_PADDING from '@components/Charts/barChartConstants'; +import ChartSkeleton from '@components/Charts/ChartSkeleton'; import ChartTooltipLayer from '@components/Charts/components/ChartTooltipLayer'; import ChartXAxisLabels from '@components/Charts/components/ChartXAxisLabels'; import ChartYAxisLabels from '@components/Charts/components/ChartYAxisLabels'; @@ -13,38 +13,35 @@ import { useDynamicYDomain, useLabelHitTesting, } from '@components/Charts/hooks'; -import {calculateMinDomainPadding, getXAxisLabel, getYAxisLabelWidth} from '@components/Charts/utils'; -import VictoryTheme, {CHART_CONTENT_MIN_HEIGHT, GLYPH_PADDING} from '@components/Charts/VictoryTheme'; +import {calculateMinDomainPadding, getCartesianPlotBounds, getXAxisLabel, getYAxisLabelWidth} from '@components/Charts/utils'; +import VictoryTheme, {getCartesianChartHeight, getXAxisLabelSpace, GLYPH_PADDING} from '@components/Charts/VictoryTheme'; import useTheme from '@hooks/useTheme'; import useThemeStyles from '@hooks/useThemeStyles'; import variables from '@styles/variables'; -import type {LayoutChangeEvent} from 'react-native'; +import CONST from '@src/CONST'; + import type {CartesianChartRenderArg, ChartBounds, PointsArray, Scale} from 'victory-native'; -import React, {useState} from 'react'; +import React from 'react'; import {View} from 'react-native'; import {GestureDetector} from 'react-native-gesture-handler'; import Animated, {useAnimatedStyle, useSharedValue} from 'react-native-reanimated'; import {Bar, CartesianChart} from 'victory-native'; -import type {BarChartProps} from './types'; +import type {BarChartBodyProps} from './types'; /** Extra pixel spacing between the chart boundary and the data range, applied per side (Victory's `domainPadding` prop) * We need bottom: 1 for proper display of the bottom label */ const BASE_DOMAIN_PADDING = {top: 32, bottom: 1, left: 0, right: 0}; -function VerticalBarChartContentBody({data, isLoading, yAxisUnit, yAxisUnitPosition = 'left', color, onBarPress}: BarChartProps) { +function VerticalBarChartContentBody({data, isLoading, yAxisUnit, yAxisUnitPosition = 'left', color, onBarPress, chartWidth}: BarChartBodyProps) { const theme = useTheme(); const styles = useThemeStyles(); const fontManager = useChartFontManager(); - const [chartWidth, setChartWidth] = useState(0); - const [barAreaWidth, setBarAreaWidth] = useState(0); - const [boundsLeft, setBoundsLeft] = useState(0); - const [boundsRight, setBoundsRight] = useState(0); const chartData = data.map((point, index) => ({ x: index, @@ -63,9 +60,16 @@ function VerticalBarChartContentBody({data, isLoading, yAxisUnit, yAxisUnitPosit } }; - const handleLayout = (event: LayoutChangeEvent) => { - setChartWidth(event.nativeEvent.layout.width); - }; + const {formatValue} = useChartLabelFormats({ + data, + unit: yAxisUnit, + unitPosition: yAxisUnitPosition, + }); + + const yAxisLabelWidth = getYAxisLabelWidth(data, formatValue, fontManager, variables.iconSizeExtraSmall, BASE_DOMAIN_PADDING); + const chartPaddingLeft = yAxisLabelWidth + GLYPH_PADDING; + const {left: boundsLeft, right: boundsRight} = getCartesianPlotBounds(chartWidth, chartPaddingLeft); + const barAreaWidth = boundsRight - boundsLeft; const domainPadding = (() => { if (chartWidth === 0) { @@ -93,12 +97,6 @@ function VerticalBarChartContentBody({data, isLoading, yAxisUnit, yAxisUnitPosit measurements, }); - const {formatValue} = useChartLabelFormats({ - data, - unit: yAxisUnit, - unitPosition: yAxisUnitPosition, - }); - const barWidth = useSharedValue(0); const chartBottom = useSharedValue(0); const yZero = useSharedValue(0); @@ -112,16 +110,6 @@ function VerticalBarChartContentBody({data, isLoading, yAxisUnit, yAxisUnitPosit chartBottom, }); - const handleChartBoundsChange = (bounds: ChartBounds) => { - const domainWidth = bounds.right - bounds.left; - const calculatedBarWidth = ((1 - BAR_INNER_PADDING) * domainWidth) / data.length; - barWidth.set(calculatedBarWidth); - yZero.set(0); - setBarAreaWidth(domainWidth); - setBoundsLeft(bounds.left); - setBoundsRight(bounds.right); - }; - const checkIsOverBar = (args: HitTestArgs) => { 'worklet'; @@ -149,6 +137,7 @@ function VerticalBarChartContentBody({data, isLoading, yAxisUnit, yAxisUnitPosit }); const handleScaleChange = (xScale: Scale, yScale: Scale) => { + barWidth.set(((1 - BAR_INNER_PADDING) * barAreaWidth) / data.length); yZero.set(yScale(0)); updateTickPositions(xScale, data.length); setPointPositions( @@ -218,15 +207,17 @@ function VerticalBarChartContentBody({data, isLoading, yAxisUnit, yAxisUnitPosit ); }; - const labelSpace = VictoryTheme.axis.labelGap + (xAxisLabelHeight ?? 0); - const dynamicChartStyle = {height: CHART_CONTENT_MIN_HEIGHT + labelSpace}; - const yAxisLabelWidth = getYAxisLabelWidth(data, formatValue, fontManager, variables.iconSizeExtraSmall, BASE_DOMAIN_PADDING); - const chartPadding = {...VictoryTheme.axis.padding, bottom: labelSpace + VictoryTheme.axis.padding.bottom, left: yAxisLabelWidth + GLYPH_PADDING}; + const labelSpace = getXAxisLabelSpace(xAxisLabelHeight); + const chartHeight = getCartesianChartHeight(xAxisLabelHeight); + const chartBoxStyle = [styles.chartContent, {height: chartHeight}]; + + const chartSize = chartWidth > 0 ? {width: chartWidth, height: chartHeight} : undefined; + const chartPadding = {...VictoryTheme.axis.padding, bottom: labelSpace + VictoryTheme.axis.padding.bottom, left: chartPaddingLeft}; if (isLoading || !fontManager) { return ( - - + + ); } @@ -240,17 +231,14 @@ function VerticalBarChartContentBody({data, isLoading, yAxisUnit, yAxisUnitPosit gesture={customGestures} touchAction="pan-y" > - - {chartWidth > 0 && ( + + {chartSize ? ( {({points, chartBounds}) => points.y.map((point) => renderBar(point, chartBounds, points.y.length))} - )} + ) : null} + + {(chartWidth) => ( + + )} + ); } diff --git a/src/components/Charts/BarChart/index.tsx b/src/components/Charts/BarChart/index.tsx index db4696a25ce7..d2ebac8dcc00 100644 --- a/src/components/Charts/BarChart/index.tsx +++ b/src/components/Charts/BarChart/index.tsx @@ -1,7 +1,11 @@ +import ChartSkeleton from '@components/Charts/ChartSkeleton'; +import ChartWidthBox from '@components/Charts/ChartWidthBox'; import SkiaWebChart from '@components/Charts/SkiaWebChart'; import useBarChartOrientation from '@hooks/useBarChartOrientation'; +import CONST from '@src/CONST'; + import React from 'react'; import type {BarChartProps} from './types'; @@ -12,10 +16,15 @@ function BarChart(props: BarChartProps) { const {isHorizontal} = useBarChartOrientation(); return ( - + + {(chartWidth) => ( + } + /> + )} + ); } diff --git a/src/components/Charts/BarChart/types.ts b/src/components/Charts/BarChart/types.ts index eab5e92441f6..e8cabc9dafa2 100644 --- a/src/components/Charts/BarChart/types.ts +++ b/src/components/Charts/BarChart/types.ts @@ -7,10 +7,15 @@ type BarChartProps = CartesianChartProps & { color?: string; }; -/** Adds the wrapper-resolved orientation. Only the dispatcher receives `isHorizontal`. Callers and bodies use `BarChartProps`. */ -type BarChartContentProps = BarChartProps & { +/** Adds the width the wrapper measured. Bodies receive it. Callers use `BarChartProps`. */ +type BarChartBodyProps = BarChartProps & { + chartWidth: number; +}; + +/** Adds the wrapper-resolved orientation. Only the dispatcher receives `isHorizontal`. */ +type BarChartContentProps = BarChartBodyProps & { /** When true, renders horizontal bars (value on the x-axis) instead of the default vertical bars. */ isHorizontal?: boolean; }; -export type {BarChartProps, BarChartContentProps}; +export type {BarChartProps, BarChartBodyProps, BarChartContentProps}; diff --git a/src/components/Charts/ChartSkeleton.tsx b/src/components/Charts/ChartSkeleton.tsx new file mode 100644 index 000000000000..350e80d2c796 --- /dev/null +++ b/src/components/Charts/ChartSkeleton.tsx @@ -0,0 +1,375 @@ +import type {ChartView} from '@components/Search/types'; +import SkeletonRect from '@components/SkeletonRect'; +import SkeletonViewContentLoader from '@components/SkeletonViewContentLoader'; + +import useContainerWidth from '@hooks/useContainerWidth'; +import useTheme from '@hooks/useTheme'; + +import variables from '@styles/variables'; + +import CONST from '@src/CONST'; + +import type {ReactElement, ReactNode} from 'react'; + +import React, {useId} from 'react'; +import {View} from 'react-native'; +import {Circle, Defs, LinearGradient, Path, Stop} from 'react-native-svg'; + +import VictoryTheme, {CHART_CONTENT_MIN_HEIGHT, getCartesianChartHeight} from './VictoryTheme'; + +const CHART_SKELETON_TEST_ID = 'chartSkeleton'; +const BASELINE_TEST_ID = 'chartSkeletonBaseline'; +const Y_AXIS_TEST_ID = 'chartSkeletonYAxis'; +const BAR_TEST_ID = 'chartSkeletonBar'; +const LINE_TEST_ID = 'chartSkeletonLine'; +const MARKER_TEST_ID = 'chartSkeletonMarker'; +const PIE_TEST_ID = 'chartSkeletonPie'; + +/** Stands in for the y-axis label column, which the loaded chart sizes from its data and the placeholder cannot. */ +const Y_AXIS_GUTTER = 64; + +const AXIS_THICKNESS = 1; + +/** + * CartesianChart lifts the bottom of its y range by twice the y label offset, to make room for x-axis labels of its + * own, even though these charts draw theirs outside the plot. + */ +const Y_RANGE_BOTTOM = CHART_CONTENT_MIN_HEIGHT - VictoryTheme.axis.padding.bottom - VictoryTheme.axis.labelGap * 2; + +/** The y scale's `nice()` rounds the padded domain out to whole steps: one below zero, since the domain starts at zero, and about ten above it. */ +const Y_RANGE_TYPICAL_STEPS = 11; + +const LINE_SERIES = [0.12, 0.2732, 0.6575, 0.2327, 0.3302, 0.7745, 0.5831, 0.7384, 0.3981, 0.6358, 0.2378, 0.88]; +const LINE_MARKER_RADIUS = 4; +const LINE_HALF_THICKNESS = 1; + +/** The design rings each marker in the card color. */ +const LINE_MARKER_CLEARANCE = LINE_MARKER_RADIUS + 2; + +/** The same fade `AreaGradient` gives the loaded chart. */ +const AREA_ALPHA_BASELINE = 0.02; +const AREA_ALPHA_LINE = 0.2; + +const BAR_HEIGHTS = [0.92, 0.68, 0.44, 0.2, 0.11]; +const BAR_WIDTH_RATIO = 0.1431; +const BAR_PITCH_RATIO = 0.1938; +const BAR_INSET_RATIO = 0.0489; +const BAR_BORDER_RADIUS = 8; + +/** Largest first, which is the order the loaded chart lays its slices out in. */ +const PIE_SLICE_SHARES = [0.41, 0.214, 0.214, 0.085, 0.077]; + +const PIE_LABEL_BARS = [ + {width: 48, offsetY: -17.5}, + {width: 72, offsetY: 6.5}, +]; +const PIE_LABEL_BAR_HEIGHT = 12; + +const PIE_LEGEND_ITEM_MARGIN_BOTTOM = 8; + +/** One row of the legend PieChartContent draws below the ring: the container's top margin, a line of `textNormal` and the item's bottom margin. */ +const PIE_LEGEND_ROW_HEIGHT = variables.qrShareHorizontalPadding + variables.fontSizeNormalHeight + PIE_LEGEND_ITEM_MARGIN_BOTTOM; + +type ChartSkeletonLayers = { + /** Drawn beneath the shimmer in a paint of its own */ + chrome?: ReactElement; + + /** Becomes the shimmer's clip path, so only fill geometry counts and a stroked shape draws nothing */ + shimmer: ReactNode; +}; + +type PlotArea = { + left: number; + top: number; + right: number; + bottom: number; +}; + +type Point = { + x: number; + y: number; +}; + +function getPlotArea(width: number): PlotArea { + const top = VictoryTheme.axis.padding.top; + + return { + left: Y_AXIS_GUTTER, + top, + right: Math.max(Y_AXIS_GUTTER, width - VictoryTheme.axis.padding.right), + bottom: Y_RANGE_BOTTOM - (Y_RANGE_BOTTOM - top) / Y_RANGE_TYPICAL_STEPS, + }; +} + +type PlotLineProps = { + plot: PlotArea; +}; + +const AXIS_HALF_THICKNESS = AXIS_THICKNESS / 2; + +function PlotBaseline({plot}: PlotLineProps) { + const theme = useTheme(); + + return ( + + ); +} + +function PlotYAxis({plot}: PlotLineProps) { + const theme = useTheme(); + + return ( + + ); +} + +type LineAreaFillProps = { + points: Point[]; + baselineY: number; +}; + +function LineAreaFill({points, baselineY}: LineAreaFillProps) { + const theme = useTheme(); + const gradientId = useId(); + const first = points.at(0); + const last = points.at(-1); + + if (!first || !last) { + return null; + } + + const along = points.map((point) => `L${point.x} ${point.y}`).join(''); + + return ( + <> + + + + + + + + + ); +} + +function buildSeriesPath(points: Point[]): string { + return points + .map((from, index) => { + const to = points.at(index + 1); + if (!to) { + return ''; + } + + const deltaX = to.x - from.x; + const deltaY = to.y - from.y; + const length = Math.hypot(deltaX, deltaY); + + if (length <= LINE_MARKER_CLEARANCE * 2) { + return ''; + } + + const unitX = deltaX / length; + const unitY = deltaY / length; + const startX = from.x + unitX * LINE_MARKER_CLEARANCE; + const startY = from.y + unitY * LINE_MARKER_CLEARANCE; + const endX = to.x - unitX * LINE_MARKER_CLEARANCE; + const endY = to.y - unitY * LINE_MARKER_CLEARANCE; + const offsetX = -unitY * LINE_HALF_THICKNESS; + const offsetY = unitX * LINE_HALF_THICKNESS; + + return `M${startX + offsetX} ${startY + offsetY}L${endX + offsetX} ${endY + offsetY}L${endX - offsetX} ${endY - offsetY}L${startX - offsetX} ${startY - offsetY}Z`; + }) + .join(''); +} + +function renderLineShape(width: number): ChartSkeletonLayers { + const plot = getPlotArea(width); + const points = LINE_SERIES.map((depth, index) => ({ + x: plot.left + ((plot.right - plot.left) * index) / (LINE_SERIES.length - 1), + y: plot.top + (plot.bottom - plot.top) * depth, + })); + + return { + chrome: ( + <> + + + + + ), + shimmer: ( + <> + + {points.map((point, index) => ( + + ))} + + ), + }; +} + +function renderBarShape(width: number): ChartSkeletonLayers { + const plot = getPlotArea(width); + const plotWidth = plot.right - plot.left; + const plotHeight = plot.bottom - plot.top; + + return { + chrome: , + shimmer: BAR_HEIGHTS.map((heightRatio, index) => { + const barHeight = plotHeight * heightRatio; + + return ( + + ); + }), + }; +} + +function toRingPoint(centerX: number, centerY: number, radius: number, angleDegrees: number): Point { + const angle = (angleDegrees * Math.PI) / 180; + return {x: centerX + radius * Math.cos(angle), y: centerY + radius * Math.sin(angle)}; +} + +// Mirrors the geometry PieChartContent hands the loaded chart, including the gap PaddedPieSlice trims from each slice. +function renderPieShape(width: number): ChartSkeletonLayers { + const centerX = width / 2; + const centerY = CHART_CONTENT_MIN_HEIGHT / 2; + const radius = Math.min(width, CHART_CONTENT_MIN_HEIGHT) / 2; + const innerRadius = radius * VictoryTheme.pie.innerRadiusRatio; + const {padAngle, startAngle: ringStartAngle} = VictoryTheme.pie; + + let sliceStartAngle = ringStartAngle; + const slicePaths = PIE_SLICE_SHARES.map((share) => { + const sweep = share * 360; + const startAngle = sliceStartAngle + padAngle / 2; + const endAngle = sliceStartAngle + sweep - padAngle / 2; + sliceStartAngle += sweep; + + const largeArcFlag = endAngle - startAngle > 180 ? 1 : 0; + const outerStart = toRingPoint(centerX, centerY, radius, startAngle); + const outerEnd = toRingPoint(centerX, centerY, radius, endAngle); + const innerEnd = toRingPoint(centerX, centerY, innerRadius, endAngle); + const innerStart = toRingPoint(centerX, centerY, innerRadius, startAngle); + + return ( + `M${outerStart.x} ${outerStart.y}A${radius} ${radius} 0 ${largeArcFlag} 1 ${outerEnd.x} ${outerEnd.y}` + + `L${innerEnd.x} ${innerEnd.y}A${innerRadius} ${innerRadius} 0 ${largeArcFlag} 0 ${innerStart.x} ${innerStart.y}Z` + ); + }); + + return { + shimmer: ( + <> + {slicePaths.map((d, index) => ( + + ))} + {PIE_LABEL_BARS.map((bar, index) => ( + + ))} + + ), + }; +} + +// Each height is the box its chart draws into, so the box holds its size when the chart replaces the placeholder. +const PLACEHOLDER_BY_VIEW: Record ChartSkeletonLayers; height: number}> = { + [CONST.SEARCH.VIEW.BAR]: {render: renderBarShape, height: getCartesianChartHeight()}, + [CONST.SEARCH.VIEW.LINE]: {render: renderLineShape, height: getCartesianChartHeight()}, + [CONST.SEARCH.VIEW.PIE]: {render: renderPieShape, height: CHART_CONTENT_MIN_HEIGHT}, +}; + +type ChartSkeletonProps = { + view: ChartView; + + /** Whether the loaded pie draws its legend. */ + shouldShowLegend?: boolean; +}; + +function ChartSkeleton({view, shouldShowLegend = true}: ChartSkeletonProps) { + const theme = useTheme(); + const {ref, onLayout, containerWidth} = useContainerWidth(); + const {render, height} = PLACEHOLDER_BY_VIEW[view]; + // The legend row is left empty, since the design draws no legend placeholder. + const legendHeight = view === CONST.SEARCH.VIEW.PIE && shouldShowLegend ? PIE_LEGEND_ROW_HEIGHT : 0; + const {chrome, shimmer} = render(containerWidth); + + return ( + + + {shimmer} + + + ); +} + +export default ChartSkeleton; +export {BAR_TEST_ID, BASELINE_TEST_ID, CHART_SKELETON_TEST_ID, LINE_TEST_ID, MARKER_TEST_ID, PIE_TEST_ID, Y_AXIS_TEST_ID}; diff --git a/src/components/Charts/ChartWidthBox.tsx b/src/components/Charts/ChartWidthBox.tsx new file mode 100644 index 000000000000..4845fb62cd4e --- /dev/null +++ b/src/components/Charts/ChartWidthBox.tsx @@ -0,0 +1,29 @@ +import useContainerWidth from '@hooks/useContainerWidth'; +import useThemeStyles from '@hooks/useThemeStyles'; + +import type {ReactNode} from 'react'; + +import React from 'react'; +import {View} from 'react-native'; + +type ChartWidthBoxProps = { + children: (chartWidth: number) => ReactNode; +}; + +/** On web the chart component mounts only once the chart engine has downloaded, so measuring here keeps the width across that mount. */ +function ChartWidthBox({children}: ChartWidthBoxProps) { + const styles = useThemeStyles(); + const {ref, onLayout, containerWidth} = useContainerWidth(); + + return ( + + {children(containerWidth)} + + ); +} + +export default ChartWidthBox; diff --git a/src/components/Charts/LineChart/LineChartContent.tsx b/src/components/Charts/LineChart/LineChartContent.tsx index f1320357b7c0..ee40b6c8547f 100644 --- a/src/components/Charts/LineChart/LineChartContent.tsx +++ b/src/components/Charts/LineChart/LineChartContent.tsx @@ -1,4 +1,4 @@ -import ActivityIndicator from '@components/ActivityIndicator'; +import ChartSkeleton from '@components/Charts/ChartSkeleton'; import AreaGradient from '@components/Charts/components/AreaGradient'; import ChartTooltipLayer from '@components/Charts/components/ChartTooltipLayer'; import ChartXAxisLabels from '@components/Charts/components/ChartXAxisLabels'; @@ -16,18 +16,19 @@ import { useDynamicYDomain, useLabelHitTesting, } from '@components/Charts/hooks'; -import {getXAxisLabel, getYAxisLabelWidth, labelOverhang} from '@components/Charts/utils'; -import VictoryTheme, {CHART_CONTENT_MIN_HEIGHT, GLYPH_PADDING, LABEL_PADDING, LABEL_ROTATIONS, SIN_45} from '@components/Charts/VictoryTheme'; +import {getCartesianPlotBounds, getXAxisLabel, getYAxisLabelWidth, labelOverhang} from '@components/Charts/utils'; +import VictoryTheme, {getCartesianChartHeight, getXAxisLabelSpace, GLYPH_PADDING, LABEL_PADDING, LABEL_ROTATIONS, SIN_45} from '@components/Charts/VictoryTheme'; import useTheme from '@hooks/useTheme'; import useThemeStyles from '@hooks/useThemeStyles'; import variables from '@styles/variables'; -import type {LayoutChangeEvent} from 'react-native'; -import type {CartesianChartRenderArg, ChartBounds, Scale} from 'victory-native'; +import CONST from '@src/CONST'; -import React, {useState} from 'react'; +import type {CartesianChartRenderArg, Scale} from 'victory-native'; + +import React from 'react'; import {View} from 'react-native'; import {GestureDetector} from 'react-native-gesture-handler'; import Animated, {useAnimatedStyle, useSharedValue} from 'react-native-reanimated'; @@ -48,14 +49,14 @@ type LineChartProps = CartesianChartProps & { onPointPress?: (dataPoint: ChartDataPoint, index: number) => void; }; -function LineChartContentBody({data, isLoading, yAxisUnit, yAxisUnitPosition = 'left', onPointPress}: LineChartProps) { +type LineChartContentProps = LineChartProps & { + chartWidth: number; +}; + +function LineChartContentBody({data, isLoading, yAxisUnit, yAxisUnitPosition = 'left', onPointPress, chartWidth}: LineChartContentProps) { const theme = useTheme(); const styles = useThemeStyles(); const fontManager = useChartFontManager(); - const [chartWidth, setChartWidth] = useState(0); - const [plotAreaWidth, setPlotAreaWidth] = useState(0); - const [boundsLeft, setBoundsLeft] = useState(0); - const [boundsRight, setBoundsRight] = useState(0); const yAxisDomain = useDynamicYDomain(data); const chartData = data.map((point, index) => ({ @@ -73,10 +74,6 @@ function LineChartContentBody({data, isLoading, yAxisUnit, yAxisUnitPosition = ' } }; - const handleLayout = (event: LayoutChangeEvent) => { - setChartWidth(event.nativeEvent.layout.width); - }; - const chartBottom = useSharedValue(0); const measurements = useChartLabelMeasurements(data, fontManager, variables.iconSizeExtraSmall); @@ -90,8 +87,11 @@ function LineChartContentBody({data, isLoading, yAxisUnit, yAxisUnitPosition = ' const yAxisLabelWidth = getYAxisLabelWidth(data, formatValue, fontManager, variables.iconSizeExtraSmall, BASE_DOMAIN_PADDING); - const tickSpacing = plotAreaWidth > 0 && data.length > 0 ? plotAreaWidth / data.length : 0; const chartPaddingLeft = yAxisLabelWidth + GLYPH_PADDING; + const {left: boundsLeft, right: boundsRight} = getCartesianPlotBounds(chartWidth, chartPaddingLeft); + const plotAreaWidth = boundsRight - boundsLeft; + + const tickSpacing = plotAreaWidth > 0 && data.length > 0 ? plotAreaWidth / data.length : 0; const domainPadding = (() => { if (!firstLabelWidth || !lastLabelWidth) { @@ -139,12 +139,6 @@ function LineChartContentBody({data, isLoading, yAxisUnit, yAxisUnitPosition = ' chartBottom, }); - const handleChartBoundsChange = (bounds: ChartBounds) => { - setPlotAreaWidth(bounds.right - bounds.left); - setBoundsLeft(bounds.left); - setBoundsRight(bounds.right); - }; - const checkIsOverDot = (args: HitTestArgs) => { 'worklet'; @@ -222,8 +216,11 @@ function LineChartContentBody({data, isLoading, yAxisUnit, yAxisUnitPosition = ' ); }; - const labelSpace = VictoryTheme.axis.labelGap + (xAxisLabelHeight ?? 0); - const dynamicChartStyle = {height: CHART_CONTENT_MIN_HEIGHT + labelSpace}; + const labelSpace = getXAxisLabelSpace(xAxisLabelHeight); + const chartHeight = getCartesianChartHeight(xAxisLabelHeight); + const chartBoxStyle = [styles.chartContent, {height: chartHeight}]; + + const chartSize = chartWidth > 0 ? {width: chartWidth, height: chartHeight} : undefined; const chartPadding = { ...VictoryTheme.axis.padding, bottom: labelSpace + VictoryTheme.axis.padding.bottom, @@ -232,8 +229,8 @@ function LineChartContentBody({data, isLoading, yAxisUnit, yAxisUnitPosition = ' if (isLoading || !fontManager) { return ( - - + + ); } @@ -247,17 +244,14 @@ function LineChartContentBody({data, isLoading, yAxisUnit, yAxisUnitPosition = ' gesture={customGestures} touchAction="pan-y" > - - {chartWidth > 0 && ( + + {chartSize ? ( @@ -315,4 +309,4 @@ function LineChartContent(props: LineChartProps) { } export default LineChartContent; -export type {LineChartProps}; +export type {LineChartProps, LineChartContentProps}; diff --git a/src/components/Charts/LineChart/index.native.tsx b/src/components/Charts/LineChart/index.native.tsx index 2c6ac7c2941f..85e9aae037b9 100644 --- a/src/components/Charts/LineChart/index.native.tsx +++ b/src/components/Charts/LineChart/index.native.tsx @@ -1,3 +1,5 @@ +import ChartWidthBox from '@components/Charts/ChartWidthBox'; + import React from 'react'; import type {LineChartProps} from './LineChartContent'; @@ -5,7 +7,16 @@ import type {LineChartProps} from './LineChartContent'; import LineChartContent from './LineChartContent'; function LineChart(props: LineChartProps) { - return ; + return ( + + {(chartWidth) => ( + + )} + + ); } LineChart.displayName = 'LineChart'; diff --git a/src/components/Charts/LineChart/index.tsx b/src/components/Charts/LineChart/index.tsx index 7de73f7fcf44..a2422f21482a 100644 --- a/src/components/Charts/LineChart/index.tsx +++ b/src/components/Charts/LineChart/index.tsx @@ -1,5 +1,9 @@ +import ChartSkeleton from '@components/Charts/ChartSkeleton'; +import ChartWidthBox from '@components/Charts/ChartWidthBox'; import SkiaWebChart from '@components/Charts/SkiaWebChart'; +import CONST from '@src/CONST'; + import React from 'react'; import type {LineChartProps} from './LineChartContent'; @@ -7,10 +11,15 @@ import type {LineChartProps} from './LineChartContent'; const getLineChartContent = () => import('./LineChartContent'); function LineChart(props: LineChartProps) { return ( - + + {(chartWidth) => ( + } + /> + )} + ); } diff --git a/src/components/Charts/PieChart/PieChartContent.tsx b/src/components/Charts/PieChart/PieChartContent.tsx index 4f261f1919bb..a7dfbcc13772 100644 --- a/src/components/Charts/PieChart/PieChartContent.tsx +++ b/src/components/Charts/PieChart/PieChartContent.tsx @@ -1,16 +1,16 @@ -import ActivityIndicator from '@components/ActivityIndicator'; +import ChartSkeleton from '@components/Charts/ChartSkeleton'; import ChartTooltip from '@components/Charts/components/ChartTooltip'; import {TOOLTIP_BAR_GAP, useChartLabelFormats, useTooltipData} from '@components/Charts/hooks'; import type {ChartDataPoint, ChartProps, PieSlice, UnitPosition} from '@components/Charts/types'; import {findSliceAtPosition, processDataIntoSlices} from '@components/Charts/utils'; -import VictoryTheme from '@components/Charts/VictoryTheme'; +import VictoryTheme, {CHART_CONTENT_MIN_HEIGHT} from '@components/Charts/VictoryTheme'; import Text from '@components/Text'; import useLocalize from '@hooks/useLocalize'; import useStyleUtils from '@hooks/useStyleUtils'; import useThemeStyles from '@hooks/useThemeStyles'; -import type {LayoutChangeEvent} from 'react-native'; +import CONST from '@src/CONST'; import React, {useState} from 'react'; import {View} from 'react-native'; @@ -34,12 +34,14 @@ type PieChartProps = ChartProps & { shouldShowLegend?: boolean; }; -function PieChartContent({data, isLoading, valueUnit, valueUnitPosition, onSlicePress, shouldShowLegend = true}: PieChartProps) { +type PieChartContentProps = PieChartProps & { + chartWidth: number; +}; + +function PieChartContent({data, isLoading, valueUnit, valueUnitPosition, onSlicePress, shouldShowLegend = true, chartWidth}: PieChartContentProps) { const styles = useThemeStyles(); const StyleUtils = useStyleUtils(); const {translate} = useLocalize(); - const [canvasWidth, setCanvasWidth] = useState(0); - const [canvasHeight, setCanvasHeight] = useState(0); const [activeSliceIndex, setActiveSliceIndex] = useState(-1); const [isHoveringOverPie, setIsHoveringOverPie] = useState(false); @@ -49,15 +51,14 @@ function PieChartContent({data, isLoading, valueUnit, valueUnitPosition, onSlice const cursorY = useSharedValue(0); const tooltipPosition = useSharedValue({x: 0, y: 0}); - const handleLayout = (event: LayoutChangeEvent) => { - setCanvasWidth(event.nativeEvent.layout.width); - setCanvasHeight(event.nativeEvent.layout.height); - }; - // Calculate pie geometry - const radius = Math.min(canvasWidth, canvasHeight) / 2; + const canvasHeight = CHART_CONTENT_MIN_HEIGHT; + const radius = Math.min(chartWidth, canvasHeight) / 2; const innerRadius = radius * VictoryTheme.pie.innerRadiusRatio; - const pieGeometry = {radius, innerRadius, centerX: canvasWidth / 2, centerY: canvasHeight / 2}; + const pieGeometry = {radius, innerRadius, centerX: chartWidth / 2, centerY: canvasHeight / 2}; + + // The chart renders no content until it has a size, so handing it one lets it render that content in the same pass it mounts in. + const chartSize = chartWidth > 0 ? {width: chartWidth, height: canvasHeight} : undefined; // Slices are sorted by absolute value (largest first) for color assignment, // so slice indices don't match the original data array. We map back via @@ -156,9 +157,10 @@ function PieChartContent({data, isLoading, valueUnit, valueUnitPosition, onSlice if (isLoading) { return ( - - - + ); } @@ -172,12 +174,10 @@ function PieChartContent({data, isLoading, valueUnit, valueUnitPosition, onSlice gesture={combinedGesture} touchAction="pan-y" > - + {processedSlices.length > 0 && ( )} @@ -231,4 +231,4 @@ function PieChartContent({data, isLoading, valueUnit, valueUnitPosition, onSlice } export default PieChartContent; -export type {PieChartProps}; +export type {PieChartProps, PieChartContentProps}; diff --git a/src/components/Charts/PieChart/index.native.tsx b/src/components/Charts/PieChart/index.native.tsx index 961f5bdfb82c..0bc22152239a 100644 --- a/src/components/Charts/PieChart/index.native.tsx +++ b/src/components/Charts/PieChart/index.native.tsx @@ -1,3 +1,5 @@ +import ChartWidthBox from '@components/Charts/ChartWidthBox'; + import React from 'react'; import type {PieChartProps} from './PieChartContent'; @@ -5,7 +7,16 @@ import type {PieChartProps} from './PieChartContent'; import PieChartContent from './PieChartContent'; function PieChart(props: PieChartProps) { - return ; + return ( + + {(chartWidth) => ( + + )} + + ); } PieChart.displayName = 'PieChart'; diff --git a/src/components/Charts/PieChart/index.tsx b/src/components/Charts/PieChart/index.tsx index 66999fee714f..4474395fb37d 100644 --- a/src/components/Charts/PieChart/index.tsx +++ b/src/components/Charts/PieChart/index.tsx @@ -1,5 +1,9 @@ +import ChartSkeleton from '@components/Charts/ChartSkeleton'; +import ChartWidthBox from '@components/Charts/ChartWidthBox'; import SkiaWebChart from '@components/Charts/SkiaWebChart'; +import CONST from '@src/CONST'; + import React from 'react'; import type {PieChartProps} from './PieChartContent'; @@ -8,10 +12,20 @@ const getPieChartContent = () => import('./PieChartContent'); function PieChart(props: PieChartProps) { return ( - + + {(chartWidth) => ( + + } + /> + )} + ); } diff --git a/src/components/Charts/SkiaWebChart/index.tsx b/src/components/Charts/SkiaWebChart/index.tsx index 781d6f18354e..c5f0c6f9d61b 100644 --- a/src/components/Charts/SkiaWebChart/index.tsx +++ b/src/components/Charts/SkiaWebChart/index.tsx @@ -10,13 +10,14 @@ import variables from '@styles/variables'; import viewRef from '@src/types/utils/viewRef'; -import type {ComponentType} from 'react'; +import type {ComponentType, ReactNode} from 'react'; import {WithSkiaWeb} from '@shopify/react-native-skia/lib/module/web'; import React, {useRef, useState} from 'react'; -import {View} from 'react-native'; +import {StyleSheet, View} from 'react-native'; import isSkiaWebSupported from './isSkiaWebSupported'; +import useHasSkiaDrawn from './useHasSkiaDrawn'; import useIsSkiaSurfaceUnavailable from './useIsSkiaSurfaceUnavailable'; type SkiaWebChartProps = { @@ -26,7 +27,8 @@ type SkiaWebChartProps = { /** Props forwarded to the lazily-loaded chart component. */ componentProps: TProps; - /** Identifies the loading skeleton span for telemetry. */ + /** Shown while the chart engine downloads and until the chart first draws. */ + loadingFallback?: ReactNode; }; function ChartUnavailable() { @@ -55,7 +57,7 @@ function ChartUnavailable() { // `object` mirrors WithSkiaWeb's own constraint; `Record` would reject the // interface-based render-html renderer props (VictoryChartRendererProps) that lack an index signature. // eslint-disable-next-line @typescript-eslint/no-restricted-types -function SkiaWebChart({getComponent, componentProps}: SkiaWebChartProps) { +function SkiaWebChart({getComponent, componentProps, loadingFallback}: SkiaWebChartProps) { const styles = useThemeStyles(); const containerRef = useRef(null); @@ -66,13 +68,15 @@ function SkiaWebChart({getComponent, componentProps}: Ski // The probe can pass while the renderer still ends up without a drawing surface, so also listen for the // renderer reporting that and degrade to the empty state. const isSurfaceUnavailable = useIsSkiaSurfaceUnavailable(containerRef); + const hasDrawn = useHasSkiaDrawn(containerRef); + const isAwaitingFirstDraw = !!loadingFallback && !hasDrawn; // If unsupported, the device can't give CanvasKit a usable WebGL surface. if (!isSupported || isSurfaceUnavailable) { return ; } - const fallback = ( + const fallback = loadingFallback ?? ( @@ -83,12 +87,16 @@ function SkiaWebChart({getComponent, componentProps}: Ski ref={viewRef(containerRef)} style={styles.mw100} > - `/${file}`}} - getComponent={getComponent} - componentProps={componentProps} - fallback={fallback} - /> + {/* The canvas stays blank until Skia draws into it, while the parts of a chart drawn as views show at once. */} + + `/${file}`}} + getComponent={getComponent} + componentProps={componentProps} + fallback={fallback} + /> + + {isAwaitingFirstDraw && {loadingFallback}} ); } diff --git a/src/components/Charts/SkiaWebChart/useHasSkiaDrawn.ts b/src/components/Charts/SkiaWebChart/useHasSkiaDrawn.ts new file mode 100644 index 000000000000..2cfbf75e2dd4 --- /dev/null +++ b/src/components/Charts/SkiaWebChart/useHasSkiaDrawn.ts @@ -0,0 +1,24 @@ +import type {RefObject} from 'react'; + +import {useEffect, useState} from 'react'; + +/** Dispatched by the patched Skia web renderer after it first draws a picture onto its canvas. */ +const FIRST_DRAW_EVENT = 'skia-first-draw'; + +function useHasSkiaDrawn(containerRef: RefObject): boolean { + const [hasDrawn, setHasDrawn] = useState(false); + + useEffect(() => { + const container = containerRef.current; + if (!(container instanceof HTMLElement)) { + return; + } + const markDrawn = () => setHasDrawn(true); + container.addEventListener(FIRST_DRAW_EVENT, markDrawn); + return () => container.removeEventListener(FIRST_DRAW_EVENT, markDrawn); + }, [containerRef]); + + return hasDrawn; +} + +export default useHasSkiaDrawn; diff --git a/src/components/Charts/VictoryTheme.ts b/src/components/Charts/VictoryTheme.ts index b11720661f13..76d99e1b5aad 100644 --- a/src/components/Charts/VictoryTheme.ts +++ b/src/components/Charts/VictoryTheme.ts @@ -2,6 +2,7 @@ * Centralized styles and layout constants for the chart components. */ import colors from '@styles/theme/colors'; +import variables from '@styles/variables'; import {CHART_FONT_FAMILY_NAMES} from './utils/chartFontConstants'; @@ -93,6 +94,23 @@ const VictoryTheme = { /** Minimum height for the chart content area (bars, Y-axis, grid lines) */ const CHART_CONTENT_MIN_HEIGHT = 250; +/** ExpensifyNeue's ascent plus descent over its em, 950 + 230 of 1000 units, which is the line height Skia reports for label text. */ +const LABEL_LINE_HEIGHT_RATIO = 1.18; + +/** + * One horizontal line of label text. CartesianChart reports the plot bounds a measured strip needs only after a + * render, and the placeholder has no loaded font to measure, so both fall back to this floor. + */ +const X_AXIS_LABEL_MIN_HEIGHT = Math.ceil(variables.iconSizeExtraSmall * LABEL_LINE_HEIGHT_RATIO); + +function getXAxisLabelSpace(xAxisLabelHeight = 0): number { + return VictoryTheme.axis.labelGap + Math.max(xAxisLabelHeight, X_AXIS_LABEL_MIN_HEIGHT); +} + +function getCartesianChartHeight(xAxisLabelHeight = 0): number { + return CHART_CONTENT_MIN_HEIGHT + getXAxisLabelSpace(xAxisLabelHeight); +} + /** Pixel height of the y-scale output range inside CartesianChart (content height minus base axis padding). */ const CHART_Y_SCALE_HEIGHT = CHART_CONTENT_MIN_HEIGHT - VictoryTheme.axis.padding.top - VictoryTheme.axis.padding.bottom; @@ -129,6 +147,8 @@ const GLYPH_PADDING = 4; export { CHART_CONTENT_MIN_HEIGHT, CHART_Y_SCALE_HEIGHT, + getCartesianChartHeight, + getXAxisLabelSpace, LABEL_ROTATIONS, SIN_45, LABEL_PADDING, diff --git a/src/components/Charts/utils/index.ts b/src/components/Charts/utils/index.ts index b4bab37aa2ec..65faf3ea2cb6 100644 --- a/src/components/Charts/utils/index.ts +++ b/src/components/Charts/utils/index.ts @@ -439,6 +439,20 @@ function getNiceYAxisTicks(rawDataMax: number, rawDataMin: number, tickCount: nu return scaleLinear().domain([paddedMin, paddedMax]).nice().ticks(tickCount); } +/** + * Predicts the horizontal plot bounds victory-native will report for a cartesian chart of this width. + * + * The library sets the x output range to [padding.left + measured y-label width + y labelOffset, width - padding.right] + * and takes the plot bounds from the ends of that range. Our y axis carries no font, so the width it measures for its + * own labels is zero and only labelOffset applies. + */ +function getCartesianPlotBounds(chartWidth: number, chartPaddingLeft: number): {left: number; right: number} { + if (chartWidth <= 0) { + return {left: 0, right: 0}; + } + return {left: chartPaddingLeft + VictoryTheme.axis.labelGap, right: chartWidth - VictoryTheme.axis.padding.right}; +} + /** * Nice-rounded value domain for the horizontal bar chart's x-axis. victory-native only applies .nice() to the * y-axis, so we pre-round here (anchored at zero unless negatives) to keep the last tick past the longest bar. Returns undefined @@ -509,6 +523,7 @@ export { isCursorInSkewedLabel, isCursorOverChartLabel, getNiceYAxisTicks, + getCartesianPlotBounds, getNiceValueDomain, getNiceValueTicks, getYAxisLabelWidth, diff --git a/src/hooks/useContainerWidth.ts b/src/hooks/useContainerWidth.ts index ca2760f5c6d8..fadcc33a7ecc 100644 --- a/src/hooks/useContainerWidth.ts +++ b/src/hooks/useContainerWidth.ts @@ -1,15 +1,29 @@ -import type {LayoutChangeEvent} from 'react-native'; +import isHTMLElement from '@libs/isHTMLElement'; -import {useCallback, useState} from 'react'; +import type {RefObject} from 'react'; +import type {LayoutChangeEvent, View} from 'react-native'; + +import {useCallback, useLayoutEffect, useRef, useState} from 'react'; /** - * Returns an onLayout handler and the measured container width (minus an optional offset). - * Used by skeleton components that need the container width for SVG layout calculations. + * Returns a ref, an onLayout handler and the measured container width (minus an optional offset). * The width updates automatically when the container resizes. + * + * Attach the returned ref to the container as well when its first frame draws from the width: on web, onLayout + * reports only after the browser has painted once, so without the ref that first frame is drawn at width 0. */ -function useContainerWidth(offset = 0): {onLayout: (event: LayoutChangeEvent) => void; containerWidth: number} { +function useContainerWidth(offset = 0): {ref: RefObject; onLayout: (event: LayoutChangeEvent) => void; containerWidth: number} { + const ref = useRef(null); const [containerWidth, setContainerWidth] = useState(0); + useLayoutEffect(() => { + const node: unknown = ref.current; + if (!isHTMLElement(node)) { + return; + } + setContainerWidth(node.getBoundingClientRect().width - offset); + }, [offset]); + const onLayout = useCallback( (event: LayoutChangeEvent) => { setContainerWidth(event.nativeEvent.layout.width - offset); @@ -17,7 +31,7 @@ function useContainerWidth(offset = 0): {onLayout: (event: LayoutChangeEvent) => [offset], ); - return {onLayout, containerWidth}; + return {ref, onLayout, containerWidth}; } export default useContainerWidth; diff --git a/src/pages/home/HomePage.tsx b/src/pages/home/HomePage.tsx index 0890196ac886..ace8bb1bda48 100644 --- a/src/pages/home/HomePage.tsx +++ b/src/pages/home/HomePage.tsx @@ -27,7 +27,7 @@ import {View} from 'react-native'; import ForYouSection from './ForYouSection'; import FreeTrialSection from './FreeTrialSection'; import GettingStartedSection from './GettingStartedSection'; -import {HomePageSkeletonRowCards, HomePageSkeletonSpinnerCard} from './HomePageSkeleton'; +import {HomePageSkeletonChartCard, HomePageSkeletonRowCards} from './HomePageSkeleton'; import InsightsSection from './InsightsSection'; import RecentlyAddedSection from './RecentlyAddedSection'; import UpcomingTravelSection from './UpcomingTravelSection'; @@ -73,7 +73,7 @@ function HomePage() { {forYouSection} {shouldShowHomeSkeleton ? ( <> - + ) : ( @@ -94,7 +94,7 @@ function HomePage() { > {forYouSection} {shouldShowHomeSkeleton ? ( - + ) : ( <> diff --git a/src/pages/home/HomePageSkeleton.tsx b/src/pages/home/HomePageSkeleton.tsx index 9aeeaf39f70e..ae034a0f04c8 100644 --- a/src/pages/home/HomePageSkeleton.tsx +++ b/src/pages/home/HomePageSkeleton.tsx @@ -1,5 +1,4 @@ -import ActivityIndicator from '@components/ActivityIndicator'; -import {CHART_CONTENT_MIN_HEIGHT} from '@components/Charts/VictoryTheme'; +import ChartSkeleton from '@components/Charts/ChartSkeleton'; import SkeletonRect from '@components/SkeletonRect'; import ItemListSkeletonView from '@components/Skeletons/ItemListSkeletonView'; import SkeletonTextLine, {BAR_HEIGHT} from '@components/Skeletons/SkeletonTextLine'; @@ -7,7 +6,6 @@ import WidgetContainer from '@components/WidgetContainer'; import useContainerWidth from '@hooks/useContainerWidth'; import useResponsiveLayout from '@hooks/useResponsiveLayout'; -import useStyleUtils from '@hooks/useStyleUtils'; import useThemeStyles from '@hooks/useThemeStyles'; import {lineHeightScale} from '@styles/typography'; @@ -29,13 +27,10 @@ const TRAILING_SUB_BAR_WIDTH = 40; const CARD_TITLE_WIDTH = 120; const LOWER_BAR_WIDTH = 80; -// The chart this stands in for holds its own loading spinner at exactly this height. -const SPINNER_CARD_HEIGHT = CHART_CONTENT_MIN_HEIGHT; const ROWS_PER_LIST_CARD = 3; const ROWS_PER_TABLE_CARD = CONST.HOME.SECTION_VISIBLE_LIMIT; const CARD_TEST_ID = 'homePageSkeletonCard'; -const SPINNER_TEST_ID = 'homePageSkeletonSpinner'; const TWO_BAR_ROW_BAR_WIDTHS = [140, 110] as const; @@ -172,19 +167,11 @@ function HomePageSkeletonTableCard() { ); } -// The card this stands in for has no bar-representable rows, so a shimmer stand-in would invent a row structure the real card does not have. -function HomePageSkeletonSpinnerCard() { - const styles = useThemeStyles(); - const StyleUtils = useStyleUtils(); - +// The line shape is drawn because the default Home insight is a line chart. +function HomePageSkeletonChartCard() { return ( - - - + ); } @@ -198,4 +185,4 @@ function HomePageSkeletonRowCards() { ); } -export {HomePageSkeletonSpinnerCard, HomePageSkeletonRowCards, CARD_TEST_ID, SPINNER_TEST_ID}; +export {HomePageSkeletonChartCard, HomePageSkeletonRowCards, CARD_TEST_ID}; diff --git a/src/styles/index.ts b/src/styles/index.ts index 950e9173495e..4b82e48a42e3 100644 --- a/src/styles/index.ts +++ b/src/styles/index.ts @@ -6767,11 +6767,6 @@ const staticStyles = (theme: ThemeColors) => chartContent: { height: CHART_CONTENT_MIN_HEIGHT, }, - chartActivityIndicator: { - height: CHART_CONTENT_MIN_HEIGHT, - justifyContent: 'center', - alignItems: 'center', - }, pieChartLegendContainer: { display: 'flex', justifyContent: 'center', diff --git a/tests/unit/components/Charts/ChartSkeleton.test.tsx b/tests/unit/components/Charts/ChartSkeleton.test.tsx new file mode 100644 index 000000000000..11f2a0f4f703 --- /dev/null +++ b/tests/unit/components/Charts/ChartSkeleton.test.tsx @@ -0,0 +1,119 @@ +import {fireEvent, render, screen} from '@testing-library/react-native'; + +import ChartSkeleton, {BAR_TEST_ID, BASELINE_TEST_ID, CHART_SKELETON_TEST_ID, LINE_TEST_ID, MARKER_TEST_ID, PIE_TEST_ID, Y_AXIS_TEST_ID} from '@components/Charts/ChartSkeleton'; +import {CHART_CONTENT_MIN_HEIGHT, getCartesianChartHeight} from '@components/Charts/VictoryTheme'; +import type {ChartView} from '@components/Search/types'; +import SkeletonRect from '@components/SkeletonRect'; +import SkeletonViewContentLoader from '@components/SkeletonViewContentLoader'; + +import CONST from '@src/CONST'; + +import React from 'react'; + +const CONTAINER_WIDTH = 320; +const NARROW_CONTAINER_WIDTH = 160; + +function renderAtContainerWidth(view: ChartView, width = CONTAINER_WIDTH) { + render(); + fireEvent(screen.getByTestId(CHART_SKELETON_TEST_ID), 'layout', {nativeEvent: {layout: {width, height: 0}}}); +} + +function getLoaderProps() { + return screen.UNSAFE_getByType(SkeletonViewContentLoader).props; +} + +type BarTransform = [{translateX: number}, {translateY: number}]; + +function isBarTransform(transform: unknown): transform is BarTransform { + return Array.isArray(transform) && transform.length === 2; +} + +function getBarBoxes() { + return screen.UNSAFE_getAllByType(SkeletonRect).map(({props}) => { + const transform: unknown = props.transform; + if (!isBarTransform(transform)) { + throw new Error('Bars are positioned by a translateX and a translateY'); + } + const [{translateX}, {translateY}] = transform; + return {left: translateX, right: translateX + Number(props.width), bottom: translateY + Number(props.height)}; + }); +} + +describe('ChartSkeleton', () => { + it('should draw bars standing on one baseline with no y-axis for the bar view', () => { + renderAtContainerWidth(CONST.SEARCH.VIEW.BAR); + + const bars = getBarBoxes(); + + expect(bars.length).toBeGreaterThan(1); + expect(bars.every((bar) => bar.right > bar.left)).toBe(true); + expect(new Set(bars.map((bar) => bar.bottom)).size).toBe(1); + expect(screen.getAllByTestId(BAR_TEST_ID)).toHaveLength(bars.length); + expect(getLoaderProps().beforeMask).toBeDefined(); + expect(screen.getByTestId(BASELINE_TEST_ID)).toBeTruthy(); + expect(screen.queryByTestId(Y_AXIS_TEST_ID)).toBeNull(); + expect(screen.queryByTestId(LINE_TEST_ID)).toBeNull(); + expect(screen.queryByTestId(PIE_TEST_ID)).toBeNull(); + }); + + it('should keep every bar inside the chart in a narrow container', () => { + renderAtContainerWidth(CONST.SEARCH.VIEW.BAR, NARROW_CONTAINER_WIDTH); + + expect(getBarBoxes().every((bar) => bar.right > bar.left && bar.right <= NARROW_CONTAINER_WIDTH)).toBe(true); + }); + + it('should draw the series line as filled segments with a marker at every point for the line view', () => { + renderAtContainerWidth(CONST.SEARCH.VIEW.LINE); + + const line = screen.getByTestId(LINE_TEST_ID); + + expect(String(line.props.d)).toMatch(/^(M[\d. -]+(L[\d. -]+){3}Z)+$/); + expect(line.props.stroke).toBeUndefined(); + expect(screen.getAllByTestId(MARKER_TEST_ID).length).toBeGreaterThan(1); + expect(screen.getByTestId(BASELINE_TEST_ID)).toBeTruthy(); + expect(screen.getByTestId(Y_AXIS_TEST_ID)).toBeTruthy(); + expect(screen.queryByTestId(BAR_TEST_ID)).toBeNull(); + expect(screen.queryByTestId(PIE_TEST_ID)).toBeNull(); + }); + + it('should draw ring slices with no axes for the pie view', () => { + renderAtContainerWidth(CONST.SEARCH.VIEW.PIE); + + const slices = screen.getAllByTestId(PIE_TEST_ID); + + expect(slices.length).toBeGreaterThan(1); + expect(slices.every((slice) => /A[\d.]+ [\d.]+ 0 [01] 1 .+A[\d.]+ [\d.]+ 0 [01] 0 .+Z$/.test(String(slice.props.d)))).toBe(true); + expect(getLoaderProps().beforeMask).toBeUndefined(); + expect(screen.queryByTestId(BASELINE_TEST_ID)).toBeNull(); + expect(screen.queryByTestId(Y_AXIS_TEST_ID)).toBeNull(); + expect(screen.queryByTestId(BAR_TEST_ID)).toBeNull(); + expect(screen.queryByTestId(LINE_TEST_ID)).toBeNull(); + }); + + it('should reserve the x-axis label strip for the cartesian views and a legend row for the pie view', () => { + renderAtContainerWidth(CONST.SEARCH.VIEW.LINE); + expect(Number(getLoaderProps().height)).toBe(getCartesianChartHeight()); + + screen.unmount(); + renderAtContainerWidth(CONST.SEARCH.VIEW.BAR); + expect(Number(getLoaderProps().height)).toBe(getCartesianChartHeight()); + + screen.unmount(); + renderAtContainerWidth(CONST.SEARCH.VIEW.PIE); + expect(Number(getLoaderProps().height)).toBeGreaterThan(CHART_CONTENT_MIN_HEIGHT); + }); + + it('should reserve no legend row for a pie that hides its legend', () => { + // Given a pie whose legend is replaced by a table below it + // When its placeholder renders + render( + , + ); + + // Then the box is only the ring, so the table does not move up when the pie replaces the placeholder + expect(Number(getLoaderProps().height)).toBe(CHART_CONTENT_MIN_HEIGHT); + }); +}); diff --git a/tests/unit/components/Charts/SkiaWebChart.test.tsx b/tests/unit/components/Charts/SkiaWebChart.test.tsx index 4b93cbb946c0..038fe5763b92 100644 --- a/tests/unit/components/Charts/SkiaWebChart.test.tsx +++ b/tests/unit/components/Charts/SkiaWebChart.test.tsx @@ -1,10 +1,13 @@ import {render, screen} from '@testing-library/react-native'; +import ActivityIndicator from '@components/ActivityIndicator'; import SkiaWebChart from '@components/Charts/SkiaWebChart'; import isSkiaWebSupported from '@components/Charts/SkiaWebChart/isSkiaWebSupported'; +import Text from '@components/Text'; import {WithSkiaWeb} from '@shopify/react-native-skia/lib/module/web'; import React from 'react'; +import {View} from 'react-native'; jest.mock('@components/Charts/SkiaWebChart/isSkiaWebSupported', () => jest.fn()); @@ -59,4 +62,35 @@ describe('SkiaWebChart', () => { expect(mockWithSkiaWeb).toHaveBeenCalled(); }); + + it('should hand the caller-supplied loading fallback to Skia', () => { + mockIsSkiaWebSupported.mockReturnValue(true); + const loadingFallback = chart skeleton; + + render( + , + ); + + expect(mockWithSkiaWeb).toHaveBeenCalledWith(expect.objectContaining({fallback: loadingFallback}), undefined); + }); + + it('should keep its own spinner fallback when the caller supplies none', () => { + mockIsSkiaWebSupported.mockReturnValue(true); + + render( + , + ); + + const fallback = mockWithSkiaWeb.mock.lastCall?.at(0)?.fallback; + render({fallback}); + + expect(screen.UNSAFE_getByType(ActivityIndicator)).toBeTruthy(); + }); }); diff --git a/tests/unit/components/Charts/chartPlaceholderHandover.test.tsx b/tests/unit/components/Charts/chartPlaceholderHandover.test.tsx new file mode 100644 index 000000000000..e69e53b51172 --- /dev/null +++ b/tests/unit/components/Charts/chartPlaceholderHandover.test.tsx @@ -0,0 +1,88 @@ +import {fireEvent, render, screen} from '@testing-library/react-native'; + +import BarChart from '@components/Charts/BarChart'; +import {CHART_SKELETON_TEST_ID} from '@components/Charts/ChartSkeleton'; +import LineChart from '@components/Charts/LineChart'; +import PieChart from '@components/Charts/PieChart'; +import type {ChartDataPoint} from '@components/Charts/types'; +import {CHART_CONTENT_MIN_HEIGHT} from '@components/Charts/VictoryTheme'; + +import React from 'react'; +import {PolarChart} from 'victory-native'; + +const CONTAINER_WIDTH = 320; + +const data: ChartDataPoint[] = [ + {label: 'Amazon', total: 120}, + {label: 'Uber', total: 80}, +]; + +/** The placeholder measures its own container as well, so the box under test is the outermost ancestor handling layout. */ +function getMeasuredBox() { + let node = screen.getByTestId(CHART_SKELETON_TEST_ID).parent; + let measuredBox = null as typeof node; + + while (node) { + if (typeof node.props.onLayout === 'function') { + measuredBox = node; + } + node = node.parent; + } + + if (!measuredBox) { + throw new Error('the placeholder has no ancestor reporting its layout'); + } + + return measuredBox; +} + +describe('chart placeholder handover', () => { + afterEach(() => { + jest.clearAllMocks(); + }); + + it.each([ + ['bar', BarChart], + ['line', LineChart], + ['pie', PieChart], + ])('should measure the %s chart box around its placeholder', (_name, Chart) => { + render( + , + ); + + expect(getMeasuredBox()).toBeTruthy(); + }); + + it('should hand the pie chart the width measured around its placeholder', () => { + const {rerender} = render( + , + ); + + fireEvent(getMeasuredBox(), 'layout', {nativeEvent: {layout: {width: CONTAINER_WIDTH, height: CHART_CONTENT_MIN_HEIGHT}}}); + rerender( + , + ); + + expect(PolarChart).toHaveBeenCalledWith(expect.objectContaining({explicitSize: {width: CONTAINER_WIDTH, height: CHART_CONTENT_MIN_HEIGHT}}), undefined); + }); + + it('should hand the pie chart no size before a width has been measured', () => { + render( + , + ); + + expect(PolarChart).toHaveBeenCalledWith(expect.objectContaining({explicitSize: undefined}), undefined); + }); +}); diff --git a/tests/unit/components/Charts/getCartesianChartHeight.test.ts b/tests/unit/components/Charts/getCartesianChartHeight.test.ts new file mode 100644 index 000000000000..05913c6e742e --- /dev/null +++ b/tests/unit/components/Charts/getCartesianChartHeight.test.ts @@ -0,0 +1,13 @@ +import VictoryTheme, {CHART_CONTENT_MIN_HEIGHT, getCartesianChartHeight} from '@components/Charts/VictoryTheme'; + +const TALL_LABEL_HEIGHT = 120; + +describe('getCartesianChartHeight', () => { + it('should hold one height until a measured strip exceeds the floor', () => { + const floored = getCartesianChartHeight(); + + expect(getCartesianChartHeight(0)).toBe(floored); + expect(getCartesianChartHeight(1)).toBe(floored); + expect(getCartesianChartHeight(TALL_LABEL_HEIGHT)).toBe(CHART_CONTENT_MIN_HEIGHT + VictoryTheme.axis.labelGap + TALL_LABEL_HEIGHT); + }); +}); diff --git a/tests/unit/pages/HomePage.test.tsx b/tests/unit/pages/HomePage.test.tsx index c8224234e4b2..b690567b6f54 100644 --- a/tests/unit/pages/HomePage.test.tsx +++ b/tests/unit/pages/HomePage.test.tsx @@ -104,19 +104,19 @@ jest.mock('@libs/NetworkState', () => ({ // Deliberately not `mockSection(...)`: that helper emits a `section-` testID, which `renderedSectionOrder` // matches, so a skeleton group would count as a rendered section. -const mockSpinnerCardTestID = 'homePageSkeletonSpinnerCard'; +const mockChartCardTestID = 'homePageSkeletonChartCard'; const mockRowCardsTestID = 'homePageSkeletonRowCards'; jest.mock('@pages/home/HomePageSkeleton', () => { const ReactModule = require('react'); const {View: RNView} = require('react-native'); - function MockHomePageSkeletonSpinnerCard() { - return ReactModule.createElement(RNView, {testID: mockSpinnerCardTestID}); + function MockHomePageSkeletonChartCard() { + return ReactModule.createElement(RNView, {testID: mockChartCardTestID}); } function MockHomePageSkeletonRowCards() { return ReactModule.createElement(RNView, {testID: mockRowCardsTestID}); } - return {HomePageSkeletonSpinnerCard: MockHomePageSkeletonSpinnerCard, HomePageSkeletonRowCards: MockHomePageSkeletonRowCards}; + return {HomePageSkeletonChartCard: MockHomePageSkeletonChartCard, HomePageSkeletonRowCards: MockHomePageSkeletonRowCards}; }); // Each section is mocked to render a stable `section-` testID so we can assert ordering and column placement. @@ -574,7 +574,7 @@ describe('HomePage', () => { const leftOrder = within(leftColumn) .getAllByTestId(/^(section-|homePageSkeleton)/) .map((el) => String(el.props.testID)); - expect(leftOrder).toEqual(['section-ForYouSection', mockSpinnerCardTestID]); + expect(leftOrder).toEqual(['section-ForYouSection', mockChartCardTestID]); expect(within(rightColumn).getByTestId(mockRowCardsTestID)).toBeOnTheScreen(); expect(renderedSectionOrder()).toEqual(['section-ForYouSection']); diff --git a/tests/unit/pages/home/HomePageSkeletonTest.tsx b/tests/unit/pages/home/HomePageSkeletonTest.tsx index 0df3d5cf0b3e..3f0bfa7dcb0d 100644 --- a/tests/unit/pages/home/HomePageSkeletonTest.tsx +++ b/tests/unit/pages/home/HomePageSkeletonTest.tsx @@ -1,24 +1,25 @@ import {render, screen} from '@testing-library/react-native'; -import {CARD_TEST_ID, HomePageSkeletonRowCards, HomePageSkeletonSpinnerCard, SPINNER_TEST_ID} from '@pages/home/HomePageSkeleton'; +import {CHART_SKELETON_TEST_ID} from '@components/Charts/ChartSkeleton'; + +import {CARD_TEST_ID, HomePageSkeletonChartCard, HomePageSkeletonRowCards} from '@pages/home/HomePageSkeleton'; import React from 'react'; -// The cards read `shouldUseNarrowLayout` for horizontal padding only, never for card count or shape. +// The cards read `shouldUseNarrowLayout` for padding only, never for card count or shape. jest.mock('@hooks/useResponsiveLayout', () => jest.fn(() => ({shouldUseNarrowLayout: false}))); -describe('HomePageSkeleton card groups', () => { - it('contributes two cards from the row group', () => { +describe('HomePageSkeleton cards', () => { + it('renders two row cards', () => { render(); expect(screen.getAllByTestId(CARD_TEST_ID)).toHaveLength(2); - expect(screen.queryByTestId(SPINNER_TEST_ID)).not.toBeOnTheScreen(); }); - it('contributes one spinner-bearing card from the spinner group', () => { - render(); + it('renders one card holding a chart placeholder', () => { + render(); expect(screen.getAllByTestId(CARD_TEST_ID)).toHaveLength(1); - expect(screen.getByTestId(SPINNER_TEST_ID)).toBeOnTheScreen(); + expect(screen.getByTestId(CHART_SKELETON_TEST_ID)).toBeOnTheScreen(); }); });