refactor(ui): #642 PR0 — token cleanup (radii, code-surface, Cat A) - #663
Conversation
There was a problem hiding this comment.
Suggested priority: P2 (includes user-path files (packages/app/src/components/debug-bar.tsx, packages/app/src/components/dialog-select-server.tsx, packages/app/src/components/status-popover-body.tsx, packages/app/src/components/status-popover.tsx, packages/app/src/pages/session/files-tab.tsx)).
P1/P0 are reserved for maintainer confirmation. Please relabel manually if this is a release blocker, security issue, data-loss risk, or updater/runtime failure.
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR consolidates the design token scale by redefining radius tokens from ChangesDesign Token Consolidation: Radius Scale and Code Surface
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Code Review
This pull request updates the design system by consolidating border-radius tokens, introducing a new --code-surface token for code backgrounds, and removing unused brand colors. Multiple components were updated to reflect these changes, including transitions from rounded-xl to rounded-lg and updates to hover states. The theme parity tests were also expanded to cover these new tokens. Feedback suggests enhancing the test suite's robustness by improving comment-stripping logic to include single-line comments and using more precise declaration parsing for radii verification.
Perf delta summaryComparator: pass
|
DESIGN.md L179 requires fence-code blocks to use the --code-surface alpha overlay, same as inline code. The previous theme registration bound editor.background to var(--color-bg-base), which Shiki's single- theme codeToHtml inlines as style="background-color: ..." on <pre class="shiki"> — overriding the plain CSS rule on .shiki and leaving fenced blocks rendered on the page base instead of the intended overlay. Bind the theme value itself to var(--code-surface). Add a source-string test in theme-parity.test.ts that locks this binding so a future Shiki theme edit cannot silently regress the spec.
theme-parity.test.ts:35 added `code` to REGULATED_PREFIXES when --code-surface landed, but undefined-tokens.test.ts kept the older regex and its "Mirrors REGULATED_PREFIXES in theme-parity.test.ts" comment drifted out of sync. A future stray var(--code-*) reference would slip past the undefined-token guard. Add `code` to undefined-tokens.test.ts REGULATED_PREFIXES regex and to the header comment that enumerates regulated prefixes. Both files now agree.
Summary
PR0 of three for #642 token redesign. Non-typography cleanup only. Introduces
--code-surfacealpha-overlay token, aligns radii dual-namespace (theme.css:root↔ Tailwind@themeboth lock to 6/10/14 px), retires Cat A unused tokens (--brand-danger,--brand-danger-hover,--radius-pill,--radius-xs,--radius-xl), migrates inline and fence code backgrounds to--code-surface, and fixes four icon-button hover sites that incorrectly usedsurface-sunken.Why
Token surface is drifting. Tailwind
@themedeclares radii in rem (0.5rem) while theme.css declares px (14px); same identifier, different value — confuses any code that hand-picks one namespace. Inline code currently piggybacks on--surface-sunken, which is also aliased to--bg-creamfor general fill — they can't both move. Cat A tokens have zero consumers and clog the file. Four icon buttons (file-search ×3, dialog-select-server ×1) usehover:bg-surface-sunkenwhich renders as a hard cream rectangle on hover instead of the standard ghost overlay.Related Issue
#642
Human Review Status
Pending.
Review Focus
packages/ui/test/theme-parity.test.ts— two new describe blocks: (a) radii dual-namespace exact-value lock at 6/10/14, (b)--code-surfacethree-block presence (light root, dark root, prefers-color-scheme dark).@themeblock (packages/ui/src/styles/tailwind/index.css):--radius-sm/md/lgswitched from rem to px;--radius-xsand--radius-xldeleted.markdown.css: inline:not(pre) > codeand.shikiboth now read--code-surface.rounded-xs|xl→rounded-sm|lg(visual is the same pixel value, just realigned identifier).Risk Notes
Visual surface: very low. Tailwind utilities
rounded-sm/md/lgpreviously resolved to 4 / 6 / 8 (rem) and now resolve to 6 / 10 / 14 (px). Same identifier, slightly larger geometry on any component using barerounded-*without a token. theme-parity test now enforces both namespaces match.Light-mode code surface changes from
#FAF9F7(cream solid) torgba(0,0,0,0.04)(alpha overlay). The two look near-identical on a white surface; on cream surfaces (--bg-cream) the overlay correctly composites darker.Cat A removals are dead code (verified by repo-wide grep — 0 consumers).
How To Verify
The 3 pre-existing failures in
icon-button-states.test.tsandbutton-states.test.ts(Button 28px / IconButton 24×24 size expectations) are stale tests ondevand unrelated to this PR.Screenshots or Recordings
Visual smoke captured locally via Playwright (home, sidebar with hover overlay, settings dialog, session with inline + fenced code). Diffs from
devare imperceptible on white surfaces; on cream surfaces the inline-code chip and fence block now composite slightly darker (intended).Checklist
dev, and my PR title and commit messages use Conventional Commits in EnglishSummary by CodeRabbit
Release Notes