-
Notifications
You must be signed in to change notification settings - Fork 11.5k
feat: rename --font-cal to --font-heading + use Cal Sans UI #26064
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Deployment failed with the following error: View Documentation: https://vercel.com/docs/accounts/team-members-and-roles |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
This PR has been marked as stale due to inactivity. If you're still working on it or need any help, please let us know or update the PR to keep it active. |
Co-Authored-By: pasquale@cal.com <pasquale@cal.com>
Co-Authored-By: pasquale@cal.com <pasquale@cal.com>
|
Deployment failed with the following error: View Documentation: https://vercel.com/docs/accounts/team-members-and-roles |
|
Deployment failed with the following error: View Documentation: https://vercel.com/docs/accounts/team-members-and-roles |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
4 issues found across 74 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/web/app/layout.tsx">
<violation number="1" location="apps/web/app/layout.tsx:109">
P1: Typo: Extra trailing double quote `"` inside the template literal will be included as part of the className, breaking the `antialiased` class application.</violation>
</file>
<file name="packages/platform/atoms/globals.css">
<violation number="1" location="packages/platform/atoms/globals.css:245">
P2: Duplicate CSS variable declaration. The line `--font-cal: var(--font-cal);` was changed to `--font-heading: var(--font-heading);`, but `--font-heading` is already declared on the preceding line. This creates a redundant duplicate that should be removed.</violation>
</file>
<file name="packages/coss-ui/src/fonts/README.md">
<violation number="1" location="packages/coss-ui/src/fonts/README.md:26">
P2: Documentation incorrectly states `fontHeading` uses "Cal Sans Regular font" but the implementation uses `CalSans-SemiBold.woff2`. Should be "Cal Sans SemiBold font" to match the actual font file.</violation>
<violation number="2" location="packages/coss-ui/src/fonts/README.md:30">
P2: Incorrect directory path in documentation. Should be `packages/coss-ui/src/fonts/` instead of `packages/ui/src/fonts/` to match the actual file location.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
apps/web/app/layout.tsx
Outdated
| </head> | ||
| <body | ||
| className="dark:bg-default bg-subtle antialiased" | ||
| className={`${fontSans.variable} ${fontHeading.variable} font-sans dark:bg-default bg-subtle antialiased"`} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1: Typo: Extra trailing double quote " inside the template literal will be included as part of the className, breaking the antialiased class application.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/app/layout.tsx, line 109:
<comment>Typo: Extra trailing double quote `"` inside the template literal will be included as part of the className, breaking the `antialiased` class application.</comment>
<file context>
@@ -114,16 +105,8 @@ export default async function RootLayout({ children }: { children: React.ReactNo
- </head>
<body
- className="dark:bg-default bg-subtle antialiased"
+ className={`${fontSans.variable} ${fontHeading.variable} font-sans dark:bg-default bg-subtle antialiased"`}
style={
isEmbed
</file context>
| className={`${fontSans.variable} ${fontHeading.variable} font-sans dark:bg-default bg-subtle antialiased"`} | |
| className={`${fontSans.variable} ${fontHeading.variable} font-sans dark:bg-default bg-subtle antialiased`} |
✅ Addressed in a2123cb
packages/platform/atoms/globals.css
Outdated
| --radius: var(--radius); | ||
| --font-heading: var(--font-heading); | ||
| --font-cal: var(--font-cal); | ||
| --font-heading: var(--font-heading); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: Duplicate CSS variable declaration. The line --font-cal: var(--font-cal); was changed to --font-heading: var(--font-heading);, but --font-heading is already declared on the preceding line. This creates a redundant duplicate that should be removed.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/platform/atoms/globals.css, line 245:
<comment>Duplicate CSS variable declaration. The line `--font-cal: var(--font-cal);` was changed to `--font-heading: var(--font-heading);`, but `--font-heading` is already declared on the preceding line. This creates a redundant duplicate that should be removed.</comment>
<file context>
@@ -242,7 +242,7 @@
--radius: var(--radius);
--font-heading: var(--font-heading);
- --font-cal: var(--font-cal);
+ --font-heading: var(--font-heading);
--shadow-dropdown: var(--shadow-dropdown);
--shadow-switch-thumb: var(--shadow-switch-thumb);
</file context>
✅ Addressed in a2123cb
packages/coss-ui/src/fonts/README.md
Outdated
|
|
||
| ## Adding New Fonts | ||
|
|
||
| 1. Place the font file in this directory (`packages/ui/src/fonts/`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: Incorrect directory path in documentation. Should be packages/coss-ui/src/fonts/ instead of packages/ui/src/fonts/ to match the actual file location.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/coss-ui/src/fonts/README.md, line 30:
<comment>Incorrect directory path in documentation. Should be `packages/coss-ui/src/fonts/` instead of `packages/ui/src/fonts/` to match the actual file location.</comment>
<file context>
@@ -0,0 +1,49 @@
+
+## Adding New Fonts
+
+1. Place the font file in this directory (`packages/ui/src/fonts/`)
+2. Add a new font configuration in `index.ts`:
+
</file context>
✅ Addressed in a2123cb
| ## Available Fonts | ||
|
|
||
| - `fontSans` - Cal Sans UI variable font (supports multiple weights and modes) | ||
| - `fontHeading` - Cal Sans Regular font |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: Documentation incorrectly states fontHeading uses "Cal Sans Regular font" but the implementation uses CalSans-SemiBold.woff2. Should be "Cal Sans SemiBold font" to match the actual font file.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/coss-ui/src/fonts/README.md, line 26:
<comment>Documentation incorrectly states `fontHeading` uses "Cal Sans Regular font" but the implementation uses `CalSans-SemiBold.woff2`. Should be "Cal Sans SemiBold font" to match the actual font file.</comment>
<file context>
@@ -0,0 +1,49 @@
+## Available Fonts
+
+- `fontSans` - Cal Sans UI variable font (supports multiple weights and modes)
+- `fontHeading` - Cal Sans Regular font
+
+## Adding New Fonts
</file context>
eunjae-lee
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall it looks great in the app
Ryukemeister
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test atoms, everything looks good!
…6064) * feat: rename --font-cal to --font-heading + use Cal Sans UI * chore: remove font-weight from font-heading elements and CSS variables Co-Authored-By: pasquale@cal.com <pasquale@cal.com> * chore: remove unneeded css vars * fix: cubic issues --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: pasquale@cal.com <pasquale@cal.com> Co-authored-by: Rajiv Sahal <sahalrajiv-extc@atharvacoe.ac.in>
What does this PR do?
Standardizes typography across the app by switching body text to Cal Sans UI and renaming the heading font from
--font-cal/font-calto--font-heading/font-heading. Font loading is now centralized via@coss/ui/fontsfor consistent usage and simpler setup.Changes
@coss/ui/fonts(fontSans= Cal Sans UI variable,fontHeading= Cal Sans SemiBold)fontSansandfontHeadingvariables on body in app layout and_document; removed scattered local font importsfont-calusages tofont-headingand updated related utilities (e.g.,prose-h3:font-heading)!font-caltofont-heading!(Tailwind v4 syntax)font-semibold,font-medium, etc.) from elements withfont-headingsince the heading font is already semibold--font-headingand aligned platform/global stylesMigration Guide
font-calwithfont-headingand CSS var--font-calwith--font-heading!font-calwithfont-heading!(Tailwind v4 important syntax)@coss/ui/fontsand add${fontSans.variable} ${fontHeading.variable}to the body classprose-h3:font-caltoprose-h3:font-headingfont-heading(the font is already semibold)Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
font-heading!) works correctlyHuman Review Checklist
font-semibold/font-mediumclasses (the heading font is inherently semibold)font-heading!syntax works correctly in Tailwind v4Link to Devin run: https://app.devin.ai/sessions/fb0be98a05a444bd9a71539f53701308
Requested by: pasquale@cal.com