refactor(dashboards): extract shared Sparkline primitive from number tile#2520
Conversation
…tile Lift the chrome-less recharts core out of NumberTileBackgroundChart into a reusable <Sparkline> (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 <noreply@anthropic.com>
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
the
review/tier-2
🔵 Tier 2 — Low RiskSmall, isolated change with no API route or data model modifications. Why this tier:
Review process: AI review + quick human skim (target: 5–15 min). Reviewer validates AI assessment and checks for domain-specific concerns. Stats
|
Greptile SummaryThis PR extracts the recharts render core from
Confidence Score: 5/5Pure refactor with no user-visible change; the recharts render is a verbatim lift, the data path is untouched, and the error boundary and aria-hidden wrapper are preserved. The same recharts props, constants, and guards move from one file to another. Tests cover all three chart variants and both height-forwarding scenarios. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
NTBChart["NumberTileBackgroundChart"]
NTBChartInner["NumberTileBackgroundChartInner"]
Sparkline["Sparkline"]
Line["LineChart"]
Area["AreaChart"]
Bar["BarChart"]
NTBChart --> NTBChartInner
NTBChartInner --> Sparkline
Sparkline --> Line
Sparkline --> Area
Sparkline --> Bar
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
NTBChart["NumberTileBackgroundChart"]
NTBChartInner["NumberTileBackgroundChartInner"]
Sparkline["Sparkline"]
Line["LineChart"]
Area["AreaChart"]
Bar["BarChart"]
NTBChart --> NTBChartInner
NTBChartInner --> Sparkline
Sparkline --> Line
Sparkline --> Area
Sparkline --> Bar
Reviews (3): Last reviewed commit: "Merge branch 'main' into alex/HDX-1360-s..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
E2E Test Results✅ All tests passed • 228 passed • 3 skipped • 1506s
Tests ran across 4 shards in parallel. |
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 <noreply@anthropic.com>
Deep Review✅ No critical issues found. This is a behavior-preserving render extraction. The line/area recharts props ( 🔵 P3 nitpicks (6)
Reviewers (7): correctness, testing, maintainability, project-standards, kieran-typescript, agent-native, learnings-researcher. Testing gaps:
|
2 participants
Builds on the number-tile background sparkline (#2489, #2501) and sets up the table-cell trend follow-up tracked in HDX-4604.
Summary
Extracts the chrome-less recharts core out of
NumberTileBackgroundChartinto a reusable<Sparkline>component (line / area / bar) and renders the number tile's background trend through it. This is a pure consolidation: the number tile issues the same query and draws the same faint line / area behind the value, so there is no user-visible change.The table-cell trend feature (HDX-4604) needs the identical render shell in every numeric cell. A shared primitive is the alternative to standing up a second chart component next to this one, which is also the reuse question Mike raised on #2489 ("any reason we couldn't reuse/extend the usual time chart instead?").
What
packages/app/src/components/Sparkline.tsx:<Sparkline points type={'line' | 'area' | 'bar'} color height />. Chrome-less (no axes, grid, legend, or tooltip; dots and animation off), sized to fill its parent viaResponsiveContainer(heightdefaults to100%). Renders nothing for fewer than two points. The line and area branches are lifted fromNumberTileBackgroundChartunchanged; the bar branch follows the existingPatternTrendChartinDBRowTableand is here for the table-cell consumer that lands next.NumberTileBackgroundChart.tsxnow renders<Sparkline points={points} type={backgroundChart.type} color={color} />inside its unchangedaria-hiddenwrapper. Its data path (buildSparklineTimeConfig,convertToTimeChartConfig,useQueriedChartConfig,formatResponseForTimeChart) and the single-seriessparklinePointsFromGraphResultshelper are unchanged.Why the helper stays put
sparklinePointsFromGraphResultsis single-series and specific to the number tile's data path. The table render reconstructs per-group trends differently, so the helper would not be shared; it stays with the number tile. Only the genuinely shared pieces (the component and theSparklinePointtype) live inSparkline.tsx. This keeps the existingNumberTileBackgroundCharttests in place and avoids a dead re-export.No behavior change
The render is a verbatim lift, so the recharts output is identical. The diff on
NumberTileBackgroundChart.tsxshows the same<Area>/<Line>props (stroke,strokeOpacity0.5,strokeWidth2,fillOpacity0.15,dataKey, margin) moving into<Sparkline>, wrapped in the sameResponsiveContainerinside the same wrapper. The number tile passes the same resolved color and type it computed before, and keeps its own "fewer than two points" guard so the background layer is still absent when there is no trend.Test plan
nx run @hyperdx/app:ci:lint(eslint + tsc) cleannx run @hyperdx/app:ci:unit: 2091 pass, 0 fail. Includes the existing number-tile render-wiring test and theDBNumberChartconsumer test, both unchanged and green.Sparkline.test.tsx: mounts the component and asserts the line / area / bar recharts layers render in the given color, and that it renders nothing below two points.Visual note: this is a verbatim render extraction with no visual delta, so the proof of "renders identically" is the render-block diff plus the unchanged consumer test rather than a before/after screenshot. [ui-check: allow]
What's not in this PR (follow-up, HDX-4604)
DBTableChart, which consumes<Sparkline>(lands after feat(dashboards): per-column color on table tiles #2517 so the per-column color drives the sparkline color).No changeset: internal refactor with no user-facing behavior change. [no-changeset: allow]