Skip to content

feat: add single-donut Binary story and story tests for donut-binary - #906

Open
c-lamoureux wants to merge 2 commits into
feat/donut-binary-secondary-color-labelsfrom
fix/donut-binary-story-tests
Open

feat: add single-donut Binary story and story tests for donut-binary#906
c-lamoureux wants to merge 2 commits into
feat/donut-binary-secondary-color-labelsfrom
fix/donut-binary-story-tests

Conversation

@c-lamoureux

Copy link
Copy Markdown
Contributor

Description

Follow-up from reviewing #888:

  • Adds a Binary story alongside the existing two-chart Boolean example: a single <Donut isBoolean> using the normal (default) categorical color scheme, matching the design mock (88.3% "Satisfied", pink categorical-600 primary). Proves the secondary-gray override in feat: s2 donut binary #888 works standalone, without requiring a second donut or explicit color overrides.
  • Adds DonutBinary.test.tsx, following the existing Donut.test.tsx pattern (render the story, assert on rendered marks/text), covering:
    • Boolean: each chart's primary segment keeps its explicit color; secondary segment is forced to secondary-gray in both charts.
    • Binary: primary segment renders via the normal color scale; secondary segment is still forced to secondary-gray; center-hole value and label render correctly.

Notes

  • isBoolean's summary value format is hardcoded to .0% (0 decimals) in donutSummaryUtils.ts, so 0.883 renders as 88%, not 88.3% as in the design mock. The Binary test asserts the actual current behavior (88%). This predates this PR (and feat: s2 donut binary #888) and isn't fixed here — separate from numberFormat, which is already threaded through for the non-boolean summary path but not the boolean one. Worth a follow-up issue.

How Has This Been Tested?

  • yarn test --testPathPattern="DonutBinary" — 6/6 passing
  • Verified visually in Storybook (Donut/Features/Binary and Boolean)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant