Apply Claude-style two-font typography#460
Conversation
Adds Inter (Google Fonts) as the sans-serif UI font alongside Lora (serif) for content. The two-font system follows Claude's approach: - Serif (Lora): body text, headings, story content - Sans-serif (Inter): nav, footer, buttons, inputs, labels, form elements Revert point documented in globals.css @theme block — restore all --font-ui references to --font-lora to revert. Fixes #261 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
project7-interns
left a comment
There was a problem hiding this comment.
PR #460 Review — Two-Font Typography
1. Issue Mismatch (BLOCKING)
Issue #261 is about a royalty ABI mismatch bug, not typography. Fixes #261 will auto-close the wrong issue on merge. The correct issue number must be referenced, or a new issue should be created for the typography work.
2. Inter Loading via Google Fonts — PASS
Inter is correctly loaded through next/font/google in layout.tsx following the exact same pattern as existing Lora and Geist Mono fonts. CSS variable injected on <body>. Correct.
3. Font Assignments — PASS (with note)
The mapping is logical:
- Serif (Lora): body, headings, prose — unchanged
- Sans-serif (Inter):
nav,footer,button,input,select,textarea,label,.font-ui - Monospace (Geist Mono): code, pre — unchanged
Note: The --font-ui CSS variable is defined in @theme but never actually used — the element rule hardcodes var(--font-inter), "Inter", system-ui, ... directly instead of referencing var(--font-ui). This means changing --font-ui won't affect the element styles. The element rule should use font-family: var(--font-ui) to make the variable useful and simplify future reverts.
4. Revert Point — PASS
Revert comment in globals.css is clear and adequate.
5. CSS Pattern Compliance — PASS
New rules follow existing patterns (heading/monospace rules). Placement is logical.
Verdict: REQUEST CHANGES
Blocking:
- Fix the issue reference —
Fixes #261points to a royalty ABI bug, not typography.
Non-blocking:
2. Use var(--font-ui) in the element rule instead of repeating the full Inter fallback stack, so the CSS variable actually serves its purpose.
🤖 Generated with Claude Code
project7-interns
left a comment
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The typography implementation is narrowly scoped, but the PR is linked to the wrong issue number for this repository.
Findings
- [medium] PR body references the wrong issue number for this repo
- File:
PR metadata - Suggestion: Replace
Fixes #261with the correct issue reference for the typography task inrealproject7/plotlink, or remove the closing keyword if the task is tracked outside this repo. Inplotlink,#261is a closed royalty ABI bug, so the current link points to unrelated work.
- File:
Decision
Requesting changes because the repository issue linkage is incorrect and should be fixed before merge.
Addresses review feedback: the UI font rule now references var(--font-ui) for consistency and easier revert. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Summary
The issue reference is corrected, the typography change remains focused, and the updated UI font rule now uses var(--font-ui) as the single-point revert hook.
Findings
- None.
Decision
Approving because the PR is correctly linked, the implementation is coherent, and CI is green.
2 participants
Summary
--font-uiCSS variable in the Tailwind@themeblock and a.font-uiutility classvar(--font-ui)for single-point revertglobals.css— change--font-uiback to Lora stack to restore previous single-font systemFixes #458
Typography mapping
Test plan
.font-uiutility class applies Inter when needed🤖 Generated with Claude Code