Skip to content

feat(ui): add Scorecard KPI card component - #142

Closed
goodbounties-nanoclaw-agent[bot] wants to merge 3 commits into
mainfrom
feat/analytics-component-scorecard-plan
Closed

feat(ui): add Scorecard KPI card component#142
goodbounties-nanoclaw-agent[bot] wants to merge 3 commits into
mainfrom
feat/analytics-component-scorecard-plan

Conversation

@goodbounties-nanoclaw-agent

@goodbounties-nanoclaw-agent goodbounties-nanoclaw-agent Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Implements all 5 analytics components planned across #139/#141 (Scorecard) and
#143#147 (Pie/Donut, Bar, Line/Area, DataTable): themeable, cross-platform
chart/table primitives in packages/ui, each following the same conventions
(createComponent, theme-token color resolution, golden-ratio type scale,
bare/card variants, Storybook story + Playwright smoke test + baseline
screenshot).

Shared utility extractions

  • resolveThemeColor (packages/ui/src/utils/resolveThemeColor.ts) —
    theme-token-to-raw-color resolution, factored out once PieDonutChart became
    the 2nd consumer after Scorecard, rather than duplicating a 5th time.
    Scorecard itself is untouched (out of scope, already shipped).
  • CHART_FONT_FAMILY (packages/ui/src/utils/chartFontFamily.ts) — shared
    font-family constant applied to every SVG <Text> in BarChart and
    LineAreaChart, matching the default preset's sans-serif stack instead of the
    browser's serif fallback (react-native-svg's Text sits outside Tamagui's
    styling system, the same reason resolveThemeColor exists for fill/stroke).

PieDonutChart fixes (post-implementation QA)

  • Center-value truncation: centerContentMaxWidth was derived from
    ringRadius * (1 - innerRadius), which shrinks as the donut hole grows
    instead of growing with it — ringRadius is the stroke's centerline, not
    the hole radius. Fixed to compute the actual hole radius
    (ringRadius - strokeWidth / 2) and size the available square off that.
    Also defaulted the center value's own formatting to 0 decimals ("450K", not
    "450.0K") so typical aggregated totals fit the hole at the component's
    default size.
  • innerRadius default: the Default story was overriding the spec
    default to a flat pie; restored to the spec default (innerRadius=0.6,
    donut with center metric) and moved the flat-pie case to its own PurePie
    story.

formatMetricValue fix

Whole-number compact values now render without decimals at every scale
("892K", not "892.0K"), mirroring the pre-existing below-K-threshold integer
check that already did this for raw values under 1,000.

Test plan

  • pnpm --filter @goodwidget/ui build and lint — no new errors
  • Full monorepo pnpm build succeeds
  • Storybook stories for all 5 components render correctly across variants
    and edge-case states (empty, single-point, near-equal split, stress/
    large-N, null values), visually compared against design references
  • Playwright smoke tests pass for all 5 components; full-page baseline
    screenshots committed (tests/design-system/test-results/story-*.png)
  • Unit tests for formatMetricValue / snapshot tests — same open
    question flagged on the Scorecard PR: no JS unit-test framework exists
    in this repo yet; Playwright/Storybook smoke coverage is what this PR
    relies on instead.

Closes #143, #144, #145, #146
Refs #139, #147

Adds a reusable Scorecard component to packages/ui — a single-metric KPI
card with label, formatted value, optional trend indicator, and bare/card
variants. Implements #139's compact/decimal/none formatting modes via a
shared formatMetricValue utility, and a golden-ratio modular typography
scale (base 24px, ratio 1.618, 12px floor) across sm/md/lg sizes.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot could not run the full agentic suite for this review because it was automatically requested on a bot-authored pull request. Request a review from Copilot under Reviewers to retry with the full agentic suite. Improved support for bot-authored pull requests is coming soon.

Adds a new Scorecard analytics primitive to @goodwidget/ui, including a shared metric formatter and Storybook + smoke test coverage.

Changes:

  • Introduces Scorecard component with optional trend indicator and variants (bare/card)
  • Adds formatMetricValue utility and exports both via the UI package index
  • Adds Storybook stories and a Playwright smoke test for the new component

Reviewed changes

Copilot reviewed 6 out of 8 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
tests/design-system/smoke.spec.ts Adds smoke test for the Scorecard Storybook story
packages/ui/src/utils/formatMetricValue.ts New shared formatter for compact/decimal/none metric display
packages/ui/src/index.ts Exposes Scorecard + formatter from the package entrypoint
packages/ui/src/components/Scorecard.tsx New Scorecard UI primitive and trend glyph rendering
packages/ui/package.json Adds react-native-svg dependency required by Scorecard
examples/storybook/src/stories/design-system/Scorecard.stories.tsx Adds Scorecard stories used by smoke test
Files not reviewed (1)
  • pnpm-lock.yaml: Generated file

