Skip to content

Add BarChart and AreaChart components to fidesui#7699

Merged
kruulik merged 25 commits intomainfrom
access-control-charts
Mar 24, 2026
Merged

Add BarChart and AreaChart components to fidesui#7699
kruulik merged 25 commits intomainfrom
access-control-charts

Conversation

@kruulik
Copy link
Copy Markdown
Contributor

@kruulik kruulik commented Mar 19, 2026

Ticket: N/A — part of PBAC work

Description Of Changes

Add generic, reusable chart components and time-series utilities to fidesui in preparation for the access control dashboard. These components are domain-agnostic and can be used by any feature requiring bar charts, area charts, or time-series visualizations.

New components:

  • BarChart — responsive bar chart with Ant Design theme integration, configurable bar size (sm/md/lg), and smart onIntervalChange callback for responsive data bucketing
  • AreaChart — multi-series area chart with gradient fills, grid lines, and the same responsive interval API as BarChart
  • XAxisTick — custom X-axis tick renderer with smart timestamp formatting based on data interval

Utilities added to chart-utils.ts:

  • formatTimestamp(timestamp, intervalMs, verbose?) — formats ISO timestamps for tick labels or tooltips using date-fns, with invalid-date fallback
  • deriveInterval(data) — auto-detects interval between consecutive data points
  • pickIntervalHours(rangeMs, containerWidth, minPointWidth) — computes ideal bucket size (in hours) that fits the data range into the container
  • calcTickInterval(pointCount, containerWidth, labelWidth) — calculates tick skip count to prevent label overlap
  • useChartAnimation(duration) — disables recharts animation after initial render to prevent re-triggering on interaction
  • useContainerSize(ref) — returns { width, height } from a ResizeObserver (refactored from existing hook)

Other changes:

  • DonutChart updated to use the refactored useContainerSize return shape
  • ChartText minor cleanup
  • New chart-constants.ts with BarSize type, BAR_SIZE_TOKEN mapping, LABEL_WIDTH, MIN_PX_PER_POINT
  • story-utils.ts with a seeded PRNG for deterministic Storybook data

Code Changes

  • clients/fidesui/src/components/charts/AreaChart.tsx — New reusable area chart component
  • clients/fidesui/src/components/charts/AreaChart.stories.tsx — Storybook stories (single and multi-series)
  • clients/fidesui/src/components/charts/BarChart.tsx — New reusable bar chart component
  • clients/fidesui/src/components/charts/BarChart.stories.tsx — Storybook stories with resizable container demo
  • clients/fidesui/src/components/charts/XAxisTick.tsx — Custom X-axis tick renderer
  • clients/fidesui/src/components/charts/chart-utils.ts — Time-series utilities and hooks
  • clients/fidesui/src/components/charts/chart-constants.ts — Shared constants and types
  • clients/fidesui/src/components/charts/story-utils.ts — Seeded PRNG for stories
  • clients/fidesui/src/index.ts — Export new components and utilities

Steps to Confirm

  1. Run cd clients/fidesui && npm run storybook
  2. Navigate to Charts > BarChart — verify the resizable container story renders and responds to resize
  3. Navigate to Charts > AreaChart — verify single-series and multi-series stories render with gradient fills
  4. Verify bar/area animations play once and do not re-trigger on hover
  5. Hover over data points to confirm tooltip styling matches Ant Design theme
  6. Resize the storybook viewport to confirm tick labels adjust density automatically

Pre-Merge Checklist

  • Issue requirements met
  • All CI pipelines succeeded
  • CHANGELOG.md updated
    • Add a db-migration This indicates that a change includes a database migration label to the entry if your change includes a DB migration
    • Add a high-risk This issue suggests changes that have a high-probability of breaking existing code label to the entry if your change includes a high-risk change (i.e. potential for performance impact or unexpected regression) that should be flagged
    • Updates unreleased work already in Changelog, no new entry necessary
  • UX feedback:
    • All UX related changes have been reviewed by a designer
    • No UX review needed
  • Followup issues:
    • Followup issues created
    • No followup issues
  • Database migrations:
    • No migrations
  • Documentation:
    • No documentation updates required

kruulik and others added 7 commits March 18, 2026 17:09
Adds BarChart component, XAxisTick renderer, and time-series utility
functions (pickInterval, formatTimestamp, deriveInterval, tooltipLabelFormatter)
to support upcoming access control dashboard visualizations.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adds the summary tab for the access control feature (behind feature flag):
- Time-series violations chart, violation rate donut, data consumers table
- Paginated findings table with policy violation aggregates
- RTK Query endpoints for summary data
- Mock data and MSW handlers for local development

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Guard formatTimestamp/tooltipLabelFormatter against invalid dates
- Add defensive defaults to XAxisTick props and document deriveInterval contract
- Use useChartAnimation in BarChart (consistent with Sparkline, DonutChart, etc.)
- Memoize useTooltipContentStyle to avoid unnecessary re-renders
- Fix lint: merge duplicate react imports, sort imports

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown
Contributor

