Repository navigation
chore(theme): migrate inline layout/typography CSS-var styles to Tailwind - #101
Conversation
…wind Promote the type-scale tokens in @theme inline from --fs-* to Tailwind v4's --text-* namespace so font-size utilities (text-h1, text-body, …) auto-generate, then drop the ~30 inline `style={{ fontSize: var(--fs-X) }}` sites in favour of those classes (companion lineHeight/letterSpacing → leading-[…] / tracking-[…]). Layout --frame-* tokens keep their names but move from inline styles to arbitrary-value classes (px-[var(--frame-gutter)], max-w-[var(--frame-max)], …), matching the existing scroll-mt-[var(--nav-h)] precedent. Updates the .prose var(--fs-*) refs to --text-* accordingly. Closes #78 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Deploying website with
|
| Latest commit: |
a09e0e2
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://04031f29.website-70y.pages.dev |
| Branch Preview URL: | https://chore-theme-tailwind-vars.website-70y.pages.dev |
There was a problem hiding this comment.
Code Review
This pull request migrates font-size and layout styles from inline React style props to Tailwind CSS classes across various components. This change is supported by renaming CSS variables in globals.css to the --text-* namespace, enabling Tailwind v4's automatic utility generation. Feedback suggests that layout-related variables should be promoted to the Tailwind theme (e.g., using the --spacing-* namespace) to avoid the use of arbitrary values in class lists and improve readability.
| className={`relative flex min-h-[min(100svh,var(--frame-min-h-cap))] scroll-mt-[var(--nav-h)] flex-col px-[var(--frame-gutter)] py-[var(--frame-pad-y)] ${TONE_CLASS[tone]} ${className}`} | ||
| > | ||
| <div | ||
| className="@container relative z-10 mx-auto flex w-full flex-1 flex-col" | ||
| style={{ maxWidth: "var(--frame-max)" }} | ||
| > | ||
| <div className="@container relative z-10 mx-auto flex w-full max-w-[var(--frame-max)] flex-1 flex-col"> |
There was a problem hiding this comment.
Using arbitrary values for theme variables makes the class list harder to read. If the variables in globals.css are promoted to the --spacing-* namespace (as suggested in that file), this can be simplified significantly using standard Tailwind utility classes.
| className={`relative flex min-h-[min(100svh,var(--frame-min-h-cap))] scroll-mt-[var(--nav-h)] flex-col px-[var(--frame-gutter)] py-[var(--frame-pad-y)] ${TONE_CLASS[tone]} ${className}`} | |
| > | |
| <div | |
| className="@container relative z-10 mx-auto flex w-full flex-1 flex-col" | |
| style={{ maxWidth: "var(--frame-max)" }} | |
| > | |
| <div className="@container relative z-10 mx-auto flex w-full max-w-[var(--frame-max)] flex-1 flex-col"> | |
| className={"relative flex min-h-[min(100svh,var(--spacing-frame-min-h-cap))] scroll-mt-[var(--nav-h)] flex-col px-frame-gutter py-frame-pad-y " + TONE_CLASS[tone] + " " + className} | |
| > | |
| <div className="@container relative z-10 mx-auto flex w-full max-w-frame-max flex-1 flex-col"> |
References
- Avoid migrating inline styles to Tailwind arbitrary values (e.g., px-[var(--variable)]) if the variable is not part of the Tailwind theme configuration. Such migrations should be handled globally by first promoting the variables to the theme to ensure consistency across the codebase.
There was a problem hiding this comment.
Deliberate — this is the split called out in #78's "Approach (open question)" section. We took Approach A for the typography tokens (--fs-* → --text-*, real text-h1/text-body/… utilities), but Approach B for the layout --frame-* tokens: keep the names, consume them via arbitrary-value classes, matching the existing scroll-mt-[var(--nav-h)] precedent in this same file. Promoting --frame-* to --spacing-*/--container-* renames the public CSS-var API, so it's intentionally kept separate.
Tracked for a possible future pass in #102 — leaving #101 as-is.
There was a problem hiding this comment.
Pull request overview
This PR completes the migration away from inline style={{ ... var(--...) }} usage for typography (--fs-*) and layout (--frame-*) tokens by moving them into Tailwind v4 utilities / arbitrary-value classes, keeping --reveal-delay inline where it’s semantically a variable write for animations.
Changes:
- Renames the type-scale tokens in
app/globals.cssfrom--fs-*to Tailwind v4’s--text-*namespace sotext-*font-size utilities are generated, and updates.proseto reference the new tokens. - Replaces inline font-size styles across UI/site/blog components with
text-h*,text-body,text-lead, etc., plus bracket utilities forleading-*/tracking-*where needed. - Replaces inline
--frame-*layout styles (gutter/padding/max width/min height) with arbitrary-value Tailwind classes likepx-[var(--frame-gutter)],max-w-[var(--frame-max)], etc., and removes now-emptystyleprops.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| components/ui/PostRow.tsx | Replaces inline font-size vars with text-h3 / text-body utilities. |
| components/ui/MethodRow.tsx | Migrates method-row and body font sizing to text-method-row / text-body and keeps leading/tracking via utilities. |
| components/ui/Frame.tsx | Converts frame sizing/padding/max-width inline styles to arbitrary-value Tailwind classes. |
| components/ui/Figure.tsx | Converts inline font-size/letter-spacing to text-body and tracking-[...]. |
| components/ui/FaqItem.tsx | Replaces inline font sizing with text-lead / text-body while keeping --reveal-delay inline. |
| components/ui/ComparisonRow.tsx | Moves fontSize from inline style to text-body class while preserving reveal delay style var. |
| components/site/Method.tsx | Replaces inline font-size with text-h3. |
| components/site/Hero.tsx | Converts headline/body/CTA font-size inline styles to text-h1 / text-body / text-cta. |
| components/site/Footer.tsx | Migrates frame padding/max-width to bracket utilities and footer micro type to text-micro. |
| components/site/Faq.tsx | Updates FAQ heading sizing to text-h2. |
| components/site/Comparison.tsx | Converts heading/body/price sizing to text-h2 / text-body / text-price, keeping reveal delay inline. |
| components/site/Close.tsx | Moves body/links sizing to text-body / text-lead, preserving reveal delay inline. |
| components/site/Chrome.tsx | Replaces frame gutter/max-width inline styles with bracket utilities. |
| app/globals.css | Renames --fs-* → --text-* in @theme inline and updates .prose references/comments accordingly. |
| app/blog/page.tsx | Updates blog index typography from inline styles to text-h2 / text-lead / text-body. |
| app/blog/[slug]/page.tsx | Updates blog post title and prev/next link typography to text-h2 / text-lead. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Follow-ups from the #101 review: - PostRow META + Pill: text-[0.6875rem] -> text-micro (the literal equals the --text-micro token now that this PR promoted it; one source of truth). - globals.css: spell out that the --text-* keys are bare (no --text-*--line-height sibling), so the utilities set font-size only. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`--text-display: clamp(3.5rem, 14vw, 12.5rem)` in the `@theme inline` block had no call sites — grep across app/, components/, lib/, content/ found only the declaration, and Tailwind v4 never emits a `text-display` utility because it's never used in a class. Dead since before #101 (was `--fs-display`; #101 renamed it along with the rest of the scale). Closes #103 Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore(theme): drop unreferenced --font-sans token The `--font-sans` entry in `@theme inline` was never read: `html, body` applies `font-family: var(--font-geist-sans), …` directly, and no component uses the `font-sans` utility class (verified by grep across `.ts(x)` / `.mdx`). Removing it loses no behavior — the `font-sans` utility falls back to Tailwind's default sans stack — and shrinks the theme surface. Closes #105 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * refactor(theme): promote --frame-* tokens to Tailwind v4 namespaces Finishes the migration left at the end of #78 / #101: the typography half landed under the `--text-*` namespace (Approach A); the layout half stayed as raw vars consumed via `px-[var(--frame-gutter)]` / `max-w-[var(--frame-max)]` arbitrary-value classes (Approach B). Renames so layout tokens auto-generate utilities: - `--frame-gutter` → `--spacing-frame-gutter` ⇒ `px-frame-gutter` - `--frame-pad-y` → `--spacing-frame-pad-y` ⇒ `py-frame-pad-y` - `--frame-max` → `--container-frame` ⇒ `max-w-frame` - `--frame-min-h-cap` → `--spacing-frame-min-h: min(100svh, 54rem)` ⇒ `min-h-frame-min-h` (the `min()` was already at the lone call site; folding it into the token removes the last arbitrary value) Frame.tsx, Footer.tsx, and Chrome.tsx's <nav> switch to the generated utilities. The mobile-menu close button's raw CSS ref becomes `right: var(--spacing-frame-gutter)`. The `--nav-h` comment block is updated to match. Closes #102 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * refactor(chrome): extract resolveActiveSection helper + unit tests The home-route-gated nav-tone ternary in `Chrome.tsx` (the #112 fix plus the back-nav flash guard) was load-bearing but uncovered. Pulling it into a pure helper lets it be unit-tested without dragging in jsdom / RTL / IntersectionObserver mocks — the function takes `(pathname, scrolled, observed, fallback)` and is independent of any runtime. Adds five cases from the issue, including the two regressions the original fix exists to prevent: off-home stale `active` (the #112 bug) and back-nav pre-scroll flash. `vitest.config.ts`'s existing `**/*.test.ts` include glob picks the new file up unchanged. Closes #117 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * chore(chrome): drop #112 back-reference from test name The "(the #112 bug)" parenthetical belongs in PR / commit history, not in a long-lived test name where it rots once the issue is forgotten. The behavior the test guards (off-home + scrolled → fall back) is already self-describing. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * refactor(chrome): tighten resolveActiveSection types + add exact-match test Addresses review feedback on the helper introduced in 39ac7c8: - Constrain `<T extends string>` so unrelated types can't unify silently (`resolveActiveSection(p, s, 42, "intro")` stops compiling). Preserves the literal-union inference at the call site. - Move the multi-line guard rationale from above the call site in Chrome.tsx to above the function in chrome-active.ts, where the `pathname === "/" && scrolled` logic actually lives. The original framing leaked Chrome.tsx-specific vocabulary (`DARK_SECTIONS`, `text-paper`); tightened to describe the helper's own contract. - Add a 6th vitest case asserting the helper is exact-match (`/?utm=x` falls back), defending against a future caller that swaps usePathname() for something query-/hash-including. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * refactor(chrome): drop T extends string + correct guard rationale Walks back two refinements from 0730f82: - The constraint blocked no real misuse — the sole call site already infers T to a literal-string union from the caller's typing. - The rehomed comment said the off-home observer "holds stale values" but the observer is never built off-home (Chrome.tsx's IO useEffect early-returns when no section nodes exist). It also kept the inherited "past the intro hero" framing for `scrolled`, which is misleading — scrolled flips at scrollY > 8, well inside intro. Rewritten as the "new observer has had a moment to fire" heuristic it actually is, with the back-nav restored-scroll caveat noted. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Summary
Follow-up to the color-token migration (#4 / #75 / #76): moves the remaining inline-style CSS-var usages — typography (
--fs-*) and layout (--frame-*) — offstyle={{}}.@theme inlinefrom--fs-*to Tailwind v4's--text-*namespace, so real font-size utilities auto-generate (text-h1,text-h2,text-h3,text-body,text-lead,text-cta,text-micro,text-method-row,text-price, plus the unusedtext-display— see chore(theme): drop the unused --text-display token #103) — font-size only (the keys are bare, no--text-*--line-heightsibling), identical to the old inline behaviour. Converted ~30style={{ fontSize: "var(--fs-X)" }}call sites toclassName="… text-X"; companion declarations in the same object became bracket utilities (lineHeight: "0.9"→leading-[0.9],letterSpacing: "-0.01em"→tracking-[-0.01em]);--reveal-delaywrites preserved. Updated the 4.prosevar(--fs-*)refs + the explanatory comment to--text-*. Also folded the two hard-codedtext-[0.6875rem]literals inPill.tsx/PostRow.tsx(META) into the newtext-microutility — they were the same magnitude as--text-micro.--frame-*token names kept; the inline styles inFrame.tsx,Footer.tsx,Chrome.tsxbecame arbitrary-value classes (px-[var(--frame-gutter)],py-[var(--frame-pad-y)],max-w-[var(--frame-max)],min-h-[min(100svh,var(--frame-min-h-cap))]); now-emptystyleprops removed. Matches the existingscroll-mt-[var(--nav-h)]precedent;globals.css'sright: var(--frame-gutter)unchanged. Promoting--frame-*to a Tailwind namespace (the alternative discussed in chore(theme): migrate inline layout/typography CSS-var styles to Tailwind #78) is tracked separately in chore(theme): promote --frame-* layout tokens to Tailwind namespaces #102.Out of scope (untouched):
scroll-mt-[var(--nav-h)], the--reveal-delaywrites.Net −90 lines across 17 files.
prettier-plugin-tailwindcssre-sorted the affected class lists.Verification
pnpm format(clean) ·pnpm lint✅ ·pnpm exec tsc --noEmit✅ ·pnpm test✅ (45/45) ·pnpm build✅ (14/14 static pages)grep var(--fs-in source → none;var(--text-*)inglobals.css→ the 4.proserefs only.text-*utilities emitted (incl..text-micro{font-size:.6875rem});--text-body/-lead/-h3(the.prose-referenced ones) present in:root,:host;--frame-*still emitted (consumed by the bracket utilities + the mobile-menuright:);--nav-hintact.out/HTML: zero leftoverstyle="font-size:var(--fs…)"/var(--frame…);text-*and[var(--frame-*)]classes present;style="--reveal-delay:…"still inline.styleattrs dropped from HTML — roughly flat).Review follow-up
text-[0.6875rem]→text-microinPostRow.META+Pill.tsx; expanded the@theme inlinetype-scale comment to spell out that the--text-*keys are bare (utilities set font-size only, so call sites bring their ownleading-*). (commita09e0e2)geminisuggestion to promote--frame-*to a Tailwind namespace was a deliberate non-goal (Approach B per chore(theme): migrate inline layout/typography CSS-var styles to Tailwind #78) — tracked in chore(theme): promote --frame-* layout tokens to Tailwind namespaces #102.--text-displaytoken (dead code, predates this PR) — tracked in chore(theme): drop the unused --text-display token #103.Closes #78
🤖 Generated with Claude Code