refactor: consolidate remaining charts into LineChart + PieChart wrappers#7328
Draft
talissoncosta wants to merge 11 commits intomainfrom
Draft
refactor: consolidate remaining charts into LineChart + PieChart wrappers#7328talissoncosta wants to merge 11 commits intomainfrom
talissoncosta wants to merge 11 commits intomainfrom
Conversation
Adds two new chart wrappers to the charts umbrella, alongside the existing BarChart: - LineChart — mirrors BarChart's API (data, series, colorMap, xAxisInterval, showLegend, seriesLabels). Monotone lines, no dots, token-based axis and grid colours via colorTextSecondary. - PieChart — fixed-size, optional donut via innerRadius, colorMap keyed by slice name, optional legend, optional tooltip. Barrel export in index.ts so consumers can import everything from `components/charts`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mirrors the BarChart.stories.tsx pattern — deterministic pseudo-random for Chromatic stability, pinned reference date, buildChartColorMap for colour assignment. Covers variants users are likely to reach for: - LineChart: UsageTrends (3 metrics), SingleLine, ManyLines (palette stress test). - PieChart: Donut, SolidPie, TwoSlices, ManySlices, WithLegend. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces ~50 lines of inline recharts JSX with the shared <LineChart> component. Loses the three hardcoded stroke colours (#0AADDF / #27AB95 / #FF9F43) in favour of buildChartColorMap + token-based axis colours, so the chart now renders correctly in dark mode. Behaviour changes: - Colour assignment moves from hand-picked hex to the shared chart palette - Legend now uses seriesLabels for display names (data keys stay snake_case for API compatibility) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…d PieChart Replaces the inline recharts PieChart/Pie/Cell/Tooltip stack with the shared <PieChart> component. Stage + Released colours are preserved by passing them via colorMap (the hand-picked palette is semantic — a stage is coded by position — so it stays). Net drop: recharts Cell/Pie/PieChart/Tooltip imports; pieData now a typed PieSlice[]. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ExperimentResultsTab is legacy v1 code with per-cell bar colouring and a `variant` x-axis, which doesn't match the shared BarChart API cleanly. Rather than expand the shared API to accommodate one legacy caller, just replace the hardcoded #EFF1F4 / #656D7B with colorTextSecondary so dark mode works here too. CartesianGrid switches to the same strokeOpacity-only pattern the shared BarChart uses, dropping the explicit light-mode stroke colour. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code review is billed via overage credits. To resume reviews, an organization admin can raise the monthly limit at claude.ai/admin-settings/claude-code.
Once credits are available, reopen this pull request to trigger a review.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
4 tasks
github-actions
Bot
added
front-end
Contributor
Docker builds report
|
Contributor
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
|
Contributor
Visual Regression16 screenshots compared. See report for details. |
Follow-up to the PieChart migration. The stage palette (six hand-picked hex values) was passed through to <PieChart> as a colorMap — bypassing the shared chart tokens. Swap to buildChartColorMap, which pulls from the CHART_COLOURS token palette and resolves theme-appropriate colours. Released (previously #27AB95) now uses colorTextSuccess — the exact same hex value, but themed via the token system, which means it also adapts via the CSS cascade. Affects the donut slice, the legend swatch, and the Released-row label text. Out of scope (tracked separately for future design-system cleanup): non-chart hex in the same file — row backgrounds, "Published" pill alpha, progress-bar semantic colours. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace inline style={{...}} blocks for the legend swatches with the
shared <ColorSwatch> component (md = 12px, close to the former inline
10px). Parent rows pick up a `gap-2` to replace the swatch's former
marginRight, matching the ChartTooltip pattern.
Replace `style={{ color: RELEASED_COLOUR }}` on the Released label with
the `.text-success` utility class from `_token-utilities.scss`.
RELEASED_COLOUR stays as an imported constant because the <PieChart>
colorMap still needs a raw value (not a className).
Non-chart inline styles in the same file (row backgrounds, "Published"
pill, progress bars, layout sizing) stay for a separate design-system
pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Swap every inline fontSize for the matching auto-generated utility class: - fontSize: 13 → .fs-caption - fontSize: 12 → .fs-captionSmall - fontSize: 11 → .fs-captionXSmall Also replaces two inline `color: '#27AB95'` usages (project "X released" and org "X released" counts) with the .text-success token utility. Out of scope (still inline): the Published/Draft pill (fontSize: 10 has no matching utility class, plus the conditional `#27AB9518` / `#9DA4AE18` alpha backgrounds), the progress bar semantic colours, row backgrounds, and layout sizing (width, flex, paddings, minWidths). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The initial PieChart used <RawPieChart width={size} height={size}>.
Recharts packs Legend *inside* that box, so when showLegend was on the
pie shrank upward and the legend rendered inside the donut bounds
(visible in the ManySlices story).
Switch to the same <ResponsiveContainer> pattern BarChart and LineChart
use — the container owns the height, legend sits below the chart
naturally. Drop `size` in favour of `height`; width flows from the
parent.
Consumer (ReleasePipelineStatsTable) now wraps the pie in a fixed-width
div to keep the 180×180 footprint. Stories updated to give each variant
enough height for pie + legend (~340px for legend variants).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Previously, <PieChart> used Recharts' default <Tooltip> with a formatter. After _recharts.scss is deleted (#7223), the default tooltip renders with Recharts' built-in styles (white background, dark text) — stylistically jarring in dark mode where the rest of the chart reads on a dark surface. Switch <PieChart> to use the shared <ChartTooltip> for theme consistency across all three chart components (Bar, Line, Pie). ChartTooltip changes: - Add hideTotal prop (default false). PieChart passes hideTotal because a single-slice hover makes "Total: X" redundant with the slice value. - Conditionally render the label header — pies don't have an x-axis label, so the header is skipped for them. - Read entry.name with fallback to entry.dataKey so pie-slice names resolve correctly without breaking bar/line tooltips (where name === dataKey by default). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reorganise Storybook titles so the three chart components sit together instead of scattering across the Components root: - Components/BarChart → Components/Charts/BarChart - Components/LineChart → Components/Charts/LineChart - Components/PieChart → Components/Charts/PieChart Matches the mental model of "charts as a family" — reviewers looking at one chart can quickly hop to the siblings. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
Closes #6889. Adds
<LineChart>and<PieChart>alongside the existing<BarChart>, migrates the last three raw-recharts consumers, and makes their dark-mode bugs go away in the process.New shared components
<LineChart><BarChart>:data,series,colorMap,xAxisInterval,showLegend,seriesLabels. Monotone lines, no dots. Token-based axis / grid colours viacolorTextSecondary.<PieChart>data({name, value}[]),colorMapkeyed by slice name,height,innerRadius(set> 0for donut),outerRadius,paddingAngle,showLegend,showTooltip. Width flows from the parent (uses<ResponsiveContainer>— same pattern as<BarChart>/<LineChart>).components/charts/index.tsimport { BarChart, LineChart, PieChart } from 'components/charts'.Shared tooltip
All three chart components now render the same
<ChartTooltip>.<PieChart>passeshideTotalbecause a single-slice hover makes "Total: X" equal to the slice's own value — redundant. Bar / line charts keep the Total row.Dropped
<PieChart>'s original default-recharts tooltip path (white box, dark text, not theme-aware) — with_recharts.scssgoing away via #7223, the default tooltip is no longer themed.Storybook stories
Added
LineChart.stories.tsxandPieChart.stories.tsxfollowing the pattern fromBarChart.stories.tsx— deterministic pseudo-random fake data, pinned reference date (Chromatic-safe),buildChartColorMapfor colour assignment.All three chart stories now live under a Components/Charts/ umbrella in the Storybook sidebar (previously scattered under
Components/).UsageTrends(3 metrics),SingleLine,ManyLines(7-line palette stress test).Donut,SolidPie,TwoSlices,ManySlices,WithLegend.Consumer migrations
UsageTrendsChart.tsx(admin dashboard)<LineChart>. Drops 3 hardcoded stroke colours (#0AADDF/#27AB95/#FF9F43) in favour of the shared chart palette.ReleasePipelineStatsTable.tsx(admin dashboard)<PieChart>plus a design-system pass: stage palette now usesbuildChartColorMap,ReleasedusescolorTextSuccess, legend swatches are<ColorSwatch>components, inlinefontSizereplaced with.fs-caption/.fs-captionSmall/.fs-captionXSmall,color: '#27AB95'replaced with.text-successutility.ExperimentResultsTab.tsx(legacy v1 experiments)variantas x-axis and per-cell bar colouring, which doesn't match the shared<BarChart>API. Rather than expand the shared API for one legacy caller, replaces the hardcoded#EFF1F4/#656D7BwithcolorTextSecondary. Still fixes the dark-mode concern.Related PRs
FeatureAnalytics.tsx) that introduced<BarChart>,ChartTooltip,buildChartColorMap, and the token foundation.OrganisationUsage+SingleSDKLabelsChartto<BarChart>. This PR is net-additive to refactor: consolidate charts with BarChart component #7223; no overlap in files. Either PR can merge first.After #7223 and this PR merge,
rechartshas zero raw consumers outside of thecomponents/charts/wrappers and the legacyExperimentResultsTab.tsx(retained until Experiments v2 replaces it).Out of scope
--color-chart-Noverrides (400/300/200shades in_tokens.scss). Not a per-chart bug — fixing is a single token-level edit that cascades to every chart consumer. Parked; low-severity (graphical, dark-mode only, admin surfaces).ReleasePipelineStatsTable.tsx— Published/Draft pill, progress-bar semantic colours, row backgrounds, layout widths/padding. Real design-system cleanup but not chart-related.<ChartContainer>,<ChartAxes>). Internal duplication across the three wrappers is small; revisit if it becomes painful.<AreaChart>/<ScatterChart>/<RadialBarChart>— no consumers today.ExperimentResultsTab.tsx— legacy v1 code, will be deleted when Experiments v2 ships.How did you test this code?
<ColorSwatch>npm run storybook→ browsed all LineChart and PieChart variants underComponents/Charts/. Chromatic-safe (deterministic data + pinned date).npm run typecheck— no new errors in any of the touched files.npx eslint --fixon all modified / added files — clean.