Conversation
Align antTheme re-export to antd/lib to avoid dual module instances breaking ConfigProvider context. Add dark() helper to reduce repeated generate() calls in dark-theme.ts. Add Alert info tokens for both light and dark themes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Keep Inter loaded for Chakra UI theme compatibility while the new Basier Square fonts are used by the Ant Design theme. 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
|
Dependency Review✅ No vulnerabilities found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice. Scanned FilesNone |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
Clean, focused PR. The font files are well-organized, font-display: swap is used throughout, and the Cypress import is a nice touch to keep component tests consistent with the app. A few things worth addressing:
Suggestions
Fallback font stacks regressed (inline comments on lines 17–18 of default-theme.ts): Both fontFamily and fontFamilyCode swap the previous system-font fallbacks for web fonts (DM Sans, DM Mono, Courier Prime) that aren't self-hosted and aren't system fonts. On any browser, these will silently fail and the stack will fall through to Helvetica/monospace. The old stack had proper cross-platform system fonts (-apple-system, BlinkMacSystemFont, Segoe UI, SFMono-Regular, Consolas, etc.) that covered Mac/Windows/Linux gracefully.
Basier Square Mono has weight/style gaps (inline comment on line 77 of fonts.scss): Weight 400 has no italic, and weight 500 has italic but no normal. May be intentional if those files don't exist from the vendor, but worth confirming.
Eliza is declared but unused (inline comment on line 111 of fonts.scss): The two Eliza @font-face blocks and .woff2 files are included but the font name isn't referenced in default-theme.ts. Browsers won't download the files unless the font-family name appears in a CSS rule, so there's no performance cost — but confirming the intended usage (or pointing to where else it's referenced) would help reviewers understand the intent.
Nice to Have
Adding unicode-range to the @font-face declarations would let browsers skip downloading font files for character ranges not present on the page, which can reduce initial load for apps with narrow character sets. Not a blocker for this PR.
| token: { | ||
| fontFamily: `Inter, -apple-system, BlinkMacSystemFont, "Segoe UI", Helvetica, Arial, sans-serif, "Apple Color Emoji", "Segoe UI Emoji", "Segoe UI Symbol"`, | ||
| fontFamilyCode: `"Basier Square Mono", 'SFMono-Regular', Consolas, 'Liberation Mono', Menlo, Courier, monospace`, | ||
| fontFamily: `"Basier Square", "DM Sans", Helvetica, sans-serif`, |
There was a problem hiding this comment.
"DM Sans" is not a system font and is not self-hosted in this PR, so it will never actually render as a fallback — the browser will skip straight to Helvetica. The previous stack had real system fonts (-apple-system, BlinkMacSystemFont, "Segoe UI", Arial) that would cover Mac, Windows, and Linux. Consider replacing "DM Sans" with system fonts so Windows/Linux users get a decent fallback if Basier Square fails to load:
fontFamily: `"Basier Square", -apple-system, BlinkMacSystemFont, "Segoe UI", Helvetica, Arial, sans-serif`,| fontFamily: `Inter, -apple-system, BlinkMacSystemFont, "Segoe UI", Helvetica, Arial, sans-serif, "Apple Color Emoji", "Segoe UI Emoji", "Segoe UI Symbol"`, | ||
| fontFamilyCode: `"Basier Square Mono", 'SFMono-Regular', Consolas, 'Liberation Mono', Menlo, Courier, monospace`, | ||
| fontFamily: `"Basier Square", "DM Sans", Helvetica, sans-serif`, | ||
| fontFamilyCode: `"Basier Square Mono", "DM Mono", "Courier Prime", monospace`, |
There was a problem hiding this comment.
Same issue here — "DM Mono" and "Courier Prime" are not system fonts and neither is self-hosted, so both will silently fail and the browser jumps to monospace. The previous fallback chain ('SFMono-Regular', Consolas, 'Liberation Mono', Menlo, Courier) were all real system fonts. Consider:
fontFamilyCode: `"Basier Square Mono", 'SFMono-Regular', Consolas, 'Liberation Mono', Menlo, Courier, monospace`,| font-display: swap; | ||
| } | ||
|
|
||
| @font-face { |
There was a problem hiding this comment.
The Basier Square Mono declarations have some weight/style gaps compared to Basier Square:
- Weight 400 has no italic variant
- Weight 500 has italic but no normal variant (
basier-square-mono-medium.woff2is not included)
If these files don't exist from the font vendor, that's fine — browsers will synthesize italic (by slanting) and interpolate missing weights. But if the files do exist and were just overlooked, adding them would prevent browser synthesis artifacts.
|
|
||
| // Eliza | ||
|
|
||
| @font-face { |
There was a problem hiding this comment.
The Eliza font is declared here but doesn't appear in default-theme.ts (it's not referenced in fontFamily or fontFamilyCode). Is it used somewhere else in the codebase, or is this a placeholder for a future use? If it's not currently used anywhere, the @font-face declarations and .woff2 files add weight to the bundle without benefit (browsers won't download the files unless the font-family name is actually referenced in a CSS rule, so it's fine from a performance standpoint — just worth confirming the intent).
Greptile SummaryThis PR self-hosts Basier Square, Basier Square Mono, and Eliza fonts inside Note on PR size: This PR touches 22 files, which exceeds the team's 15-file guideline. 14 of those files are binary font assets with no code complexity, so the spirit of the rule is arguably not violated — but worth flagging for awareness. Key changes and findings:
Confidence Score: 4/5Safe to merge; no runtime errors or data-loss risk — only minor font-fallback and missing-face improvements remain. The changes are straightforward CSS/SCSS font declarations and a small theme-token update. The two P2 comments (missing Mono weight-500 normal face, non-system fallbacks) are cosmetic and won't break functionality, but are worth addressing before the fonts land in a polished state. No logic, API, or data-layer changes are present. clients/fidesui/src/fonts/fonts.scss (missing Basier Square Mono medium normal face) and clients/fidesui/src/ant-theme/default-theme.ts (fallback font stack). Important Files Changed
Reviews (1): Last reviewed commit: "fix bad merge and import correct font" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| font-family: "Basier Square Mono"; | ||
| src: url("./basier-square-mono-semibold.woff2") format("woff2"); | ||
| font-weight: 600; | ||
| font-style: normal; | ||
| font-display: swap; | ||
| } | ||
|
|
||
| @font-face { | ||
| font-family: "Basier Square Mono"; | ||
| src: url("./basier-square-mono-semibold-italic.woff2") format("woff2"); |
There was a problem hiding this comment.
The Basier Square Mono declarations include font-weight: 500, font-style: italic but there is no corresponding font-weight: 500, font-style: normal face. When code renders non-italic font-weight: 500 monospace text, the browser will synthesize the weight from the nearest declared face (likely 400 normal), which may look noticeably different from a true medium weight.
If the basier-square-mono-medium.woff2 file is available, add a declaration for it:
| font-family: "Basier Square Mono"; | |
| src: url("./basier-square-mono-semibold.woff2") format("woff2"); | |
| font-weight: 600; | |
| font-style: normal; | |
| font-display: swap; | |
| } | |
| @font-face { | |
| font-family: "Basier Square Mono"; | |
| src: url("./basier-square-mono-semibold-italic.woff2") format("woff2"); | |
| @font-face { | |
| font-family: "Basier Square Mono"; | |
| src: url("./basier-square-mono-regular.woff2") format("woff2"); | |
| font-weight: 400; | |
| font-style: normal; | |
| font-display: swap; | |
| } | |
| @font-face { | |
| font-family: "Basier Square Mono"; | |
| src: url("./basier-square-mono-medium.woff2") format("woff2"); | |
| font-weight: 500; | |
| font-style: normal; | |
| font-display: swap; | |
| } | |
| @font-face { | |
| font-family: "Basier Square Mono"; | |
| src: url("./basier-square-mono-medium-italic.woff2") format("woff2"); | |
| font-weight: 500; | |
| font-style: italic; | |
| font-display: swap; | |
| } |
If the medium-normal file is intentionally absent (not part of the license), a comment here would help future maintainers understand why the symmetry is broken.
| fontFamily: `"Basier Square", "DM Sans", Helvetica, sans-serif`, | ||
| fontFamilyCode: `"Basier Square Mono", "DM Mono", "Courier Prime", monospace`, |
There was a problem hiding this comment.
Both "DM Sans" and "DM Mono" are web fonts, not system fonts. If "Basier Square" / "Basier Square Mono" fail to load (e.g. network error, CORS issue in SSR), the browser falls through to these fonts which are also unlikely to be available on the user's system, then all the way to Helvetica / monospace. The original fontFamily string used -apple-system, BlinkMacSystemFont as the first fallbacks, which provide a native OS font immediately.
Consider interleaving system-font fallbacks before the generic category:
| fontFamily: `"Basier Square", "DM Sans", Helvetica, sans-serif`, | |
| fontFamilyCode: `"Basier Square Mono", "DM Mono", "Courier Prime", monospace`, | |
| fontFamily: `"Basier Square", "DM Sans", -apple-system, BlinkMacSystemFont, "Segoe UI", Helvetica, Arial, sans-serif`, | |
| fontFamilyCode: `"Basier Square Mono", "DM Mono", "SFMono-Regular", Consolas, "Liberation Mono", Menlo, Courier, monospace`, |
kruulik
changed the title
1 participant
Description Of Changes
Add self-hosted Basier Square, Basier Square Mono, and Eliza font files to align with our brand guidelines.
Inter is retained for Chakra UI theme compatibility.
Code Changes
clients/fidesui/src/fonts/fonts.scss- New@font-facedeclarations for Basier Square (400-700), Basier Square Mono, and Elizaclients/fidesui/src/fonts/*.woff2- 14 self-hosted font filesclients/fidesui/src/ant-theme/global.scss- Import new fonts stylesheetclients/fidesui/src/ant-theme/default-theme.ts- UpdatefontFamilyandfontFamilyCodetokens to use Basier SquareSteps to Confirm
cd clients/admin-ui && npm run build) — should succeed without errors.woff2files load on demandPre-Merge Checklist
CHANGELOG.mdupdatedmaindowngrade()migration is correct and works