Comment on lines +22 to +31
function formatCompact(value: number, decimals: number): string {
const absValue = Math.abs(value)
const match = COMPACT_THRESHOLDS.find(({ threshold }) => absValue >= threshold)

if (!match) {
return value.toFixed(decimals)
}

return `${(value / match.threshold).toFixed(decimals)}${match.suffix}`
}
Comment on lines +33 to +39
function formatDecimal(value: number, decimals: number): string {
return new Intl.NumberFormat('en-US', {
minimumFractionDigits: decimals,
maximumFractionDigits: decimals,
useGrouping: true,
}).format(value)
}
Comment on lines +201 to +203
return (
<ScorecardFrame data-testid={testID}>
<ScorecardLabelText size={size}>{label}</ScorecardLabelText>
Comment on lines +155 to +156
/** Up/down/neutral arrow glyph, drawn with react-native-svg for cross-platform rendering. */
function TrendGlyph({ direction, color, size }: { direction: ScorecardTrend['direction']; color: string; size: number }) {
Comment on lines +166 to +168
<Svg width={size} height={size} viewBox="0 0 12 12" accessibilityRole="image">
<Path d={path} stroke={color} strokeWidth={1.5} strokeLinecap="round" />
</Svg>
Comment on lines +173 to +175
<Svg width={size} height={size} viewBox="0 0 12 12" accessibilityRole="image">
<Path d={path} fill={color} />
</Svg>
Comment thread packages/ui/package.json Outdated
"@tamagui/core": "1.121.0",
"@tamagui/lucide-icons": "1.121.0",
"@tamagui/themes": "1.121.0",
"react-native-svg": "15.15.5",
Comment thread tests/design-system/smoke.spec.ts Outdated
Comment on lines +114 to +120
test('Scorecard/Default story renders all mock-data rows', async ({ page }) => {
await gotoStory(page, 'design-system-primitives-scorecard--default')
const frame = getStoryFrame(page)
await expect(frame.getByTestId('Scorecard-default')).toBeVisible()
await expect(frame.getByTestId('Scorecard-bare')).toBeVisible()
await screenshotStory(page, 'tests/design-system/test-results/story-scorecard-default.png')
})
- formatMetricValue: fix a rounding-boundary bug where a scaled value could
  round up to the next unit's threshold without promoting (e.g. 999_950 ->
  "1000.0K" instead of "1.0M"); guard non-finite input (NaN/Infinity) with a
  "--" fallback shared by all consumers; note the intentional en-US locale.
- Scorecard: set both testID and data-testid so React Native test tooling
  and web DOM queries both work; mark the trend arrow SVG as decorative
  (accessible=false / aria-hidden) since adjacent text already conveys
  direction; give resolveThemeColor a visible fallback and dev warning
  instead of silently rendering an empty fill; type its theme param via
  ReturnType<typeof useTheme> instead of an unsafe double cast.
- package.json: move react-native-svg to peerDependencies (kept in
  devDependencies for local build/test) to avoid native-module duplication
  for consumers.
- Storybook story + smoke test: render all 5 mock-data rows in both the
  bare and card variants with per-row testIDs, and assert all 10 are
  visible so the test matches what it claims to cover.

Co-Authored-By: Claude <noreply@anthropic.com>
…ing pass

- formatMetricValue: whole numbers below the compact threshold render
  without decimals ("47", not "47.0").
- Scorecard card variant: vertically centers content regardless of
  whether a trend row is present.
- Value text uses $color instead of $primary; prefix/suffix render at
  fontWeight 400 in $placeholderColor, subordinate to the value.
- Card variant elevation now uses a lightness-overlay + top highlight
  instead of a hard border, scoped to Scorecard's <Card> call site so
  the shared Card primitive is untouched.
- Spacing (label-to-value gap, value-to-trend gap, card padding) now
  derives from the same golden-ratio constants as the type scale.
- Bump the Scorecard smoke test's viewport height to fit the taller
  card layout without clipping the screenshot.

Co-Authored-By: Claude <noreply@anthropic.com>
@goodbounties-nanoclaw-agent

Copy link
Copy Markdown
Contributor Author

Closing this PR — its head branch was deleted during an earlier rename and GitHub doesn't re-link a PR to a recreated branch of the same name, so this PR's diff has been frozen at the pre-chart-work Scorecard-only state despite the branch itself moving forward. Re-opened as #148 from the actual current branch (feat/analytics-components), carrying the same description (all 5 components: Scorecard, Pie/Donut, Bar, Line/Area, DataTable).

Continuing review there: #148

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.

[Feature]: Add Pie/Donut chart component

1 participant