fix: SW-1890 Fix PieChart legend rendering unstyled#145
Open
owilliams-tetrascience wants to merge 2 commits into
Open
fix: SW-1890 Fix PieChart legend rendering unstyled#145owilliams-tetrascience wants to merge 2 commits into
owilliams-tetrascience wants to merge 2 commits into
Conversation
PieChart rendered its legend and wrapper using global, unscoped class names (legend-container, legend-item, color-box, divider, card-container, chart-container, title-container) whose only CSS rules live in Histogram/Histogram.scss — which PieChart never imports. Consumers using PieChart standalone got invisible color swatches and an unstyled, layout-less legend. Migrate PieChart off the orphaned global classes onto Tailwind utilities (the library's standard pattern), so it ships its own styling and no longer depends on global CSS or namespace-prone class names. Add text-foreground for dark-mode legibility and stable data-slot hooks for testability. Also fix the same orphaned chart-container class on LineGraph and ScatterGraph (replaced with relative size-full). Add a play-function test asserting the legend color swatches have non-zero rendered size. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
owilliams-tetrascience
temporarily deployed
to
artifactory-prod
GitHub Actions
Inactive
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes PieChart legend rendering by removing reliance on orphaned, global CSS class names (defined only in Histogram.scss) and replacing them with Tailwind utility classes, ensuring the legend is styled correctly when PieChart is consumed standalone.
Changes:
- Migrated
PieChartwrapper/title/legend markup from global class names to Tailwind utilities and added stabledata-slothooks for testing. - Removed the orphaned
chart-containerclass usage fromLineGraphandScatterGraphwrappers. - Updated
PieChartStorybook play function selectors to usedata-slotand added a rendered-size assertion for legend swatches.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/components/charts/PieChart/PieChart.tsx | Replaces unscoped legend/container classes with Tailwind utilities and adds data-slot hooks to make PieChart styling self-contained. |
| src/components/charts/PieChart/PieChart.stories.tsx | Updates legend selectors to data-slot and adds a play assertion that swatches render with non-zero size. |
| src/components/charts/LineGraph/LineGraph.tsx | Removes orphaned chart-container class usage in the wrapper, switching to Tailwind sizing utilities. |
| src/components/charts/ScatterGraph/ScatterGraph.tsx | Removes orphaned chart-container class usage in the wrapper, switching to Tailwind sizing utilities. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
owilliams-tetrascience
changed the title
Use `${label}-${i}` for the legend item key so repeated labels don't
collide, per review feedback.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
owilliams-tetrascience
temporarily deployed
to
artifactory-prod
GitHub Actions
Inactive
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||
54321jenn-ts
approved these changes
Jun 23, 2026
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
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.
Summary
PieChartrendered its legend and wrapper using global, unscoped class names (legend-container,legend-item,color-box,divider,card-container,chart-container,title-container) whose only CSS rules live inHistogram/Histogram.scss— whichPieChartnever imports. Any consumer usingPieChartwithout also pulling inHistogramgot invisible color swatches and an unstyled, layout-less legend (a wall of plain text).This implements Option B from SW-1890 (the recommended fix): migrate off the orphaned global classes onto Tailwind utilities — the library's standard pattern — so
PieChartships its own styling and no longer depends on global CSS or namespace-prone class names.Changes
PieChart.tsx— legend container/rows/items, color swatches (size-3 rounded-[4px]), divider (bg-border), title and wrapper all use Tailwind classes. Addedtext-foregroundfor dark-mode legibility, and stabledata-slot="pie-legend"/data-slot="pie-legend-swatch"hooks for testing (instead of relying on style-bearing class names).LineGraph.tsx/ScatterGraph.tsx— fixed the same orphanedchart-containerclass flagged in the ticket's audit item (replacedchart-container relative→relative size-full).PieChart.stories.tsx— updated legend selectors to thedata-slothook and added a play-function step asserting the color swatches have non-zero rendered size (acceptance criterion).Audit note
Chromatogram/ChromatogramChartalso referencechart-container, but only on a "No data available" text fallback and a Plotly native legend (no custom HTML swatches) — no broken-swatch bug there, so they were left untouched.Type of Change
Checklist
yarn lintpassesyarn buildpassesyarn test:allpasses (548 unit + 653 storybook)Testing
New swatch-size assertion added to the
PieChart"Default" story play function. Existing PieChart Zephyr cases: SW-T1012–SW-T1016.Verification
Screenshots
Acceptance: importing
PieChartstandalone in a fresh app now produces a legend with visible colored swatches and proper layout in both light and dark mode.🤖 Generated with Claude Code