From 62ec80603825a2b23ba363f4477faade1ddb2641 Mon Sep 17 00:00:00 2001 From: Alex Fedotyev <61838744+alex-fedotyev@users.noreply.github.com> Date: Thu, 25 Jun 2026 01:50:10 +0000 Subject: [PATCH 1/2] refactor(dashboards): extract shared Sparkline primitive from number tile Lift the chrome-less recharts core out of NumberTileBackgroundChart into a reusable (line / area / bar) and render the number tile's background trend through it. No user-visible change: the number tile issues the same query and renders the same faint line / area behind the value. The upcoming table-cell trend feature needs the identical render shell per cell, so a shared primitive is the alternative to a second parallel chart component. It also answers the reuse question raised on #2489. Co-Authored-By: Claude Opus 4.8 --- .../components/NumberTileBackgroundChart.tsx | 54 +----------- packages/app/src/components/Sparkline.tsx | 88 +++++++++++++++++++ .../components/__tests__/Sparkline.test.tsx | 69 +++++++++++++++ 3 files changed, 160 insertions(+), 51 deletions(-) create mode 100644 packages/app/src/components/Sparkline.tsx create mode 100644 packages/app/src/components/__tests__/Sparkline.test.tsx diff --git a/packages/app/src/components/NumberTileBackgroundChart.tsx b/packages/app/src/components/NumberTileBackgroundChart.tsx index 0a7ad0b98b..5ea3b83214 100644 --- a/packages/app/src/components/NumberTileBackgroundChart.tsx +++ b/packages/app/src/components/NumberTileBackgroundChart.tsx @@ -1,12 +1,5 @@ import { useMemo } from 'react'; import { ErrorBoundary } from 'react-error-boundary'; -import { - Area, - AreaChart, - Line, - LineChart, - ResponsiveContainer, -} from 'recharts'; import { isBuilderChartConfig } from '@hyperdx/common-utils/dist/guards'; import { BackgroundChart, @@ -26,23 +19,12 @@ import { useQueriedChartConfig } from '@/hooks/useChartConfig'; import { useSource } from '@/source'; import { getColorFromCSSToken } from '@/utils'; +import { Sparkline, type SparklinePoint } from './Sparkline'; + // Trend hue used when neither the background override nor the tile's static // `color` is set, so a sparkline is always visible once enabled. const DEFAULT_BACKGROUND_TOKEN: ChartPaletteToken = 'chart-blue'; -// The sparkline sits behind the value, so it is intentionally low-contrast: -// a translucent stroke with a fainter area fill. -const STROKE_OPACITY = 0.5; -const STROKE_WIDTH = 2; -const AREA_FILL_OPACITY = 0.15; - -// `y` is the plotted value; `x` (bucket timestamp, seconds) is retained for -// ordering and future axis use. With no ``, recharts spaces points by -// array order, which is already sorted by bucket. -const VALUE_KEY = 'y'; - -type SparklinePoint = { x: number; y: number }; - /** * Flatten the time-chart formatter's `graphResults` into sparkline points. * Number tiles are single-series, so a single value series is read by key. @@ -163,8 +145,6 @@ function NumberTileBackgroundChartInner({ DEFAULT_BACKGROUND_TOKEN, ); - const margin = { top: 4, right: 0, bottom: 0, left: 0 }; - return (
- - {backgroundChart.type === 'area' ? ( - - - - ) : ( - - - - )} - +
); } diff --git a/packages/app/src/components/Sparkline.tsx b/packages/app/src/components/Sparkline.tsx new file mode 100644 index 0000000000..8ef8cf8317 --- /dev/null +++ b/packages/app/src/components/Sparkline.tsx @@ -0,0 +1,88 @@ +import { + Area, + AreaChart, + Bar, + BarChart, + Line, + LineChart, + ResponsiveContainer, +} from 'recharts'; + +type SparklineType = 'line' | 'area' | 'bar'; + +export type SparklinePoint = { x: number; y: number }; + +// `y` is the plotted value; `x` (bucket timestamp, seconds) is retained for +// ordering and future axis use. With no ``, recharts spaces points by +// array order, which callers keep sorted by bucket. +const VALUE_KEY = 'y'; + +// The line / area variants are drawn behind or beside a value, so they are +// intentionally low-contrast: a translucent stroke with a fainter area fill. +const STROKE_OPACITY = 0.5; +const STROKE_WIDTH = 2; +const AREA_FILL_OPACITY = 0.15; + +const CHART_MARGIN = { top: 4, right: 0, bottom: 0, left: 0 }; + +/** + * Chrome-less recharts trend, drawn as a line, area, or bar. No axes, grid, + * legend, or tooltip; dots and animation are off. Fills its parent via + * `ResponsiveContainer`, so the parent owns the dimensions (pass `height` to + * override the default 100%). Renders nothing for fewer than two points, since + * a single point has no trend to draw. + */ +export function Sparkline({ + points, + type, + color, + height = '100%', +}: { + points: SparklinePoint[]; + type: SparklineType; + color: string; + height?: number | string; +}) { + if (points.length < 2) return null; + + return ( + + {type === 'bar' ? ( + + + + ) : type === 'area' ? ( + + + + ) : ( + + + + )} + + ); +} diff --git a/packages/app/src/components/__tests__/Sparkline.test.tsx b/packages/app/src/components/__tests__/Sparkline.test.tsx new file mode 100644 index 0000000000..37abaa0ff1 --- /dev/null +++ b/packages/app/src/components/__tests__/Sparkline.test.tsx @@ -0,0 +1,69 @@ +import React from 'react'; + +import { Sparkline, type SparklinePoint } from '@/components/Sparkline'; + +// recharts `ResponsiveContainer` sizes itself from its parent via a +// ResizeObserver, which is a no-op in jsdom (see setupTests), so it never +// reports a size and the chart never paints. Swap it for a fixed-size +// pass-through so the SVG renders and the variant can be asserted. +jest.mock('recharts', () => { + const actual = jest.requireActual('recharts'); + const { cloneElement } = jest.requireActual('react'); + return { + ...actual, + ResponsiveContainer: ({ + children, + }: { + children: React.ReactElement<{ width?: number; height?: number }>; + }) => cloneElement(children, { width: 300, height: 80 }), + }; +}); + +const COLOR = '#abcdef'; + +const POINTS: SparklinePoint[] = [ + { x: 1, y: 3 }, + { x: 2, y: 7 }, + { x: 3, y: 5 }, +]; + +describe('Sparkline', () => { + it('renders a line trend in the given color', () => { + const { container } = renderWithMantine( + , + ); + expect(container.querySelector('.recharts-line')).toBeInTheDocument(); + expect(container.innerHTML).toContain(COLOR); + }); + + it('renders an area trend in the given color', () => { + const { container } = renderWithMantine( + , + ); + expect(container.querySelector('.recharts-area')).toBeInTheDocument(); + expect(container.innerHTML).toContain(COLOR); + }); + + it('renders a bar trend in the given color', () => { + const { container } = renderWithMantine( + , + ); + expect(container.querySelector('.recharts-bar')).toBeInTheDocument(); + expect(container.innerHTML).toContain(COLOR); + }); + + it('renders nothing for fewer than two points', () => { + const { container } = renderWithMantine( + , + ); + const surface = container.querySelector('.recharts-surface'); + expect(surface).not.toBeInTheDocument(); + }); + + it('renders with an explicit numeric height (table-cell usage)', () => { + const { container } = renderWithMantine( + , + ); + expect(container.querySelector('.recharts-line')).toBeInTheDocument(); + }); +}); From f6449bd7cb340001fbc238e7bbe4a03bb60c449a Mon Sep 17 00:00:00 2001 From: Alex Fedotyev <61838744+alex-fedotyev@users.noreply.github.com> Date: Thu, 25 Jun 2026 02:16:05 +0000 Subject: [PATCH 2/2] test(dashboards): assert Sparkline forwards its height prop The height test only checked that the chart still rendered; capture the height passed to ResponsiveContainer and assert it (the 100% default and an explicit numeric height), so the prop's contract is actually covered. Co-Authored-By: Claude Opus 4.8 --- .../components/__tests__/Sparkline.test.tsx | 25 ++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/packages/app/src/components/__tests__/Sparkline.test.tsx b/packages/app/src/components/__tests__/Sparkline.test.tsx index 37abaa0ff1..70ed4c355f 100644 --- a/packages/app/src/components/__tests__/Sparkline.test.tsx +++ b/packages/app/src/components/__tests__/Sparkline.test.tsx @@ -5,7 +5,11 @@ import { Sparkline, type SparklinePoint } from '@/components/Sparkline'; // recharts `ResponsiveContainer` sizes itself from its parent via a // ResizeObserver, which is a no-op in jsdom (see setupTests), so it never // reports a size and the chart never paints. Swap it for a fixed-size -// pass-through so the SVG renders and the variant can be asserted. +// pass-through so the SVG renders and the variant can be asserted, and record +// the requested height so its forwarding can be asserted. The `mock` prefix +// lets the hoisted jest.mock factory reference it. +const mockResponsiveContainerHeights: Array = []; + jest.mock('recharts', () => { const actual = jest.requireActual('recharts'); const { cloneElement } = jest.requireActual('react'); @@ -13,9 +17,14 @@ jest.mock('recharts', () => { ...actual, ResponsiveContainer: ({ children, + height, }: { children: React.ReactElement<{ width?: number; height?: number }>; - }) => cloneElement(children, { width: 300, height: 80 }), + height?: number | string; + }) => { + mockResponsiveContainerHeights.push(height); + return cloneElement(children, { width: 300, height: 80 }); + }, }; }); @@ -28,6 +37,10 @@ const POINTS: SparklinePoint[] = [ ]; describe('Sparkline', () => { + beforeEach(() => { + mockResponsiveContainerHeights.length = 0; + }); + it('renders a line trend in the given color', () => { const { container } = renderWithMantine( , @@ -60,10 +73,16 @@ describe('Sparkline', () => { expect(surface).not.toBeInTheDocument(); }); - it('renders with an explicit numeric height (table-cell usage)', () => { + it('fills its parent height by default', () => { + renderWithMantine(); + expect(mockResponsiveContainerHeights).toContain('100%'); + }); + + it('forwards an explicit numeric height (table-cell usage)', () => { const { container } = renderWithMantine( , ); expect(container.querySelector('.recharts-line')).toBeInTheDocument(); + expect(mockResponsiveContainerHeights).toContain(24); }); });