From 1dd11a9e52410816b0157c0cdc9eea8b27e8392c Mon Sep 17 00:00:00 2001 From: Alfonso Curbelo Date: Tue, 11 Aug 2026 13:41:50 -0400 Subject: [PATCH 1/3] feat: add interactive prop for lightweight static mode to mobile charts Adds `interactive?: boolean` (default true) to cds-mobile CartesianChart (inherited by LineChart). `interactive={false}` skips the scrubber context and entrance animation and renders lines via a reanimated-free static Path that clips with a cheap rect (canvas.clipRect) instead of the anti-aliased path clip, cutting the main-thread SoftwarePathRenderer cost when many charts recycle in scrolling lists. Additive and backward-compatible (default preserves behavior). Linear: APP-770 Co-Authored-By: Claude --- .../visualizations/chart/CartesianChart.tsx | 125 +++++++++------- .../mobile/src/visualizations/chart/Path.tsx | 134 +++++++++++++----- .../chart/__tests__/Path.test.tsx | 80 +++++++++++ .../visualizations/chart/line/LineChart.tsx | 7 + .../line/__stories__/LineChart.stories.tsx | 15 ++ .../chart/line/__tests__/LineChart.test.tsx | 113 +++++++++++++++ 6 files changed, 389 insertions(+), 85 deletions(-) create mode 100644 packages/mobile/src/visualizations/chart/__tests__/Path.test.tsx create mode 100644 packages/mobile/src/visualizations/chart/line/__tests__/LineChart.test.tsx diff --git a/packages/mobile/src/visualizations/chart/CartesianChart.tsx b/packages/mobile/src/visualizations/chart/CartesianChart.tsx index 29ab07d017..12d21d2772 100644 --- a/packages/mobile/src/visualizations/chart/CartesianChart.tsx +++ b/packages/mobile/src/visualizations/chart/CartesianChart.tsx @@ -122,6 +122,20 @@ export type CartesianChartBaseProps = Omit & * @default true */ animate?: boolean; + /** + * Whether the chart is interactive. + * + * When `false`, renders a lightweight, static chart for non-interactive or decorative + * contexts (e.g. row sparklines in scrolling lists): scrubbing is disabled, the entrance + * animation is skipped, and lines use a cheap rectangular clip instead of the anti-aliased + * path clip that pushes rendering onto Skia's CPU path renderer. + * + * @note Forces `animate` off at the chart level (a per-`Line`/`Path` `animate` override still + * wins). Scrubbing requires `interactive`; nesting a `Scrubber` under `interactive={false}` is + * unsupported. + * @default true + */ + interactive?: boolean; /** * Configuration for x-axis(es). Can be a single config or array of configs. * @@ -207,6 +221,7 @@ export const CartesianChart = memo( children, layout = 'vertical', animate = true, + interactive = true, enableScrubbing, getScrubberAccessibilityLabel, scrubberAccessibilityLabelStep, @@ -239,6 +254,11 @@ export const CartesianChart = memo( }) => { const [containerLayout, onContainerLayout] = useChartLayout(); + // Non-interactive charts skip scrubbing + entrance animation for a lightweight static render. + const isInteractive = interactive !== false; + const effectiveAnimate = isInteractive ? animate : false; + const effectiveEnableScrubbing = isInteractive ? enableScrubbing : false; + const chartWidth = containerLayout.width; const chartHeight = containerLayout.height; @@ -559,7 +579,7 @@ export const CartesianChart = memo( series: series ?? [], getSeries, getSeriesData: getStackedSeriesData, - animate, + animate: effectiveAnimate, width: chartWidth, height: chartHeight, fontFamilies, @@ -581,7 +601,7 @@ export const CartesianChart = memo( series, getSeries, getStackedSeriesData, - animate, + effectiveAnimate, chartWidth, chartHeight, fontFamilies, @@ -627,7 +647,7 @@ export const CartesianChart = memo( width, ...props, // Claim RN responder so parent PanResponders (e.g. Tray) don't steal the touch. - ...(enableScrubbing + ...(effectiveEnableScrubbing ? { onStartShouldSetResponder: claimTouchResponder, onMoveShouldSetResponder: claimTouchResponder, @@ -635,7 +655,7 @@ export const CartesianChart = memo( : null), }; - if (enableScrubbing) { + if (effectiveEnableScrubbing) { return { ...rootProps, onStartShouldSetResponder: claimTouchResponder, @@ -644,56 +664,59 @@ export const CartesianChart = memo( } return rootProps; - }, [ref, height, rootStyles, width, props, enableScrubbing]); + }, [ref, height, rootStyles, width, props, effectiveEnableScrubbing]); + + const chartCanvas = ( + + {children} + + ); + + // Screen-reader scrubbing affordance is only meaningful for interactive charts. + const scrubberAccessibility = isInteractive ? ( + + ) : null; + + const body = legend ? ( + + {(legendPosition === 'top' || legendPosition === 'left') && legendElement} + + {chartCanvas} + {scrubberAccessibility} + + {(legendPosition === 'bottom' || legendPosition === 'right') && legendElement} + + ) : ( + + {chartCanvas} + {scrubberAccessibility} + + ); return ( - - {legend ? ( - - {(legendPosition === 'top' || legendPosition === 'left') && legendElement} - - - {children} - - - - {(legendPosition === 'bottom' || legendPosition === 'right') && legendElement} - - ) : ( - - - {children} - - - - )} - + {isInteractive ? ( + + {body} + + ) : ( + body + )} ); }, diff --git a/packages/mobile/src/visualizations/chart/Path.tsx b/packages/mobile/src/visualizations/chart/Path.tsx index 4fd78a16a2..16c5e1ed22 100644 --- a/packages/mobile/src/visualizations/chart/Path.tsx +++ b/packages/mobile/src/visualizations/chart/Path.tsx @@ -208,7 +208,11 @@ const AnimatedPath = memo< }, ); -export const Path = memo((props) => { +/** + * Animated chart path: holds the reanimated shared values, clip-reveal interpolation, and the + * anti-aliased path clip. Used when the chart animates (the interactive default). + */ +const AnimatedChartPath = memo((props) => { const { animate: animateProp, clipRect, @@ -349,39 +353,7 @@ export const Path = memo((props) => { return undefined; }, [clipPathProp, animateClip, targetClipPath]); - // Convert SVG path string to SkPath for static rendering - const staticPath = useDerivedValue(() => { - const dValue = unwrapAnimatedValue(d); - if (!dValue) return Skia.Path.Make(); - return Skia.Path.MakeFromSVGString(dValue) ?? Skia.Path.Make(); - }, [d]); - - const isFilled = fill !== undefined && fill !== 'none'; - const isStroked = stroke !== undefined && stroke !== 'none'; - - const content = !animate ? ( - <> - {isFilled && ( - - {children} - - )} - {isStroked && ( - - {children} - - )} - - ) : ( + const content = ( ((props) => { ); }); + +/** + * Static chart path: no reanimated hooks and a cheap rectangular clip (routed to + * `canvas.clipRect`) instead of the anti-aliased path clip. Used when the chart does not animate + * (e.g. `interactive={false}`), keeping per-instance cost low when many charts recycle in a list. + */ +const StaticChartPath = memo( + ({ + clipRect, + clipPath: clipPathProp, + clipOffset = 0, + d = '', + fill, + fillOpacity, + stroke, + strokeOpacity, + strokeWidth, + strokeCap, + strokeJoin, + children, + // Static render ignores animation-only props. + animate: _animate, + initialPath: _initialPath, + transition: _transition, + transitions: _transitions, + ...pathProps + }) => { + const context = useCartesianChartContext(); + const rect = clipRect ?? context.drawingArea; + + const path = useMemo(() => { + const dValue = unwrapAnimatedValue(d); + if (!dValue) return Skia.Path.Make(); + return Skia.Path.MakeFromSVGString(dValue) ?? Skia.Path.Make(); + }, [d]); + + // A rectangular clip routes to canvas.clipRect (a cheap GPU scissor) rather than the + // anti-aliased path clip, while still constraining curve overshoot to the drawing area. + const clip = useMemo(() => { + if (clipPathProp !== undefined) return clipPathProp; + if (!rect) return null; + return { + x: rect.x - clipOffset, + y: rect.y - clipOffset, + width: rect.width + clipOffset * 2, + height: rect.height + clipOffset * 2, + }; + }, [clipPathProp, rect, clipOffset]); + + const isFilled = fill !== undefined && fill !== 'none'; + const isStroked = stroke !== undefined && stroke !== 'none'; + + const content = ( + <> + {isFilled && ( + + {children} + + )} + {isStroked && ( + + {children} + + )} + + ); + + if (clip === null) { + return {content}; + } + + return {content}; + }, +); + +/** + * Renders a chart path. Delegates to a lightweight static renderer when the chart is not + * animating (no reanimated hooks, cheap rect clip) and to the animated renderer otherwise. + */ +export const Path = memo((props) => { + const context = useCartesianChartContext(); + const animate = props.animate ?? context.animate; + + return animate ? : ; +}); diff --git a/packages/mobile/src/visualizations/chart/__tests__/Path.test.tsx b/packages/mobile/src/visualizations/chart/__tests__/Path.test.tsx new file mode 100644 index 0000000000..6b27cd5efe --- /dev/null +++ b/packages/mobile/src/visualizations/chart/__tests__/Path.test.tsx @@ -0,0 +1,80 @@ +import { render, screen } from '@testing-library/react-native'; + +import { useCartesianChartContext } from '../ChartProvider'; +import { Path } from '../Path'; + +type MockSkPath = { type: string; addRect: jest.Mock }; + +const makePath = (): MockSkPath => ({ type: 'SkPath', addRect: jest.fn() }); + +jest.mock('@shopify/react-native-skia', () => { + const React = require('react'); + const { View } = require('react-native'); + return { + // Surface the `clip` prop so tests can distinguish a rect clip from an SkPath clip. + Group: ({ children, clip }: { children?: React.ReactNode; clip?: unknown }) => + React.createElement(View, { testID: 'group', clip }, children), + Path: ({ style }: { style?: string }) => React.createElement(View, { testID: `path-${style}` }), + Skia: { + Path: { + Make: jest.fn(makePath), + MakeFromSVGString: jest.fn((str: string) => ({ ...makePath(), svgString: str })), + }, + }, + usePathInterpolation: jest.fn(() => makePath()), + }; +}); + +jest.mock('react-native-reanimated', () => ({ + ...jest.requireActual('react-native-reanimated/mock'), + isSharedValue: jest.fn(() => false), + useSharedValue: jest.fn((v: unknown) => ({ value: v })), + useDerivedValue: jest.fn((fn: () => unknown) => ({ value: fn() })), +})); + +jest.mock('../ChartProvider', () => ({ useCartesianChartContext: jest.fn() })); + +const mockedUseContext = useCartesianChartContext as unknown as jest.Mock; + +const drawingArea = { x: 0, y: 0, width: 100, height: 40 }; + +function mockContext(animate: boolean) { + mockedUseContext.mockReturnValue({ + animate, + layout: 'vertical', + drawingArea, + getXScale: () => (value: number) => value, + }); +} + +describe('Path interactive/static rendering', () => { + afterEach(() => jest.clearAllMocks()); + + it('uses a cheap rectangular clip (not a path clip) when the chart is not animating', () => { + mockContext(false); + render(); + + const clip = screen.getByTestId('group').props.clip; + // A rect clip is a plain object with numeric bounds; it routes to canvas.clipRect. + expect(typeof clip.width).toBe('number'); + expect(typeof clip.height).toBe('number'); + expect(clip.type).toBeUndefined(); + }); + + it('honors an explicit clipPath in static mode', () => { + mockContext(false); + render(); + + // clipPath={null} disables clipping entirely. + expect(screen.getByTestId('group').props.clip).toBeUndefined(); + }); + + it('renders the animated path (SkPath clip) when the chart is animating', () => { + mockContext(true); + render(); + + const clip = screen.getByTestId('group').props.clip; + // The animated renderer clips with an SkPath, not a plain rect. + expect(clip?.type ?? clip?.svgString !== undefined).toBeTruthy(); + }); +}); diff --git a/packages/mobile/src/visualizations/chart/line/LineChart.tsx b/packages/mobile/src/visualizations/chart/line/LineChart.tsx index 3f5479a0af..f694060d3a 100644 --- a/packages/mobile/src/visualizations/chart/line/LineChart.tsx +++ b/packages/mobile/src/visualizations/chart/line/LineChart.tsx @@ -96,6 +96,13 @@ export type LineChartProps = LineChartBaseProps & scrubberAccessibilityLabelStep?: number; }; +/** + * A line chart built on `CartesianChart`. + * + * For non-interactive, decorative usage (e.g. row sparklines in scrolling lists), pass + * `interactive={false}` to render a lightweight static line — no scrubbing, no entrance + * animation, and a cheap rectangular clip instead of the anti-aliased path clip. + */ export const LineChart = memo( ({ ref, diff --git a/packages/mobile/src/visualizations/chart/line/__stories__/LineChart.stories.tsx b/packages/mobile/src/visualizations/chart/line/__stories__/LineChart.stories.tsx index d20a84e017..40c837326b 100644 --- a/packages/mobile/src/visualizations/chart/line/__stories__/LineChart.stories.tsx +++ b/packages/mobile/src/visualizations/chart/line/__stories__/LineChart.stories.tsx @@ -1999,6 +1999,21 @@ function ExampleNavigator() { /> ), }, + { + title: 'Lightweight (Static)', + component: ( + + ), + }, { title: 'Horizontal Layout', component: , diff --git a/packages/mobile/src/visualizations/chart/line/__tests__/LineChart.test.tsx b/packages/mobile/src/visualizations/chart/line/__tests__/LineChart.test.tsx new file mode 100644 index 0000000000..71da6007ad --- /dev/null +++ b/packages/mobile/src/visualizations/chart/line/__tests__/LineChart.test.tsx @@ -0,0 +1,113 @@ +import { render, screen } from '@testing-library/react-native'; + +import { DefaultThemeProvider } from '../../../../utils/testHelpers'; +import { LineChart } from '../LineChart'; + +type MockSkPath = { type: string; addRect: jest.Mock; interpolate: jest.Mock }; + +const makePath = (): MockSkPath => ({ + type: 'SkPath', + addRect: jest.fn(), + interpolate: jest.fn(() => makePath()), +}); + +jest.mock('@shopify/react-native-skia', () => { + const React = require('react'); + const { View } = require('react-native'); + return { + Canvas: ({ children, style }: { children: React.ReactNode; style?: unknown }) => + React.createElement(View, { style, testID: 'skia-canvas' }, children), + Group: ({ children }: { children?: React.ReactNode }) => children ?? null, + Path: () => null, + ClipOp: { Intersect: 0 }, + Skia: { + Path: { + Make: jest.fn(makePath), + MakeFromSVGString: jest.fn((str: string) => ({ ...makePath(), svgString: str })), + }, + TypefaceFontProvider: { Make: jest.fn(() => ({})) }, + }, + usePathInterpolation: jest.fn(() => makePath()), + notifyChange: jest.fn(), + }; +}); + +jest.mock('react-native-reanimated', () => ({ + ...jest.requireActual('react-native-reanimated/mock'), + useSharedValue: jest.fn((v: unknown) => ({ value: v })), +})); + +jest.mock('../../ChartContextBridge', () => { + const React = require('react'); + return { + ChartBridgeProvider: ({ children }: { children: React.ReactNode }) => children, + useChartContextBridge: + () => + ({ children }: { children: React.ReactNode }) => + children, + }; +}); + +// Surface whether the scrubber context is mounted — the interactive-only machinery. +jest.mock('../../scrubber/ScrubberProvider', () => { + const React = require('react'); + const { View } = require('react-native'); + return { + ScrubberProvider: ({ children }: { children: React.ReactNode }) => + React.createElement(View, { testID: 'scrubber-provider' }, children), + }; +}); + +// Renders null in tests (screen reader off); mock it so it doesn't need the real ScrubberContext. +jest.mock('../../scrubber/ScrubberAccessibilityView', () => ({ + ScrubberAccessibilityView: () => null, +})); + +const series = [{ id: 'a', data: [1, 2, 3, 2, 4], color: 'green' }]; + +describe('LineChart interactive mode', () => { + it('mounts the scrubber provider by default (interactive)', () => { + render( + + + , + ); + + expect(screen.getByTestId('scrubber-provider')).toBeTruthy(); + }); + + it('skips the scrubber provider when interactive={false}', () => { + render( + + + , + ); + + expect(screen.queryByTestId('scrubber-provider')).toBeNull(); + // The chart shell still renders. + expect(screen.getByTestId('skia-canvas')).toBeTruthy(); + }); + + it('keeps scrubbing off under interactive={false} even if enableScrubbing is set', () => { + render( + + + , + ); + + expect(screen.queryByTestId('scrubber-provider')).toBeNull(); + }); +}); From b518fc81ce0cbf995b96f8f8653eedaf6c5cbb97 Mon Sep 17 00:00:00 2001 From: Alfonso Curbelo Date: Tue, 11 Aug 2026 13:41:51 -0400 Subject: [PATCH 2/3] =?UTF-8?q?refactor(charts):=20address=20review=20?= =?UTF-8?q?=E2=80=94=20drop=20interactive=20prop,=20optimize=20ScrubberPro?= =?UTF-8?q?vider?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove the `interactive` prop; the lightweight static render now rides on the existing `animate={false}` (Path routes to a reanimated-free StaticChartPath with a cheap rect clip). - ScrubberProvider skips the pan gesture + animated reaction when scrubbing is disabled. - Add a `strokeWidth` prop to ReferenceLine; narrow StaticChartPath props (review nit). - Update docs, story (animate={false}), and tests; add ScrubberProvider test. - Bump @coinbase/cds-mobile to 9.13.0 (+ release sync). Linear: APP-770 Co-Authored-By: Claude --- packages/common/CHANGELOG.md | 4 + packages/common/package.json | 2 +- packages/mcp-server/CHANGELOG.md | 4 + packages/mcp-server/package.json | 2 +- packages/mobile/CHANGELOG.md | 6 + packages/mobile/package.json | 2 +- .../visualizations/chart/CartesianChart.tsx | 130 ++++++++---------- .../mobile/src/visualizations/chart/Path.tsx | 38 +++-- .../visualizations/chart/line/LineChart.tsx | 5 +- .../chart/line/ReferenceLine.tsx | 8 ++ .../line/__stories__/LineChart.stories.tsx | 10 +- .../chart/line/__tests__/LineChart.test.tsx | 57 +------- .../chart/scrubber/ScrubberProvider.tsx | 34 ++++- .../__tests__/ScrubberProvider.test.tsx | 81 +++++++++++ packages/web/CHANGELOG.md | 4 + packages/web/package.json | 2 +- 16 files changed, 236 insertions(+), 153 deletions(-) create mode 100644 packages/mobile/src/visualizations/chart/scrubber/__tests__/ScrubberProvider.test.tsx diff --git a/packages/common/CHANGELOG.md b/packages/common/CHANGELOG.md index e251708ce3..ae0ef08baa 100644 --- a/packages/common/CHANGELOG.md +++ b/packages/common/CHANGELOG.md @@ -8,6 +8,10 @@ All notable changes to this project will be documented in this file. +## 9.13.0 ((8/11/2026, 09:52 AM PST)) + +This is an artificial version bump with no new change. + ## 9.12.3 ((8/10/2026, 09:17 AM PST)) This is an artificial version bump with no new change. diff --git a/packages/common/package.json b/packages/common/package.json index 2d7e739104..b26cbf22a1 100644 --- a/packages/common/package.json +++ b/packages/common/package.json @@ -1,6 +1,6 @@ { "name": "@coinbase/cds-common", - "version": "9.12.3", + "version": "9.13.0", "description": "Coinbase Design System - Common", "repository": { "type": "git", diff --git a/packages/mcp-server/CHANGELOG.md b/packages/mcp-server/CHANGELOG.md index 595629d00f..8a419e93e6 100644 --- a/packages/mcp-server/CHANGELOG.md +++ b/packages/mcp-server/CHANGELOG.md @@ -8,6 +8,10 @@ All notable changes to this project will be documented in this file. +## 9.13.0 ((8/11/2026, 09:52 AM PST)) + +This is an artificial version bump with no new change. + ## 9.12.3 ((8/10/2026, 09:17 AM PST)) This is an artificial version bump with no new change. diff --git a/packages/mcp-server/package.json b/packages/mcp-server/package.json index 3e19277abd..6d159dea5d 100644 --- a/packages/mcp-server/package.json +++ b/packages/mcp-server/package.json @@ -1,6 +1,6 @@ { "name": "@coinbase/cds-mcp-server", - "version": "9.12.3", + "version": "9.13.0", "description": "Coinbase Design System - MCP Server", "repository": { "type": "git", diff --git a/packages/mobile/CHANGELOG.md b/packages/mobile/CHANGELOG.md index a63a562b47..391b8861a5 100644 --- a/packages/mobile/CHANGELOG.md +++ b/packages/mobile/CHANGELOG.md @@ -8,6 +8,12 @@ All notable changes to this project will be documented in this file. +## 9.13.0 (8/11/2026 PST) + +#### 🚀 Updates + +- Add a lightweight static render path for non-animated LineChart/CartesianChart (animate={false} uses a cheap rectangular clip instead of the anti-aliased path clip); ScrubberProvider now skips the pan gesture and animated reaction when scrubbing is disabled; add a strokeWidth prop to ReferenceLine. [[#840](https://github.com/coinbase/cds/pull/840)] + ## 9.12.3 (8/10/2026 PST) #### 🐞 Fixes diff --git a/packages/mobile/package.json b/packages/mobile/package.json index 6148726fd0..b721ba58ff 100644 --- a/packages/mobile/package.json +++ b/packages/mobile/package.json @@ -1,6 +1,6 @@ { "name": "@coinbase/cds-mobile", - "version": "9.12.3", + "version": "9.13.0", "description": "Coinbase Design System - Mobile", "repository": { "type": "git", diff --git a/packages/mobile/src/visualizations/chart/CartesianChart.tsx b/packages/mobile/src/visualizations/chart/CartesianChart.tsx index 12d21d2772..064ecf471c 100644 --- a/packages/mobile/src/visualizations/chart/CartesianChart.tsx +++ b/packages/mobile/src/visualizations/chart/CartesianChart.tsx @@ -118,24 +118,13 @@ export type CartesianChartBaseProps = Omit & */ layout?: CartesianChartLayout; /** - * Whether to animate the chart. + * Whether to animate the chart. When `false`, lines render via a lightweight static path — a + * cheap rectangular clip instead of the anti-aliased path clip that pushes rendering onto + * Skia's CPU path renderer — which is well-suited to non-interactive, decorative charts such + * as row sparklines in scrolling lists. * @default true */ animate?: boolean; - /** - * Whether the chart is interactive. - * - * When `false`, renders a lightweight, static chart for non-interactive or decorative - * contexts (e.g. row sparklines in scrolling lists): scrubbing is disabled, the entrance - * animation is skipped, and lines use a cheap rectangular clip instead of the anti-aliased - * path clip that pushes rendering onto Skia's CPU path renderer. - * - * @note Forces `animate` off at the chart level (a per-`Line`/`Path` `animate` override still - * wins). Scrubbing requires `interactive`; nesting a `Scrubber` under `interactive={false}` is - * unsupported. - * @default true - */ - interactive?: boolean; /** * Configuration for x-axis(es). Can be a single config or array of configs. * @@ -221,7 +210,6 @@ export const CartesianChart = memo( children, layout = 'vertical', animate = true, - interactive = true, enableScrubbing, getScrubberAccessibilityLabel, scrubberAccessibilityLabelStep, @@ -254,11 +242,6 @@ export const CartesianChart = memo( }) => { const [containerLayout, onContainerLayout] = useChartLayout(); - // Non-interactive charts skip scrubbing + entrance animation for a lightweight static render. - const isInteractive = interactive !== false; - const effectiveAnimate = isInteractive ? animate : false; - const effectiveEnableScrubbing = isInteractive ? enableScrubbing : false; - const chartWidth = containerLayout.width; const chartHeight = containerLayout.height; @@ -579,7 +562,7 @@ export const CartesianChart = memo( series: series ?? [], getSeries, getSeriesData: getStackedSeriesData, - animate: effectiveAnimate, + animate, width: chartWidth, height: chartHeight, fontFamilies, @@ -601,7 +584,7 @@ export const CartesianChart = memo( series, getSeries, getStackedSeriesData, - effectiveAnimate, + animate, chartWidth, chartHeight, fontFamilies, @@ -647,7 +630,7 @@ export const CartesianChart = memo( width, ...props, // Claim RN responder so parent PanResponders (e.g. Tray) don't steal the touch. - ...(effectiveEnableScrubbing + ...(enableScrubbing ? { onStartShouldSetResponder: claimTouchResponder, onMoveShouldSetResponder: claimTouchResponder, @@ -655,7 +638,7 @@ export const CartesianChart = memo( : null), }; - if (effectiveEnableScrubbing) { + if (enableScrubbing) { return { ...rootProps, onStartShouldSetResponder: claimTouchResponder, @@ -664,59 +647,56 @@ export const CartesianChart = memo( } return rootProps; - }, [ref, height, rootStyles, width, props, effectiveEnableScrubbing]); - - const chartCanvas = ( - - {children} - - ); - - // Screen-reader scrubbing affordance is only meaningful for interactive charts. - const scrubberAccessibility = isInteractive ? ( - - ) : null; - - const body = legend ? ( - - {(legendPosition === 'top' || legendPosition === 'left') && legendElement} - - {chartCanvas} - {scrubberAccessibility} - - {(legendPosition === 'bottom' || legendPosition === 'right') && legendElement} - - ) : ( - - {chartCanvas} - {scrubberAccessibility} - - ); + }, [ref, height, rootStyles, width, props, enableScrubbing]); return ( - {isInteractive ? ( - - {body} - - ) : ( - body - )} + + {legend ? ( + + {(legendPosition === 'top' || legendPosition === 'left') && legendElement} + + + {children} + + + + {(legendPosition === 'bottom' || legendPosition === 'right') && legendElement} + + ) : ( + + + {children} + + + + )} + ); }, diff --git a/packages/mobile/src/visualizations/chart/Path.tsx b/packages/mobile/src/visualizations/chart/Path.tsx index 16c5e1ed22..e285620bee 100644 --- a/packages/mobile/src/visualizations/chart/Path.tsx +++ b/packages/mobile/src/visualizations/chart/Path.tsx @@ -210,7 +210,7 @@ const AnimatedPath = memo< /** * Animated chart path: holds the reanimated shared values, clip-reveal interpolation, and the - * anti-aliased path clip. Used when the chart animates (the interactive default). + * anti-aliased path clip. Used when the chart animates (the default). */ const AnimatedChartPath = memo((props) => { const { @@ -392,9 +392,11 @@ const AnimatedChartPath = memo((props) => { /** * Static chart path: no reanimated hooks and a cheap rectangular clip (routed to * `canvas.clipRect`) instead of the anti-aliased path clip. Used when the chart does not animate - * (e.g. `interactive={false}`), keeping per-instance cost low when many charts recycle in a list. + * (e.g. `animate={false}`), keeping per-instance cost low when many charts recycle in a list. */ -const StaticChartPath = memo( +const StaticChartPath = memo< + Omit +>( ({ clipRect, clipPath: clipPathProp, @@ -408,11 +410,6 @@ const StaticChartPath = memo( strokeCap, strokeJoin, children, - // Static render ignores animation-only props. - animate: _animate, - initialPath: _initialPath, - transition: _transition, - transitions: _transitions, ...pathProps }) => { const context = useCartesianChartContext(); @@ -476,9 +473,24 @@ const StaticChartPath = memo( * Renders a chart path. Delegates to a lightweight static renderer when the chart is not * animating (no reanimated hooks, cheap rect clip) and to the animated renderer otherwise. */ -export const Path = memo((props) => { - const context = useCartesianChartContext(); - const animate = props.animate ?? context.animate; +export const Path = memo( + ({ animate: animateProp, initialPath, transition, transitions, ...staticProps }) => { + const context = useCartesianChartContext(); + const animate = animateProp ?? context.animate; + + if (animate) { + return ( + + ); + } - return animate ? : ; -}); + // Animation-only props are omitted from the static renderer. + return ; + }, +); diff --git a/packages/mobile/src/visualizations/chart/line/LineChart.tsx b/packages/mobile/src/visualizations/chart/line/LineChart.tsx index f694060d3a..4fba164473 100644 --- a/packages/mobile/src/visualizations/chart/line/LineChart.tsx +++ b/packages/mobile/src/visualizations/chart/line/LineChart.tsx @@ -100,8 +100,9 @@ export type LineChartProps = LineChartBaseProps & * A line chart built on `CartesianChart`. * * For non-interactive, decorative usage (e.g. row sparklines in scrolling lists), pass - * `interactive={false}` to render a lightweight static line — no scrubbing, no entrance - * animation, and a cheap rectangular clip instead of the anti-aliased path clip. + * `animate={false}` to render a lightweight static line — a cheap rectangular clip instead of the + * anti-aliased path clip, and no entrance animation. Scrubbing is off unless enabled via + * `enableScrubbing`. */ export const LineChart = memo( ({ diff --git a/packages/mobile/src/visualizations/chart/line/ReferenceLine.tsx b/packages/mobile/src/visualizations/chart/line/ReferenceLine.tsx index d985f496f3..2e02c3fb72 100644 --- a/packages/mobile/src/visualizations/chart/line/ReferenceLine.tsx +++ b/packages/mobile/src/visualizations/chart/line/ReferenceLine.tsx @@ -110,6 +110,11 @@ export type ReferenceLineBaseProps = { * @default theme.color.bgLine */ stroke?: string; + /** + * Width of the line. + * @default the line component's default (2) + */ + strokeWidth?: number; /** * Opacity applied to both the line and label. * @default 1 @@ -169,6 +174,7 @@ export const ReferenceLine = memo( labelVerticalAlignment, labelBoundsInset, stroke, + strokeWidth, opacity = 1, }) => { const theme = useTheme(); @@ -235,6 +241,7 @@ export const ReferenceLine = memo( d={horizontalLine} stroke={effectiveLineStroke} strokeOpacity={opacity} + strokeWidth={strokeWidth} /> {label && ( ( d={verticalLine} stroke={effectiveLineStroke} strokeOpacity={opacity} + strokeWidth={strokeWidth} /> {label && ( + yAxis={{ domain: { min: -40, max: 45 } }} + > + {/* Curve weaves through the reference line (negative + positive values). */} + + ), }, { diff --git a/packages/mobile/src/visualizations/chart/line/__tests__/LineChart.test.tsx b/packages/mobile/src/visualizations/chart/line/__tests__/LineChart.test.tsx index 71da6007ad..c009858863 100644 --- a/packages/mobile/src/visualizations/chart/line/__tests__/LineChart.test.tsx +++ b/packages/mobile/src/visualizations/chart/line/__tests__/LineChart.test.tsx @@ -34,6 +34,7 @@ jest.mock('@shopify/react-native-skia', () => { jest.mock('react-native-reanimated', () => ({ ...jest.requireActual('react-native-reanimated/mock'), + isSharedValue: jest.fn(() => false), useSharedValue: jest.fn((v: unknown) => ({ value: v })), })); @@ -48,66 +49,16 @@ jest.mock('../../ChartContextBridge', () => { }; }); -// Surface whether the scrubber context is mounted — the interactive-only machinery. -jest.mock('../../scrubber/ScrubberProvider', () => { - const React = require('react'); - const { View } = require('react-native'); - return { - ScrubberProvider: ({ children }: { children: React.ReactNode }) => - React.createElement(View, { testID: 'scrubber-provider' }, children), - }; -}); - -// Renders null in tests (screen reader off); mock it so it doesn't need the real ScrubberContext. -jest.mock('../../scrubber/ScrubberAccessibilityView', () => ({ - ScrubberAccessibilityView: () => null, -})); - const series = [{ id: 'a', data: [1, 2, 3, 2, 4], color: 'green' }]; -describe('LineChart interactive mode', () => { - it('mounts the scrubber provider by default (interactive)', () => { - render( - - - , - ); - - expect(screen.getByTestId('scrubber-provider')).toBeTruthy(); - }); - - it('skips the scrubber provider when interactive={false}', () => { +describe('LineChart', () => { + it('renders a static (animate=false) chart shell', () => { render( - + , ); - expect(screen.queryByTestId('scrubber-provider')).toBeNull(); - // The chart shell still renders. expect(screen.getByTestId('skia-canvas')).toBeTruthy(); }); - - it('keeps scrubbing off under interactive={false} even if enableScrubbing is set', () => { - render( - - - , - ); - - expect(screen.queryByTestId('scrubber-provider')).toBeNull(); - }); }); diff --git a/packages/mobile/src/visualizations/chart/scrubber/ScrubberProvider.tsx b/packages/mobile/src/visualizations/chart/scrubber/ScrubberProvider.tsx index baa52ff3fc..8087ee090a 100644 --- a/packages/mobile/src/visualizations/chart/scrubber/ScrubberProvider.tsx +++ b/packages/mobile/src/visualizations/chart/scrubber/ScrubberProvider.tsx @@ -23,10 +23,11 @@ export type ScrubberProviderProps = Partial = ({ +const EnabledScrubberProvider: React.FC = ({ children, enableScrubbing, onScrubberPositionChange, @@ -192,3 +193,30 @@ export const ScrubberProvider: React.FC = ({ return content; }; + +/** + * The scrubbing-disabled provider: supplies the ScrubberContext without the pan gesture or the + * animated reaction, so non-interactive charts (the common case) don't pay that per-instance cost. + */ +const DisabledScrubberProvider: React.FC<{ children: React.ReactNode }> = ({ children }) => { + const scrubberPosition = useSharedValue(undefined); + + const contextValue = useMemo( + () => ({ enableScrubbing: false, scrubberPosition }), + [scrubberPosition], + ); + + return {children}; +}; + +/** + * A component which encapsulates the ScrubberContext. + * It depends on a ChartContext in order to provide accurate touch tracking. + */ +export const ScrubberProvider: React.FC = (props) => { + if (props.enableScrubbing) { + return ; + } + + return {props.children}; +}; diff --git a/packages/mobile/src/visualizations/chart/scrubber/__tests__/ScrubberProvider.test.tsx b/packages/mobile/src/visualizations/chart/scrubber/__tests__/ScrubberProvider.test.tsx new file mode 100644 index 0000000000..8eb32f514b --- /dev/null +++ b/packages/mobile/src/visualizations/chart/scrubber/__tests__/ScrubberProvider.test.tsx @@ -0,0 +1,81 @@ +import { Text } from 'react-native'; +import { render, screen } from '@testing-library/react-native'; + +import { useCartesianChartContext } from '../../ChartProvider'; +import { ScrubberProvider } from '../ScrubberProvider'; + +jest.mock('react-native-gesture-handler', () => { + const React = require('react'); + const { View } = require('react-native'); + // Chainable Gesture.Pan() mock — every builder method returns the gesture. + const makeGesture = () => { + const gesture: Record unknown> = {}; + for (const method of [ + 'activateAfterLongPress', + 'shouldCancelWhenOutside', + 'failOffsetY', + 'failOffsetX', + 'onStart', + 'onUpdate', + 'onEnd', + 'onTouchesCancelled', + ]) { + gesture[method] = () => gesture; + } + return gesture; + }; + return { + Gesture: { Pan: makeGesture }, + GestureDetector: ({ children }: { children: React.ReactNode }) => + React.createElement(View, { testID: 'gesture-detector' }, children), + }; +}); + +jest.mock('react-native-reanimated', () => ({ + ...jest.requireActual('react-native-reanimated/mock'), + useSharedValue: jest.fn((v: unknown) => ({ value: v })), + useAnimatedReaction: jest.fn(), + runOnJS: (fn: unknown) => fn, +})); + +jest.mock('../../ChartProvider', () => ({ useCartesianChartContext: jest.fn() })); + +const mockedUseContext = useCartesianChartContext as unknown as jest.Mock; + +beforeEach(() => { + jest.clearAllMocks(); + mockedUseContext.mockReturnValue({ + layout: 'vertical', + getXSerializableScale: () => undefined, + getYSerializableScale: () => undefined, + getXAxis: () => undefined, + getYAxis: () => undefined, + }); +}); + +describe('ScrubberProvider', () => { + it('wires up the pan gesture when scrubbing is enabled', () => { + render( + + content + , + ); + + expect(screen.getByTestId('gesture-detector')).toBeTruthy(); + expect(screen.getByTestId('child')).toBeTruthy(); + }); + + it('skips the gesture and animated reaction when scrubbing is disabled', () => { + const { useAnimatedReaction } = require('react-native-reanimated'); + + render( + + content + , + ); + + expect(screen.queryByTestId('gesture-detector')).toBeNull(); + expect(screen.getByTestId('child')).toBeTruthy(); + expect(useAnimatedReaction).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/web/CHANGELOG.md b/packages/web/CHANGELOG.md index 8fdeacb630..5fbc8da6c8 100644 --- a/packages/web/CHANGELOG.md +++ b/packages/web/CHANGELOG.md @@ -8,6 +8,10 @@ All notable changes to this project will be documented in this file. +## 9.13.0 ((8/11/2026, 09:52 AM PST)) + +This is an artificial version bump with no new change. + ## 9.12.3 ((8/10/2026, 09:17 AM PST)) This is an artificial version bump with no new change. diff --git a/packages/web/package.json b/packages/web/package.json index fad7bd555f..e73760e279 100644 --- a/packages/web/package.json +++ b/packages/web/package.json @@ -1,6 +1,6 @@ { "name": "@coinbase/cds-web", - "version": "9.12.3", + "version": "9.13.0", "description": "Coinbase Design System - Web", "repository": { "type": "git", From 1c2c9e408850dc9b8bef25e93da515c0321c9f3f Mon Sep 17 00:00:00 2001 From: Alfonso Curbelo Date: Tue, 11 Aug 2026 13:41:51 -0400 Subject: [PATCH 3/3] refactor(charts): apply review feedback - ScrubberProvider: enabled provider no longer branches on enableScrubbing (always on). - Path: static path uses useDerivedValue so an animated d (e.g. reference lines) tracks scrubbing. - Trim verbose JSDoc/comments per review. Co-Authored-By: Claude --- .../visualizations/chart/CartesianChart.tsx | 6 +-- .../mobile/src/visualizations/chart/Path.tsx | 17 +++----- .../visualizations/chart/line/LineChart.tsx | 8 +--- .../chart/scrubber/ScrubberProvider.tsx | 43 +++++-------------- 4 files changed, 20 insertions(+), 54 deletions(-) diff --git a/packages/mobile/src/visualizations/chart/CartesianChart.tsx b/packages/mobile/src/visualizations/chart/CartesianChart.tsx index 064ecf471c..5f0cf2fab0 100644 --- a/packages/mobile/src/visualizations/chart/CartesianChart.tsx +++ b/packages/mobile/src/visualizations/chart/CartesianChart.tsx @@ -118,10 +118,8 @@ export type CartesianChartBaseProps = Omit & */ layout?: CartesianChartLayout; /** - * Whether to animate the chart. When `false`, lines render via a lightweight static path — a - * cheap rectangular clip instead of the anti-aliased path clip that pushes rendering onto - * Skia's CPU path renderer — which is well-suited to non-interactive, decorative charts such - * as row sparklines in scrolling lists. + * Whether to animate the chart. When `false`, lines render via a lightweight static path + * (a cheap rectangular clip) suited to non-interactive charts like row sparklines. * @default true */ animate?: boolean; diff --git a/packages/mobile/src/visualizations/chart/Path.tsx b/packages/mobile/src/visualizations/chart/Path.tsx index e285620bee..1e85ebefd6 100644 --- a/packages/mobile/src/visualizations/chart/Path.tsx +++ b/packages/mobile/src/visualizations/chart/Path.tsx @@ -208,10 +208,7 @@ const AnimatedPath = memo< }, ); -/** - * Animated chart path: holds the reanimated shared values, clip-reveal interpolation, and the - * anti-aliased path clip. Used when the chart animates (the default). - */ +// Animated path: reanimated clip-reveal + shared values. Used when the chart animates (default). const AnimatedChartPath = memo((props) => { const { animate: animateProp, @@ -389,11 +386,7 @@ const AnimatedChartPath = memo((props) => { ); }); -/** - * Static chart path: no reanimated hooks and a cheap rectangular clip (routed to - * `canvas.clipRect`) instead of the anti-aliased path clip. Used when the chart does not animate - * (e.g. `animate={false}`), keeping per-instance cost low when many charts recycle in a list. - */ +// Non-animated path: skips the clip-reveal animation and uses a cheap rectangular clip. const StaticChartPath = memo< Omit >( @@ -415,14 +408,14 @@ const StaticChartPath = memo< const context = useCartesianChartContext(); const rect = clipRect ?? context.drawingArea; - const path = useMemo(() => { + // Derived (not memoized) so an animated `d` — e.g. a reference line — still tracks scrubbing. + const path = useDerivedValue(() => { const dValue = unwrapAnimatedValue(d); if (!dValue) return Skia.Path.Make(); return Skia.Path.MakeFromSVGString(dValue) ?? Skia.Path.Make(); }, [d]); - // A rectangular clip routes to canvas.clipRect (a cheap GPU scissor) rather than the - // anti-aliased path clip, while still constraining curve overshoot to the drawing area. + // Rect clip routes to canvas.clipRect (a cheap GPU scissor), not the anti-aliased path clip. const clip = useMemo(() => { if (clipPathProp !== undefined) return clipPathProp; if (!rect) return null; diff --git a/packages/mobile/src/visualizations/chart/line/LineChart.tsx b/packages/mobile/src/visualizations/chart/line/LineChart.tsx index 4fba164473..404e996a97 100644 --- a/packages/mobile/src/visualizations/chart/line/LineChart.tsx +++ b/packages/mobile/src/visualizations/chart/line/LineChart.tsx @@ -97,12 +97,8 @@ export type LineChartProps = LineChartBaseProps & }; /** - * A line chart built on `CartesianChart`. - * - * For non-interactive, decorative usage (e.g. row sparklines in scrolling lists), pass - * `animate={false}` to render a lightweight static line — a cheap rectangular clip instead of the - * anti-aliased path clip, and no entrance animation. Scrubbing is off unless enabled via - * `enableScrubbing`. + * A line chart built on `CartesianChart`. For non-interactive usage (e.g. row sparklines), pass + * `animate={false}` for a lightweight static render (cheap rectangular clip, no entrance animation). */ export const LineChart = memo( ({ diff --git a/packages/mobile/src/visualizations/chart/scrubber/ScrubberProvider.tsx b/packages/mobile/src/visualizations/chart/scrubber/ScrubberProvider.tsx index 8087ee090a..a1b77bd9d2 100644 --- a/packages/mobile/src/visualizations/chart/scrubber/ScrubberProvider.tsx +++ b/packages/mobile/src/visualizations/chart/scrubber/ScrubberProvider.tsx @@ -22,14 +22,9 @@ export type ScrubberProviderProps = Partial void; }; -/** - * The scrubbing-enabled provider: sets up the pan gesture, touch tracking, and the animated - * reaction that reports scrubber position. Only mounted when scrubbing is enabled so the - * reanimated + gesture allocations are skipped for static/non-interactive charts. - */ +// Sets up the pan gesture + animated reaction. Only mounted when scrubbing is enabled. const EnabledScrubberProvider: React.FC = ({ children, - enableScrubbing, onScrubberPositionChange, allowOverflowGestures, }) => { @@ -155,15 +150,11 @@ const EnabledScrubberProvider: React.FC = ({ } }) .onEnd(function onEnd() { - if (enableScrubbing) { - runOnJS(handleStartEndHaptics)(); - scrubberPosition.value = undefined; - } + runOnJS(handleStartEndHaptics)(); + scrubberPosition.value = undefined; }) .onTouchesCancelled(function onTouchesCancelled() { - if (enableScrubbing) { - scrubberPosition.value = undefined; - } + scrubberPosition.value = undefined; }); }, [ allowOverflowGestures, @@ -171,33 +162,21 @@ const EnabledScrubberProvider: React.FC = ({ getDataIndexFromPosition, categoryAxisIsX, scrubberPosition, - enableScrubbing, ]); const contextValue: ScrubberContextValue = useMemo( - () => ({ - enableScrubbing: !!enableScrubbing, - scrubberPosition, - }), - [enableScrubbing, scrubberPosition], + () => ({ enableScrubbing: true, scrubberPosition }), + [scrubberPosition], ); - const content = ( - {children} + return ( + + {children} + ); - - // Wrap with gesture handler only if scrubbing is enabled - if (enableScrubbing) { - return {content}; - } - - return content; }; -/** - * The scrubbing-disabled provider: supplies the ScrubberContext without the pan gesture or the - * animated reaction, so non-interactive charts (the common case) don't pay that per-instance cost. - */ +// Supplies the ScrubberContext without the gesture/animated reaction (the common, disabled case). const DisabledScrubberProvider: React.FC<{ children: React.ReactNode }> = ({ children }) => { const scrubberPosition = useSharedValue(undefined);