Replace palette JS imports with CSS variables in Admin UI#7954
Replace palette JS imports with CSS variables in Admin UI#7954gilluminate merged 4 commits intomainfrom
Conversation
…vacy-center The fidesui palette is available both as a JS import (palette.FIDESUI_*) and as CSS variables (var(--fidesui-*)). This replaces all 40 JS imports with CSS variable equivalents, reducing build-time SCSS module resolution dependencies and aligning with the CSS-first direction of the codebase. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This is a well-scoped, mechanical refactor that removes 40 import palette from "fidesui/src/palette/palette.module.scss" calls across admin-ui and privacy-center and replaces ~114 palette.FIDESUI_* references with their "var(--fidesui-*)" CSS variable equivalents. The changes are consistent and correctly follow the FIDESUI_FOO_BAR → --fidesui-foo-bar naming convention throughout.
Highlights
Bug fix bonus: TaxonomyTreeEdge.tsx had palette.NEUTRAL_400 (missing the FIDESUI_ prefix), which silently resolved to undefined at runtime. This is fixed as a side effect of the migration — a nice catch.
Approach is sound: Using CSS variables aligns with the Ant Design token direction and decouples TypeScript build artifacts from SCSS module resolution. The palette.module.scss :export block still has internal consumers in fidesui/, so no premature removal there.
Findings
Type safety trade-off (non-blocking): The palette object gave TypeScript a compile-time guarantee that the color key existed. CSS variable strings are opaque — a typo like var(--fidesui-sucess) silently renders with no color. This is acceptable for now, but if the pattern grows it may be worth a thin typed helper or a const map. Inline comment left on AuthorizationStatus.tsx as a representative example.
ExternalTaskLayout.tsx CSS variable scope (verify during spot-check): The privacy-center's external tasks layout uses backgroundColor: "var(--fidesui-neutral-75)" in an inline style. Confirm that the fidesui CSS variable definitions are in scope for this page's bundle. If the page can be served with a reduced stylesheet, the fallback would be transparent/white rather than the neutral grey. Not a regression from this PR, but worth including in the visual spot-check.
Summary
No blockers. The migration is correct, complete (no remaining palette imports outside fidesui/), and the bug fix is a welcome bonus. The inline comments are advisory / for future consideration.
🔬 Codegraph: unavailable
💡 Write /code-review in a comment to re-run this review.
gilluminate
changed the title
lucanovera
left a comment
lucanovera
left a comment
There was a problem hiding this comment.
Thanks for removing the awkward JS imports for the palette. Found no visual regressions. Approved!
2 participants
First step of ENG-3410
Description Of Changes
The fidesui palette is exposed two ways: as a JS import (
palette.FIDESUI_*) and as CSS variables (var(--fidesui-*)), both generated from the same SCSS source map. This PR replaces all 40 JS palette imports acrossadmin-uiandprivacy-centerwith their CSS variable equivalents.This reduces build-time SCSS module resolution dependencies in TS files and aligns with the CSS-first direction the codebase is heading with Ant Design tokens.
The
:exportblock inpalette.module.scssis still needed for fidesui internals (theme config, CustomTag, Storybook) but no longer has any consumers outside offidesui/.Code Changes
import palette from "fidesui/src/palette/palette.module.scss"from 40 files (39 admin-ui + 1 privacy-center)palette.FIDESUI_*references with"var(--fidesui-*)"CSS variable stringsTaxonomyTreeEdge.tsxwherepalette.NEUTRAL_400(missingFIDESUI_prefix) was likely resolving toundefinedSteps to Confirm
npx tsc --noEmitin bothclients/admin-uiandclients/privacy-center-- should pass with no errorsgrep -r 'from "fidesui/src/palette' clients/admin-ui/src/ clients/privacy-center/Pre-Merge Checklist
CHANGELOG.mdupdated