vercel bot commented Mar 19, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
fides-plus-nightly Ready Ready Preview, Comment Mar 24, 2026 6:02pm
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
fides-privacy-center Ignored Ignored Mar 24, 2026 6:02pm

Request Review

kruulik and others added 2 commits March 19, 2026 00:26
Categorical labels were rendered by Recharts' default tick (browser
sans-serif) while time-series labels went through ChartText (Bazier Mono).
Now both paths use ChartText so the font is always consistent.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@greptile-apps

This comment was marked as outdated.

kruulik and others added 4 commits March 19, 2026 09:34
- Fix tickFormatter being silently ignored when intervalMs is set by
  unifying both tick paths into a single ChartText-based renderer
- Rename i → index in story generator
- Replace div with Flex in story decorator

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@kruulik kruulik changed the title Add reusable BarChart component and chart utilities to fidesui Add reusable BarChart and LineChart Mar 23, 2026
@kruulik kruulik marked this pull request as ready for review March 23, 2026 21:40
@kruulik kruulik requested a review from a team as a code owner March 23, 2026 21:40
@kruulik kruulik requested review from gilluminate and lucanovera and removed request for a team, gilluminate and lucanovera March 23, 2026 21:40
claude[bot]

This comment was marked as resolved.

@kruulik kruulik marked this pull request as draft March 23, 2026 21:59
@kruulik kruulik changed the title Add reusable BarChart and LineChart Add reusable BarChart and AreaChart components to fidesui Mar 24, 2026
Copy link
Copy Markdown

@claude claude bot left a comment

Choose a reason for hiding this comment

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

Code Review — BarChart + chart utilities

Overall this is clean, well-structured work. The component design is consistent with the existing chart patterns (animation hook, container-size observer, Ant Design token integration), and the new utility functions are focused and testable. A few things worth addressing before merge:

Suggestions

  • formatTimestamp uses inconsistent 12h/24h formats — axis labels use 24h (HH:mm) but tooltip (verbose) uses 12h (h:mm a) for the same intervals. Pick one convention.
  • onIntervalChange in effect deps — if callers pass an inline function, the effect re-fires on every parent render. Callers should wrap it in useCallback, or the hook should internalize it via a ref. Same pattern in both BarChart and AreaChart.
  • pickIntervalHours 72h cap is unexplainedMath.min(idealHours, 72) should either be a named constant or have a comment explaining why 72h is the ceiling.
  • size prop in BarChart doesn't affect rendered bar widthbarWidth is used as a density hint for pickIntervalHours but never passed to <Bar barSize>. Clarify in a comment if this is intentional.
  • LABEL_WIDTH change (90 → 110) is a silent behavior change for all existing consumers of ChartText (e.g. DonutChart center label). Worth calling out explicitly.
  • BarChart.stories.tsx only exports Default but the PR description references 6 stories. Either add the missing stories or update the description.

Nice to have

  • The exported public surface in index.ts includes several internal helpers (calcTickInterval, deriveInterval, pickIntervalHours, useContainerSize). Worth reviewing whether these need to be public API.
  • No unit tests for the new pure utility functions. formatTimestamp, deriveInterval, calcTickInterval, and pickIntervalHours each have meaningful edge cases (invalid dates, empty data, zero container width) that are straightforward to cover with @jest/globals or Vitest.

- Stabilize onIntervalChange via ref in BarChart and AreaChart
- Add barSize prop to Bar element for consistent visual width
- Restore original width in ChartText to avoid visual regression
- Use consistent 24h time format in formatTimestamp
- Extract MAX_INTERVAL_HOURS constant
- Remove internal helpers from public API exports

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@kruulik kruulik changed the title Add reusable BarChart and AreaChart components to fidesui Add BarChart and AreaChart components to fidesui Mar 24, 2026
Copy link
Copy Markdown
Contributor

@gilluminate gilluminate left a comment

Choose a reason for hiding this comment

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

One tiny nit-pick but looks good!

return gap > 0 ? gap : HOUR_MS;
};

const MAX_INTERVAL_HOURS = 72;
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.

probably belongs in chart-constants.ts

@kruulik kruulik enabled auto-merge March 24, 2026 17:57
@kruulik kruulik added this pull request to the merge queue Mar 24, 2026
Merged via the queue into main with commit 175c558 Mar 24, 2026
47 of 49 checks passed
@kruulik kruulik deleted the access-control-charts branch March 24, 2026 18:17
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.

2 participants