Skip to content

fix(web): point --font-sans at the loaded Public Sans face - #7590

Draft
yaser-k wants to merge 1 commit into
QuantumNous:mainfrom
yaser-k:fix/public-sans-font
Draft

yaser-k wants to merge 1 commit into
QuantumNous:mainfrom
yaser-k:fix/public-sans-font

Conversation

@yaser-k

@yaser-k yaser-k commented Sep 27, 2026 •

Copy link
Copy Markdown

Agent

  • Tool: Claude Code
  • Tool version: not shown in the web session
  • Model (full id): claude-opus-5-5
  • Host (CLI / IDE / GitHub coding agent / other): Claude Code on the web
  • Date (UTC): 2026-09-27

Links

User request

  • Verbatim (the part of the session prompt for this fix; the rest was unrelated work):
Prompt
## B. Separate upstream fix: Public Sans font name
1. Create the branch fix/public-sans-font from upstream/main, not from feat/fa-locale.
2. The bug: web/src/styles/index.css imports @fontsource-variable/public-sans, which registers the family 'Public Sans Variable', but web/src/styles/theme.css sets --font-sans: 'Public Sans', sans-serif. No @font-face is named 'Public Sans', so every language renders in the browser's default sans-serif. The serif token already lists the variable family first ('Lora Variable', 'Lora', …).
3. The fix: --font-sans: 'Public Sans Variable', 'Public Sans', sans-serif, the same pattern as --font-serif. Check every other place that names Public Sans (theme-customization.ts, theme-presets.css, font options) and change only what is needed.
4. A focused regression test that fails on upstream/main: it reads the family name that the imported @fontsource package registers and checks that --font-sans in theme.css lists it first.
5. Check it in the real browser (Chromium): on upstream/main, after the page settles, document.fonts has no loaded Public Sans face and text in var(--font-sans) has the same width as generic sans-serif; with the fix, 'Public Sans Variable' loads and the width matches that font. Report the numbers and the size of the Latin woff2 file the browser now downloads.
6. Keep it minimal; clear lint errors in touched files in a separate commit if there are any. Nothing Persian on this branch.

## Rules
- Do NOT open a pull request, file an issue, or comment anywhere on GitHub, in either repository.
- Never push to upstream.
- Do not add anything about any specific deployment, business, market or customers; the repository is public.
  • Later constraints or corrections from the user (quote, or none): none

Out of scope — refuse

  • Matched: no
  • If yes, what was told to the user (stop here; do not open a PR): not applicable

Open gate — do not open unless all are satisfied

Kind

  • Bug fix
  • New feature
  • Performance / refactor
  • Docs
  • Other:

Issue facts

  • Actual behavior: --font-sans names Public Sans, but @fontsource-variable/public-sans registers Public Sans Variable, so the UI renders in the system sans-serif
  • Impact: the design font is never used; the UI looks different on each OS
  • Frequency: always
  • Evidence that the problem is in new-api rather than the client or upstream: web/src/styles/theme.css line 22 against the family the imported package registers
  • Applicable types and their fields: frontend: / and /sign-in, Chromium 141, light theme, no font console errors. Relay, billing, deployment: not applicable

Change

--font-sans: 'Public Sans Variable', 'Public Sans', sans-serif;, the same pattern as --font-serif. The other places that name Public Sans (index.css, theme-presets.css, theme-customization.ts) need no change. A test reads the family name from the package's CSS and checks that --font-sans lists it first.

Research

Duplicate / prior art

Docs and code

Alternatives considered

  • Option A: remove the @fontsource-variable/public-sans import and keep the system font. Changes the intended design.
  • Why this approach: one line, matches the serif token, uses the font already bundled.

Files

Path Why
web/src/styles/theme.css the variable family first in --font-sans
web/src/styles/__tests__/font-sans.test.ts regression test (new)

