Skip to content

Create RadarChart#7505

Merged
kruulik merged 47 commits intomainfrom
karolis-radarchart
Feb 26, 2026
Merged

Create RadarChart#7505
kruulik merged 47 commits intomainfrom
karolis-radarchart

Conversation

@kruulik
Copy link
Contributor

@kruulik kruulik commented Feb 26, 2026

Ticket []

Description Of Changes

Creates a new RadarChart component. Refactors some charting logic to be shareable. Improves storybook UI to demonstrate light and dark modes.

Code Changes

  • Add RadarChart and RadarChart story
  • Create ChartGradient file and refactor Sparkline
  • Modify storybook preview file
  • Added gitignore rules for my workflow

Steps to Confirm

turbo run storybook and check the Chart components

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:
    • Ensure that your downrev is up to date with the latest revision on main
    • Ensure that your downgrade() migration is correct and works
      • If a downgrade migration is not possible for this change, please call this out in the PR description!
    • No migrations
  • Documentation:
    • Documentation complete, PR opened in fidesdocs
    • Documentation issue created in fidesdocs
    • If there are any new client scopes created as part of the pull request, remember to update public-facing documentation that references our scope registry
    • No documentation updates required

@vercel
Copy link
Contributor

vercel bot commented Feb 26, 2026

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

2 Skipped Deployments
Project Deployment Actions Updated (UTC)
fides-plus-nightly Ignored Ignored Preview Feb 26, 2026 8:19pm
fides-privacy-center Ignored Ignored Feb 26, 2026 8:19pm

Request Review

@kruulik kruulik marked this pull request as ready for review February 26, 2026 19:28
@kruulik kruulik requested a review from a team as a code owner February 26, 2026 19:28
@kruulik kruulik requested review from gilluminate and removed request for a team February 26, 2026 19:28
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 26, 2026

Greptile Summary

This PR adds a new RadarChart component to the fidesui component library, backed by Recharts. It also extracts shared charting primitives (ChartGradient, ChartText, chart-constants) that are already being used to refactor the existing Sparkline component. On the Storybook side, a new withAntTheme decorator enables live light/dark theme switching via a toolbar button, which is a nice developer-experience improvement.

Key observations:

  • The component architecture is clean — RadarTick and RadarDot are appropriately separated as internal sub-components, status colours are memoised, and the empty/placeholder state is handled gracefully.
  • The Storybook decorator correctly cleans up body styles in useEffect return callbacks.
  • PR size: This PR contains 15 files changed and 600 total line changes (557 insertions + 43 deletions), which exceeds the repository's 500-line-change threshold per the team's convention. Consider splitting Storybook improvements (preview / withAntTheme) from the chart component work in a future PR.
  • CHART_ANIMATION, CHART_GRADIENT, and CHART_STROKE are exported as public API from index.ts, but they are implementation-level constants. Whether external consumers of fidesui need these is worth confirming before committing to their stability.
  • CHART_TYPOGRAPHY.lineHeight is defined in chart-constants.ts but is never consumed — it is either dead code or should have a comment noting it is reserved.

Confidence Score: 4/5

  • This PR is safe to merge — it adds a new UI component and refactors shared chart utilities with no backend or data-layer changes.
  • The code is well-structured, the refactor of Sparkline is backward-compatible, and the Storybook changes are isolated to the dev environment. Minor concerns around a redundant wrapper div, an unused constant, and the public export of internal implementation constants prevent a perfect score.
  • Pay close attention to clients/fidesui/src/index.ts regarding what internal constants are exposed as public API, and clients/fidesui/src/components/charts/RadarChart.tsx for the unnecessary wrapper div.

Important Files Changed

Filename Overview
clients/fidesui/src/components/charts/RadarChart.tsx New RadarChart component built on Recharts with status-coloured dots/labels, empty-state placeholder, animation, and gradient fill. Minor: wraps ResponsiveContainer in an extra div where pointer-events-none could instead be applied via the style prop directly on ResponsiveContainer.
clients/fidesui/src/components/charts/chart-constants.ts Extracts shared chart constants (typography, animation, stroke, gradient, colour options) from Sparkline into a common module. CHART_TYPOGRAPHY.lineHeight is defined but not consumed anywhere in the current codebase.
clients/fidesui/src/components/charts/ChartGradient.tsx Reusable SVG gradient component (linear and radial) extracted from Sparkline. Clean abstraction with no notable issues.
clients/fidesui/src/components/charts/ChartText.tsx New shared SVG text component that reads Ant Design tokens for font sizing and family. Well-structured with no notable issues.
clients/fidesui/.storybook/withAntTheme.tsx New Storybook decorator enabling light/dark theme switching via a toolbar global. Well-documented with appropriate cleanup in useEffect returns.
clients/fidesui/src/components/charts/RadarChart.stories.tsx Storybook stories for RadarChart including animated click-to-replay wrapper. Uses eslint-disable comments for accessibility warnings on a clickable div.
clients/fidesui/src/components/charts/Sparkline.tsx Refactored to use shared chart constants and the new ChartGradient component. No behaviour changes.
clients/fidesui/src/index.ts Exports new RadarChart, ChartText, and chart constants. Internal implementation constants (CHART_ANIMATION, CHART_GRADIENT, CHART_STROKE) are now part of the public API.
clients/fidesui/.storybook/preview.tsx Refactored to delegate theming to the new withAntTheme decorator and expose a theme toolbar item. Clean change.

Last reviewed commit: 7b2775c

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

15 files reviewed, 3 comments

Edit Code Review Agent Settings | Greptile

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
@kruulik
Copy link
Contributor Author

kruulik commented Feb 26, 2026

@gilluminate (since you're already here) is the PR line limit a hard block? I'm not over by that much (557/500). If it is, I'll probably need to move the storybook-related work into a separate PR.

@gilluminate
Copy link
Contributor

@kruulik No, it's more of a guideline/reminder. Merging won't be blocked by that.

kruulik and others added 4 commits February 26, 2026 15:08
@kruulik
Copy link
Contributor Author

kruulik commented Feb 26, 2026

@gilluminate addressed your comments

Copy link
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.

Looks good to me. The updates are clean and address the feedback well. Ship it.

@kruulik kruulik added this pull request to the merge queue Feb 26, 2026
Merged via the queue into main with commit bb9b4bb Feb 26, 2026
40 of 41 checks passed
@kruulik kruulik deleted the karolis-radarchart branch February 26, 2026 20:41
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.

3 participants