feat: SW-2037 Wire CVD chart palette tokens into chart components#128
Conversation
…ents Chart components previously defaulted to legacy graph colors (or required explicit colors), so the CVD-friendly palette introduced in SW-1795 never reached rendered charts. - Resolve chart tokens through a 1x1 canvas pixel readback in colors.ts: tokens are declared in oklch, which Plotly's color parser (tinycolor2) cannot parse — without normalization Plotly silently fell back to its d3 defaults - Add toPlotlyColorscale() to convert CHART_SEQUENTIAL / CHART_DIVERGING ramps into Plotly colorscale stops - Default PieChart, DotPlot, Histogram series cycles to CHART_COLORS - Make series color optional on BarGraph, LineGraph, AreaGraph, ScatterGraph, Boxplot with CHART_COLORS auto-assignment by index - Map Chromatogram A/T/G/C trace defaults onto palette hues - Use CHART_COLORS + CHART_DIVERGING.blueOrange for InteractiveScatter and PlateMap defaults (named Plotly colorscales left intact) - Strip hard-coded legacy colors from chart stories so Storybook renders the default palette; explicit colors kept only in custom-color demos - Write vitest browser failure screenshots to test-results/ instead of src/ — __screenshots__/*.stories.tsx directories matched Storybook's stories glob and broke story indexing (EISDIR) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR completes the wiring of the CVD-friendly chart palette tokens into the rendered Plotly chart components by switching default series/slice colors to CHART_COLORS and adding a Plotly-friendly colorscale utility. It also fixes Vitest browser screenshot output to avoid generating __screenshots__ directories under src/ that can break Storybook indexing.
Changes:
- Normalize CSS token colors (including
oklch(...)) into Plotly-parseable hex/rgba, and addtoPlotlyColorscale()for Plotly colorscale stops. - Update chart components to default/auto-assign
CHART_COLORSwhen explicit series colors aren’t provided; remove legacy hard-coded palette usage from stories. - Configure Vitest browser screenshots to write under
test-results/viascreenshotDirectory.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| vite.config.ts | Routes Vitest browser failure screenshots to test-results/screenshots to avoid Storybook glob collisions under src/. |
| src/utils/colors.ts | Adds Plotly-safe color normalization for CSS tokens and exports toPlotlyColorscale() for ramp-to-colorscale conversion. |
| src/components/charts/ScatterGraph/ScatterGraph.tsx | Makes series color optional and auto-assigns from CHART_COLORS. |
| src/components/charts/ScatterGraph/ScatterGraph.stories.tsx | Removes legacy hard-coded series colors so defaults showcase token palette. |
| src/components/charts/PlateMap/constants.ts | Switches default categorical and continuous scales to CHART_COLORS and CHART_DIVERGING via toPlotlyColorscale(). |
| src/components/charts/PieChart/PieChart.tsx | Uses CHART_COLORS as the default slice colors. |
| src/components/charts/LineGraph/LineGraph.tsx | Makes series color optional and auto-assigns from CHART_COLORS. |
| src/components/charts/LineGraph/LineGraph.stories.tsx | Removes legacy hard-coded series colors so defaults showcase token palette. |
| src/components/charts/InteractiveScatter/utils.ts | Replaces hard-coded fallback blue with COLORS.primary from constants. |
| src/components/charts/InteractiveScatter/InteractiveScatter.tsx | Replaces hard-coded selection outline color with COLORS.selected. |
| src/components/charts/InteractiveScatter/constants.ts | Switches defaults to CHART_COLORS and diverging ramp via toPlotlyColorscale(). |
| src/components/charts/Histogram/Histogram.tsx | Uses CHART_COLORS as the default series color cycle. |
| src/components/charts/Histogram/Histogram.stories.tsx | Removes legacy hard-coded series colors so defaults showcase token palette. |
| src/components/charts/DotPlot/DotPlot.tsx | Uses CHART_COLORS as the default series color cycle. |
| src/components/charts/DotPlot/DotPlot.stories.tsx | Removes legacy hard-coded series colors so defaults showcase token palette. |
| src/components/charts/Chromatogram/Chromatogram.tsx | Maps A/T/G/C default trace colors to CHART_COLORS. |
| src/components/charts/Boxplot/Boxplot.tsx | Makes series color optional and auto-assigns from CHART_COLORS. |
| src/components/charts/Boxplot/Boxplot.stories.tsx | Removes legacy hard-coded series colors so defaults showcase token palette. |
| src/components/charts/BarGraph/BarGraph.tsx | Makes series color optional and auto-assigns from CHART_COLORS. |
| src/components/charts/BarGraph/BarGraph.stories.tsx | Removes legacy hard-coded series colors so defaults showcase token palette. |
| src/components/charts/AreaGraph/AreaGraph.tsx | Makes series color optional and auto-assigns from CHART_COLORS in stacked and normal modes. |
| src/components/charts/AreaGraph/AreaGraph.stories.tsx | Removes legacy hard-coded series colors so defaults showcase token palette. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…verage - Use a two-sentinel probe in toPlotlySafeColor so valid black tokens are not misclassified as unparseable (Copilot review) - Guard toPlotlyColorscale against 0/1-length ramps (Copilot review) - Add unit tests for mapColors fallback paths and the color utilities to satisfy the new-code coverage gate Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…iles Revert the cosmetic literal swaps in InteractiveScatter.tsx and utils.ts: the coverage gate scores whole changed files at 95% and both sit at ~76% from untested Plotly event-handler code. Token wiring for InteractiveScatter still lands through constants.ts (COLORS, DEFAULT_CATEGORY_COLORS, DEFAULT_COLOR_SCALE), which is fully covered. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
boramyi-ts
left a comment
There was a problem hiding this comment.
Looks great thank you so much!
…okens The COLORS object (black/white/blue/grey scales, graph primary and secondary colors) predates the CVD-friendly chart palette and had no remaining component consumers: - ChromatogramChart annotations now style user-defined annotations via CHROMATOGRAM_ANNOTATION constants (same resolved values) - Custom-color demo stories use explicit hex literals - ColorToken type removed alongside the object - DESIGN.md updated to document the current chart color exports Note: removes a public export while the package is pre-1.0; intentionally not marked as a breaking-change commit so it rides the v0.6.0 minor. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
… white InteractiveScatter was the only chart not using usePlotlyTheme — it hardcoded white paper/plot backgrounds and light-mode axis colors, so it rendered as a white box in dark mode. - getPlotlyLayoutConfig now takes the usePlotlyTheme colors: transparent backgrounds, themed title/axis/tick/grid colors in both modes - Restore token-backed defaults (COLORS.primary fallbacks in mapColors, COLORS.selected outline) and prune COLORS keys obsoleted by theming - Add ContinuousColorMapping and SelectionEvents stories with play tests covering the continuous colorbar branch and the click/box-select/ deselect handler pipeline - Expand utils unit tests (shape/size mapping, downsampling, tooltips, selection modes, axis ranges, layout config) Coverage: InteractiveScatter.tsx 76% -> 99%, utils.ts 76% -> 98% lines. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…id lines - Replace Plotly's hover labels in InteractiveScatter with a theme-styled HTML tooltip (popover tokens, positioned over the hovered point via the axis data->pixel converters); tooltip.native: true restores the built-in labels - Add markerOutline to usePlotlyTheme — dark outline on light mode, light outline on dark mode — so scatter dots read against either background - Raise grid line contrast in both themes (grey-400/55% light, grey-500/55% dark) across all charts - Hoist stable default prop objects: the inline xAxis/yAxis/tooltip defaults created new identities every render, so any internal state change re-ran the plot effect, tearing down the plot (and the tooltip) on every hover - Add ThemedTooltip story with play tests for hover/unhover and the suppressed native hover label Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
f840326 to
4c59d76
Compare
Replace the repeated `series.color ?? CHART_COLORS[index % CHART_COLORS.length]` idiom across the chart components with a shared seriesColor() helper in the color util, addressing PR #128 review feedback (Boxplot R130). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
🎉 This PR is included in version 0.7.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Jira: SW-2036 (epic SW-1673 — React UI Kit v0.6.0)
Summary
Follow-up to #124 (SW-1795): the CVD-friendly chart palette tokens existed in
index.tailwind.css/colors.tsbut never reached rendered charts — components defaulted to legacy graph colors or required explicit colors, and stories hard-coded the old palette on top of that.Token plumbing (
src/utils/colors.ts)CHART_COLORSconsumers were silently getting Plotly's stock d3 palette. Resolved token values are now normalized to hex/rgba via a 1×1 canvas pixel readback (a plainfillStyleround-trip doesn't work; browsers serialize oklch back as oklch).toPlotlyColorscale()converts theCHART_SEQUENTIAL/CHART_DIVERGINGramps into Plotly colorscale stops; exported from the package root.Components
COLORS.*default cycles →CHART_COLORSseries.coloris now optional, auto-assignedCHART_COLORS[index % 12](explicit colors still win — non-breaking)CHART_COLORSandCHART_DIVERGING.blueOrange; named Plotly colorscales (Viridis,Blues, …) intentionally untouchedLegacy palette removal
COLORSexport (black/white/blue/grey scales, graph primary/secondary colors) fromcolors.ts— no remaining consumers after the token wiring. ChromatogramChart annotation styling moved toCHROMATOGRAM_ANNOTATIONconstants with identical resolved values; demo stories use explicit hex literals. Removes a public export while pre-1.0; intentionally not marked breaking so it rides the v0.6.0 minor.Tooling fix
test-results/(screenshotDirectorywas defined but never wired). Previously they created__screenshots__/*.stories.tsxdirectories undersrc/, which match Storybook's stories glob and crash story indexing withEISDIR— wedging dev servers on a stale module build.Screenshots
Type of Change
Checklist
yarn lintpassesyarn buildpassesyarn test:allpasses (all 96 chart Storybook tests green; unrelated pre-existing failures in untracked FormulaEditor WIP excluded)Testing
Covered by existing chart Zephyr-mapped play tests (SW-T966–SW-T984, SW-T1013 and the PlateMap/PieChart/Histogram suites) — all passing.
Verification
Verified locally in Storybook (light + dark): LineGraph auto-assigns chart-1…6 (#2F45B5/#FD972F/#038599/#E15759/#8243BA/#94C2FF), PieChart slices and PlateMap's blue→orange diverging default render the CVD palette.
🤖 Generated with Claude Code