Refactor web UI colors to shared theme tokens#21
Merged
Conversation
- Replace hard-coded status, wordmark, chip, glyph, and terminal colors with semantic CSS variables in web components - Add new app-level design tokens in `index.css` and compute them in `theme.logic.ts` - Update theme tests to assert presence and values for newly introduced app tokens - Keep behavior unchanged while improving theme penetration and consistency across light/dark themes
|
Warning Review limit reached
More reviews will be available in 26 minutes and 47 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThis PR consolidates hardcoded colors across the web application into a comprehensive CSS custom property system. The changes establish theme-driven variables for status indicators, UI components, and agent/subagent colors, refactor theme generation to use helper builders, and update all affected components plus tests to reference these variables. ChangesTheme Variables & Component Styling Consolidation
🎯 4 (Complex) | ⏱️ ~60 minutes
🚥 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 |
github-actions
Bot
added
size:L
vouch:trusted
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/theme/theme.logic.ts (1)
1-1:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFix formatting issues before merge.
The pipeline reports formatting violations. Please run the formatter to resolve:
# Run the formatter to fix issues npm run fmt # or if using a different command: pnpm fmt🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/theme/theme.logic.ts` at line 1, Run the project's formatter on the changed file (apps/web/src/theme/theme.logic.ts) to resolve the pipeline formatting violations; execute the project's formatting script (e.g., npm run fmt or pnpm fmt) and re-stage the formatted file so the PR contains the formatted version before merging.
🧹 Nitpick comments (4)
apps/web/src/components/PullRequestThreadDialog.tsx (1)
110-121: ⚡ Quick winConsider swapping the status token mapping for PR states.
The current semantic mapping may be confusing:
- "merged" →
--app-status-plan-fg(info/violet color for planning)- "open" →
--app-status-success-fg(green color for success)Typically:
- Merged PRs represent successful completion and would fit
--app-status-success-*- Open PRs are in-progress and would fit
--app-status-working-*or--app-status-plan-*The current mapping shows open PRs in green (success color) even though they're incomplete, and merged PRs in violet/info (planning color) even though they're successfully complete.
💡 Suggested semantic token swap
const statusTone = useMemo(() => { switch (resolvedPullRequest?.state) { case "merged": - return "text-[var(--app-status-plan-fg)]"; + return "text-[var(--app-status-success-fg)]"; case "closed": return "text-zinc-500 dark:text-zinc-400/80"; case "open": - return "text-[var(--app-status-success-fg)]"; + return "text-[var(--app-status-working-fg)]"; default: return "text-muted-foreground"; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/PullRequestThreadDialog.tsx` around lines 110 - 121, The statusTone mapping in the useMemo (variable statusTone, using resolvedPullRequest?.state) uses semantically inverted tokens for "merged" and "open"; update the switch so "merged" returns the success token (--app-status-success-fg) and "open" returns the working/plan token (--app-status-plan-fg or a working token if available), keeping "closed" and default unchanged; ensure the dependency remains [resolvedPullRequest?.state] so the tone updates correctly when state changes.apps/web/src/components/Sidebar.tsx (2)
813-816: ⚡ Quick winKeep the wordmark prefix sourced from branding constants
Line 815 hardcodes
$$$whilearia-labelstill usesAPP_WORDMARK_PREFIX. This can drift from configured branding and unintentionally alter the header text.💡 Proposed fix
function AppWordmarkPrefix() { return ( - <span aria-label={APP_WORDMARK_PREFIX} className="shrink-0 text-[18px] font-semibold text-[var(--app-wordmark-prefix)]">$$$</span> + <span + aria-label={APP_WORDMARK_PREFIX} + className="shrink-0 text-[18px] font-semibold text-[var(--app-wordmark-prefix)]" + > + {APP_WORDMARK_PREFIX} + </span> ); }As per coding guidelines,
apps/web/src/**/*.tsx: For UI changes inapps/web, preserve current visual language unless explicitly asked to redesign.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/Sidebar.tsx` around lines 813 - 816, The span in AppWordmarkPrefix currently renders a hardcoded "$$$" while aria-label uses APP_WORDMARK_PREFIX; replace the hardcoded visible text with the branding constant so the displayed wordmark matches the aria-label and branding config (update AppWordmarkPrefix to render APP_WORDMARK_PREFIX inside the span, keeping the existing aria-label and className intact).
5633-5642: ⚡ Quick winTokenize the remaining workspace terminal status dots
This branch still mixes hardcoded amber/teal with tokenized success color. Using status dot tokens for all states keeps theme behavior consistent.
🎨 Proposed fix
{workspace.terminalStatus && ( <span className={cn( "inline-flex size-1.5 shrink-0 rounded-full", workspace.terminalStatus.label === "Terminal input needed" - ? "bg-amber-500 dark:bg-amber-300/90" + ? "bg-[var(--app-status-warning-dot)]" : workspace.terminalStatus.label === "Terminal process running" - ? "bg-teal-500 dark:bg-teal-300/90" + ? "bg-[var(--app-status-input-dot)]" : "bg-[var(--app-status-success-dot)]", )} /> )}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/Sidebar.tsx` around lines 5633 - 5642, The span rendering the workspace terminal status dot currently mixes hardcoded colors with a tokenized success color; update the conditional inside the className (the span that checks workspace.terminalStatus.label in Sidebar.tsx) to use CSS tokens for all states instead of hardcoded amber/teal (e.g., replace "bg-amber-500 dark:bg-amber-300/90" and "bg-teal-500 dark:bg-teal-300/90" with tokenized classes like "bg-[var(--app-status-warning-dot)]" and "bg-[var(--app-status-running-dot)]" or your project's equivalent token names), keeping the same conditional branches and dark variants so theme behavior remains consistent.apps/web/src/components/TerminalScrollToBottom.tsx (1)
60-60: 💤 Low valueConsider consistent type-hint syntax for CSS variable arbitrary values.
The className mixes two styles:
border-[color:var(--app-scroll-button-border)](with explicitcolor:type hint)bg-[var(--app-scroll-button-bg)](without type hint)Both are valid in Tailwind v4, but for consistency and clarity you could add the type hint to all color-related arbitrary values or remove it from all. For example:
className="... border-[color:var(--app-scroll-button-border)] bg-[color:var(--app-scroll-button-bg)] text-[color:var(--app-scroll-button-fg)] ... hover:bg-[color:var(--app-scroll-button-hover-bg)] hover:text-[color:var(--app-scroll-button-hover-fg)]"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/TerminalScrollToBottom.tsx` at line 60, The Tailwind class string in the TerminalScrollToBottom component mixes arbitrary-value type hints (e.g., border-[color:var(--app-scroll-button-border)]) with unhinted values (e.g., bg-[var(--app-scroll-button-bg)]); update the className on the TerminalScrollToBottom component to use a consistent pattern for color arbitrary values (either add the color: type hint to bg-, text-, and hover- variants or remove the color: hint from the border- variant) so all color-related arbitrary values follow the same syntax.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/components/terminal/terminalRuntimeAppearance.ts`:
- Line 52: The theme currently sets scrollbarSliderBackground to the CSS
variable string "var(--app-scrollbar-thumb)"; xterm v6 expects a concrete color
string. Change the code that builds the theme in
apps/web/src/components/terminal/terminalRuntimeAppearance.ts to resolve the CSS
custom property to a computed value (e.g. use
getComputedStyle(document.documentElement).getPropertyValue('--app-scrollbar-thumb')
with trim() and a sensible fallback like "`#000000`" or existing theme color) and
assign that resolved value to scrollbarSliderBackground (update both occurrences
referenced by the symbol scrollbarSliderBackground); ensure the resolution runs
in a browser context before creating the ITheme object so xterm receives a real
color string.
In `@apps/web/src/components/TerminalSearch.tsx`:
- Around line 13-18: SEARCH_DECORATIONS currently holds CSS var strings which
`@xterm/addon-search` v0.16 won't resolve; change it from a static constant to a
function or regenerated object that reads computed styles (e.g. via
getComputedStyle on the terminal container or documentElement) to resolve
--app-terminal-search-* to concrete color strings (like `#RRGGBB`) and use that
concrete decorations object when calling findNext/findPrevious; also subscribe
to theme/style changes (or recompute before each search) so the resolved colors
are refreshed when the theme updates. Ensure you update references to
SEARCH_DECORATIONS in TerminalSearch.tsx (where findNext/findPrevious are
invoked) to call the new resolver and pass the computed decoration object
instead of the raw CSS variable strings.
---
Outside diff comments:
In `@apps/web/src/theme/theme.logic.ts`:
- Line 1: Run the project's formatter on the changed file
(apps/web/src/theme/theme.logic.ts) to resolve the pipeline formatting
violations; execute the project's formatting script (e.g., npm run fmt or pnpm
fmt) and re-stage the formatted file so the PR contains the formatted version
before merging.
---
Nitpick comments:
In `@apps/web/src/components/PullRequestThreadDialog.tsx`:
- Around line 110-121: The statusTone mapping in the useMemo (variable
statusTone, using resolvedPullRequest?.state) uses semantically inverted tokens
for "merged" and "open"; update the switch so "merged" returns the success token
(--app-status-success-fg) and "open" returns the working/plan token
(--app-status-plan-fg or a working token if available), keeping "closed" and
default unchanged; ensure the dependency remains [resolvedPullRequest?.state] so
the tone updates correctly when state changes.
In `@apps/web/src/components/Sidebar.tsx`:
- Around line 813-816: The span in AppWordmarkPrefix currently renders a
hardcoded "$$$" while aria-label uses APP_WORDMARK_PREFIX; replace the hardcoded
visible text with the branding constant so the displayed wordmark matches the
aria-label and branding config (update AppWordmarkPrefix to render
APP_WORDMARK_PREFIX inside the span, keeping the existing aria-label and
className intact).
- Around line 5633-5642: The span rendering the workspace terminal status dot
currently mixes hardcoded colors with a tokenized success color; update the
conditional inside the className (the span that checks
workspace.terminalStatus.label in Sidebar.tsx) to use CSS tokens for all states
instead of hardcoded amber/teal (e.g., replace "bg-amber-500
dark:bg-amber-300/90" and "bg-teal-500 dark:bg-teal-300/90" with tokenized
classes like "bg-[var(--app-status-warning-dot)]" and
"bg-[var(--app-status-running-dot)]" or your project's equivalent token names),
keeping the same conditional branches and dark variants so theme behavior
remains consistent.
In `@apps/web/src/components/TerminalScrollToBottom.tsx`:
- Line 60: The Tailwind class string in the TerminalScrollToBottom component
mixes arbitrary-value type hints (e.g.,
border-[color:var(--app-scroll-button-border)]) with unhinted values (e.g.,
bg-[var(--app-scroll-button-bg)]); update the className on the
TerminalScrollToBottom component to use a consistent pattern for color arbitrary
values (either add the color: type hint to bg-, text-, and hover- variants or
remove the color: hint from the border- variant) so all color-related arbitrary
values follow the same syntax.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: abceeb3b-9584-4dd2-8499-6cbfd37492af
📒 Files selected for processing (20)
apps/web/src/components/DiffPanel.tsxapps/web/src/components/PluginLibrary.tsxapps/web/src/components/PullRequestThreadDialog.tsxapps/web/src/components/Sidebar.logic.test.tsapps/web/src/components/Sidebar.logic.tsapps/web/src/components/Sidebar.structure.test.tsapps/web/src/components/Sidebar.tsxapps/web/src/components/SidebarHeaderNavigationControls.tsxapps/web/src/components/TerminalScrollToBottom.tsxapps/web/src/components/TerminalSearch.tsxapps/web/src/components/TerminalWorkspaceTabs.tsxapps/web/src/components/chat/ChatTranscriptPane.tsxapps/web/src/components/chat/MessagesTimeline.tsxapps/web/src/components/composer-nodes/index.tsxapps/web/src/components/terminal/TerminalActivityIndicator.tsxapps/web/src/components/terminal/terminalRuntimeAppearance.tsapps/web/src/index.cssapps/web/src/lib/subagentPresentation.tsapps/web/src/theme/theme.logic.test.tsapps/web/src/theme/theme.logic.ts
This was referenced May 31, 2026
This was referenced Jun 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
var(--app-*)theme tokens across web UI (sidebar, diff panel, plugin glyphs, terminal, chat, and PR/status visuals).apps/web/src/theme/theme.logic.tsto emit the new token set with resolved palette-driven values.apps/web/src/theme/theme.logic.test.tsand updates existing component tests to validate token usage.Testing
Summary by CodeRabbit
Release Notes