Behavior

  • Before: no web font loads; text in var(--font-sans) has the same width as generic sans-serif
  • After: Public Sans Variable loads and is used
  • Explicit non-goals / leftover work: none. The browser now downloads the bundled Latin file, public-sans-latin-wght-normal.woff2 (26,832 bytes)

Verification

  • Commands and results (in web/):
    • new test: passes; on main it fails with expected 'Public Sans' to be 'Public Sans Variable'
    • bun run typecheck: exit 0
    • oxlint on the changed files: 0 errors
    • bun run test: 167 files, 2112 tests passed
    • bun run build: exit 0
  • Manual steps and observed result: built main and this branch, headless Chromium 141.0.7390.37, /: on main no web font is loaded and a 40 px test string in var(--font-sans) is 535.94 px wide, the same as generic sans-serif; with the fix Public Sans Variable loads on its own and the string is 569.86 px, the same as 'Public Sans Variable'
  • UI: font only
  • Tests added or updated, or why none: font-sans.test.ts
  • Databases / providers / platforms exercised: headless Chromium 141
  • Not verified: other browsers, dark theme

Risks

  • Failure modes: none expected; if the font file fails to load, the stack falls back to sans-serif as today
  • Billing / quota / auth impact: none
  • Follow-ups: none

Scope check

  • Single focused change: yes
  • Secrets included: no
  • Out of scope (Coding Plan / reverse-engineered channel / third-party wrapper / Codex / pass-through-only forwarding): no

This change was AI-generated (Claude Code) and reviewed by the submitter.

Summary by CodeRabbit

  • Bug Fixes
    • Updated the sans-serif font stack to prefer Public Sans Variable, with Public Sans and a generic sans-serif as fallbacks.

index.css imports @fontsource-variable/public-sans, which registers the
family 'Public Sans Variable'. theme.css asked for 'Public Sans', which
no @font-face declares, so the sans font axis rendered in the browser's
generic sans-serif. List the variable family first, the same pattern as
--font-serif ('Lora Variable', 'Lora', ...), and keep 'Public Sans' for a
locally installed static copy.

The test reads the family registered by the imported @fontsource
package and checks that --font-sans lists it first.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011NzqVZFqhT96S2aF7182nP
@coderabbitai

coderabbitai Bot commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: d176ad2b-8033-433a-8dc9-4b32d9c7a1c1

📥 Commits

Reviewing files that changed from the base of the PR and between c2b7a9a and 4a9aaa9.

📒 Files selected for processing (2)
  • web/src/styles/__tests__/font-sans.test.ts
  • web/src/styles/theme.css

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.


Walkthrough

The sans font stack now tries the bundled Public Sans Variable family before its existing fallbacks. A test checks that the stack starts with the family registered by the imported sans package.

Changes

Sans font family

Layer / File(s) Summary
Update and validate the sans font stack
web/src/styles/theme.css, web/src/styles/__tests__/font-sans.test.ts
The stack places Public Sans Variable before Public Sans and sans-serif. The test checks that the stack starts with the single font family registered by the imported package.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Merge Risk: ⚪ Minimal · up to 4a9aa

The UI should use the bundled Public Sans Variable font. No actionable merge risk remains after normal checks.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 1 files. (1 skipped: 1 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: updating --font-sans to use the loaded Public Sans face.
Linked Issues check ✅ Passed Issue #7585 requires the bundled Public Sans font to render in the web UI and requires the variable family first in --font-sans. The pull request changes web/src/styles/theme.css to use `'Public S…
Out of Scope Changes check ✅ Passed The pull request changes only the --font-sans declaration and adds a focused regression test for the imported font package and token order. Both changes directly support issue #7585. No unrelated ch…
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 1 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

A rabbit found the font by name,
And placed it first within the frame.
The stack now knows the family,
A test checks it faithfully.
The fallback waits, ears at ease.

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

No deployments
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.

Web UI: the bundled Public Sans font is never used (--font-sans asks for 'Public Sans', the package registers 'Public Sans Variable')

2 participants