refactor(ui): collapse light surface system, drop shell gradient#841
Merged
Conversation
The shell painted a `linear-gradient(180deg, #FFFFFF, #F9F9F9)` plus a radial highlight overlay. Transparent inner regions (thread, right pane) bled through, landing at ~#FCFCFC instead of the spec #FFFFFF. Remove the gradient and the three vars feeding it (--shell-background-base, --shell-background-weak, --shell-highlight-overlay). The `bg-bg-base` Tailwind class on the shell element now paints a flat var(--bg-base) surface, and the Electron BrowserWindow's native backgroundColor (set from --background-base via setBackgroundColor IPC) handles any transparent fallthrough. Both light and dark blocks are cleaned up symmetrically. Shell border / frame / titlebar vars are kept; they are color-mix inputs for OS window chrome, not body dividers. Verified: shell-frame-contract.test.ts (9/9), tsgo -b clean.
Light theme carried a 4-tier ink scale, a 3-tier border scale mixing opaque hex and high alpha, and an unused --surface-list-hover token. Dark already collapsed (PR #834) to 2-tier ink + 1-tier alpha border; light is now symmetric: - ink 4 → 2: --fg-base aliases --fg-strong, --fg-weaker aliases --fg-weak - icon: --icon-weak aliases --icon-base (mirror of collapsed ink) - border 3 → 1: all three tiers resolve to rgba(0, 0, 0, 0.08) (was rgba 0.162 / #ece6df / #f3ede6, an inconsistent mix) - drop --surface-list-hover: no Tailwind callsite consumes it; the row-hover pattern lives on --row-hover-overlay Token names stay as aliases so callsites do not need touching. --surface-interactive-base / --surface-interactive-hover keep their separate values: context-items.tsx uses both to differentiate default / hover / selected states; collapsing them would force callsite invention. Regenerated tailwind/colors.css from script/colors.txt. Closes #840. Verified: theme-parity (199/199), tsgo clean on app + ui.
Contributor
📝 WalkthroughWalkthroughThis PR collapses the light-mode color token system to match dark theme's minimal structure: foreground, border, and icon tokens are consolidated to fewer visual tiers, the ChangesLight Theme Color System Collapse
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a 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
app
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request streamlines the application's theme system by collapsing color tiers for ink, borders, and icons, and removing the surface-list-hover token across multiple configuration files. It also removes the background painting from the desktop shell to prevent visual artifacts in transparent regions. A review comment identifies a typo in a CSS comment where --background-base should be corrected to --bg-base to ensure semantic consistency with the actual theme tokens.
Perf delta summaryComparator: pass
|
Icon color was conditional on disabled state (white when active, fg-weak when disabled). After the light surface collapse merged --icon-weak and --fg-weak to the same #8c817a, the disabled-state arrow rendered the same color as its own background, becoming invisible. Preview source (docs/design/preview/composer.html L264-273) locks the icon at var(--bg-base) unconditionally and uses var(--brand-primary) / var(--fg-strong) for enabled / stopping backgrounds. Drop the conditional icon color and the hardcoded #1A1613 / --button-brand-base literals so the runtime matches the preview spec across all three states.
Owner
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Astro-Han
added a commit
that referenced
this pull request
May 22, 2026
# Conflicts: # packages/opencode/src/session/compaction.ts
13 tasks
Astro-Han
added a commit
that referenced
this pull request
May 22, 2026
…845) ## Summary Restore the 1px hairline between the sidebar column and the thread column, and unify the dark hairline alpha to match light. After the surface collapses in #834 and #841, both themes lost a visible boundary at the sidebar↔thread seam: dark sidebar and canvas resolve to the same `#1A1917`, and light's 6-unit brightness gap reads as "panels glued together" without a hairline. Two changes: 1. Add `border-r border-border-weaker` to the sidebar `<aside>` in `packages/app/src/pages/layout.tsx`. The class swaps via the existing `:root[data-color-scheme="dark"]` rule, no Tailwind `dark:` variant needed. 2. Unify dark `--border-base / --border-weak / --border-weaker` from `rgba(255,255,255,0.06)` to `rgba(255,255,255,0.08)`. Light side stays at `rgba(0,0,0,0.08)`; both themes now resolve to alpha 0.08, mirroring the same intent on both sides. A new `theme-parity` test locks the value in all three theme blocks (`:root`, `:root[data-color-scheme="dark"]`, `@media (prefers-color-scheme: dark)`) so a silent rollback fails CI. ## Why `docs/DESIGN.md` L226/L228 already specifies "internal divisions use 1px `--border-weaker`" and "the 1px hairline between columns is the only divider — no color contrast". The spec was correct; the code never rendered the `border-right` on the sidebar. Direct user report after #841: "侧边会话栏和会话流黏在一起，看不清了" — both themes confirmed in side-by-side preview. On the alpha unification: 0.06 (dark) vs 0.08 (light) was a perceptual-asymmetry argument that does not hold up at this scale — the two values are visually indistinguishable in a real side-by-side. Keeping two numbers when one suffices is unjustified drift, and the bumped dark value carries the divider role better in the one place where the alpha is the *only* boundary signal. ## Related Issue No tracking issue. Reported direct, fixed direct. ## Human Review Status Pending ## Review Focus - `packages/ui/src/styles/theme.css` L300-307 and L445-447 — dark border alpha bump. Affects every dark border in the app (tool cards, popovers, code fences, button ghosts), not just the sidebar divider. Snap diff against current dev shows they read slightly more defined and stay calm. - `packages/ui/src/theme/themes/pawwork.json` L148-150 — dark border alpha bump mirrored in the theme JSON registration path. - `packages/app/src/pages/layout.tsx` L2486 — the only callsite change. Adding `border-r border-border-weaker` to the sidebar `<aside>`; the resize handle and main pane geometry are untouched. - `packages/ui/test/theme-parity.test.ts` — new describe block locks both themes at 0.08 across all three theme blocks. - `packages/app/e2e/snap/sidebar-divider.snap.ts` — new focused snap as a human-eye preview anchor, not a regression gate. The test only writes the PNG and asserts the buffer is non-empty; token-level regressions are caught by `theme-parity`. App-shell covers the full composition; this one zooms in on the 80px window straddling the seam so reviewers can see the hairline. Crop is anchored to the sidebar bounding box (y/height from `box.y` / `box.height`) so the titlebar above the sidebar does not leak into the frame. ## Risk Notes - **Dark border bump is global.** Every dark `border-border-*` callsite reads ~33% stronger (0.06 → 0.08 white alpha). Verified in three snap grids: `app-shell` (full shell stack), `sidebar` (popover and sort menu borders), `session-turn-changes` (tool card and diff card borders). All read cleaner, none read "hard". - **DESIGN.md L194 / L228 numeric prose is now stale.** `docs/DESIGN.md` is local-only in this checkout per AGENTS.md ("`docs/` are local-only in this checkout and excluded via `.git/info/exclude`"), so no in-PR doc edit; the maintainer will sync the prose after merge as part of normal doc upkeep. - **No platform / packaging surface touched.** Pure CSS tokens + one className. Electron native window background path is unchanged. ## How To Verify ```text cd packages/ui && bun test theme-parity — 200 pass / 403 expects (includes new lock) cd packages/app && bun test shell-frame-contract — 9 pass / 95 expects (unchanged) bun --cwd packages/ui run typecheck — clean bun --cwd packages/app run typecheck — clean bun --cwd packages/opencode run typecheck — clean bun run snap app-shell sidebar session-turn-changes sidebar-divider — 4 grids regenerated, divider visible both themes, no surface regressions ``` ## Screenshots or Recordings `docs/design/preview/screenshots/sidebar-divider.png` — focused crop of the new hairline in both themes. `docs/design/preview/screenshots/app-shell.png` — full shell composition, divider visible at the sidebar right edge in both themes. `docs/design/preview/screenshots/session-turn-changes.png` — tool / diff cards in dark with the bumped 0.08 border, still calm. ## Checklist - [x] **Type label** — this PR carries exactly one of `bug`, `enhancement`, `task`, `documentation`. Type labels are author-added; the labeler bot does NOT assign them. Add the label in the GitHub UI, then tick this. - [x] **Routing labels** — this PR carries at least one of `app`, `ui`, `platform`, `harness`, `ci`. The labeler bot assigns these on PR open based on changed paths. Confirm the bot's choice (or override if wrong), then tick this. - [x] **Priority label** — this PR carries exactly one of `P0`, `P1`, `P2`, `P3`. The priority-triage bot suggests one on PR open. Confirm or override, then tick this. - [x] Human Review Status above is set to `Pending`, `Approved by @<reviewer>`, or `Not required: <reason>` (default is `Pending`; "not required" is restricted to bot-authored low-risk PRs). - [x] I linked the related issue, or stated in Summary why there is no issue. - [x] I described the review focus and any meaningful risks. - [x] I replaced the example block in How To Verify with the real verification steps and the key result for each. - [x] I did not introduce unrelated refactors, dependencies, generated files, or file changes beyond the stated scope. - [x] **(conditional)** I manually checked visible UI or copy changes when needed, with screenshots or recordings. Leave unticked only if no visible UI or copy changed. - [ ] **(conditional)** I considered macOS and Windows impact for platform, packaging, updater, signing, paths, shell, or permissions changes. Leave unticked only if no platform/packaging surface was touched. - [x] **(conditional)** I called out docs, release notes, dependencies, permissions, credentials, deletion behavior, generated content, or local file changes when relevant. Leave unticked only if none of those surfaces was touched. - [x] I reviewed the final diff for unrelated changes and suspicious dependency changes. - [x] I am targeting `dev`, and my PR title and commit messages use Conventional Commits in English. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a visible border divider between the sidebar and main content area. * **Style** * Enhanced border visibility in dark mode with improved opacity. * **Tests** * Added snapshot tests for sidebar divider appearance across light and dark themes. * Added tests to ensure border token consistency across theme definitions. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/Astro-Han/pawwork/pull/845?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
](https://app.coderabbit.ai/change-stack/Astro-Han/pawwork/pull/845?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack){kind=link}
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
Three cuts to the light theme surface system. They are bundled because all three fix the same complaint: the light canvas should be a calm flat sheet, and it wasn't.
[data-component="desktop-shell"]painted alinear-gradient(180deg, #FFFFFF, #F9F9F9)plus a radial highlight overlay. Transparent inner regions (thread, right pane) sat on top of it and read as ~#FCFCFC instead of the spec #FFFFFF. Thebg-bg-baseTailwind class on the same element now paints a flatvar(--bg-base)surface, and the Electron BrowserWindow nativebackgroundColor(already set from--background-baseviasetBackgroundColorIPC) handles any transparent fallthrough.--surface-list-hovertoken. Dark already collapsed to 2-tier ink + 1-tier alpha border; light is now symmetric.send-button.tsxset the icon tovar(--fg-weak)when disabled while the background usedvar(--icon-weak). After the collapse merged both tokens to#8c817a, the disabled-state arrow rendered the same color as its own background and went invisible.docs/design/preview/composer.htmlL264-273 locks the icon atvar(--bg-base)unconditionally; runtime now matches that spec across enabled / disabled / stopping states, and the hardcoded#1A1613/--button-brand-baseliterals are replaced with thevar(--fg-strong)/var(--brand-primary)tokens the preview uses.Token names survive as aliases so callsites do not need touching.
--surface-interactive-base/--surface-interactive-hoverkeep their separate values becausecontext-items.tsxuses both to differentiate default / hover / selected states; collapsing them would force callsite invention.Why
A user-supplied screenshot of the production light theme read off-spec: pixel-sampled center thread and right pane were
#FCFCFCinstead of the spec#FFFFFF. Composer and sidebar measured correctly because they paint their own opaque background; the bug was specifically in regions that depended on inheriting the canvas. The shell gradient was the source:linear-gradient(180deg, #FFFFFF, #F9F9F9)at the mid point resolves to ~#FCFCFC, exactly the measured value.Light's token system also drifted from the dark collapse: 4 ink tiers, 3 border tiers mixing opaque #ECE6DF / #F3EDE6 with high-alpha rgba(0,0,0,0.162), and a
--surface-list-hoverthat no Tailwind callsite consumed. Collapsing brings the two themes to the same shape (2 ink, 1 border alpha) and removes one dead token.Related Issue
Closes #840.
Human Review Status
Pending
Review Focus
packages/app/src/index.css— both light and dark shell rules lose the gradient +--shell-background-*+--shell-highlight-overlayvars symmetrically. Border / frame / titlebar vars are kept; they arecolor-mixinputs for OS window chrome, not body dividers, and have a comment explaining the asymmetry with the rest of the surface system.packages/ui/src/styles/theme.css— light block (around L83-160): ink, border, icon collapse. Both dark mirrors (:root[data-color-scheme="dark"]first-paint and the@media (prefers-color-scheme: dark)block) drop--surface-list-hoveronly; everything else dark stays as feat(ui): collapse dark mode to 2-surface palette #834 landed it.packages/ui/src/theme/themes/pawwork.json— overrides mirror the theme.css collapse for both light and dark.packages/ui/script/colors.txtand the regeneratedpackages/ui/src/styles/tailwind/colors.css—--surface-list-hoverremoved from the Tailwind color enumeration; tailwind utility classes for that token no longer exist.Risk Notes
--border-basedrops fromrgba(0,0,0,0.162)torgba(0,0,0,0.08), mirroring dark's airyrgba(255,255,255,0.06)posture. Button / input outlines, tooltip / popover containers, code fences and tool card borders all lighten slightly. The visual reads as cleaner, but it is a perceivable change.--surface-list-hoverdeleted (light and dark both). The earlier grep confirmed no Tailwind callsite consumes it; the dead removal is safe. If any future component reaches forbg-surface-list-hover, the build will fail to find the utility — that is correct; the row hover pattern lives on--row-hover-overlay.docs/DESIGN.mdL184 / L192 / L194 prose still describes light as 4-tier ink + 3-tier border. These are local-only files in this checkout and a doc-sync follow-up will rewrite that section together withdocs/design/preview/colors-neutrals.html(currently stamped 2026-05-03 stale). Doc drift does not block the code change.How To Verify
Snap grids show: light center thread / right pane / composer all flat white; sidebar
#F9F9F9; light tool card and sidebar selected row carry the softer border / overlay; dark side unchanged.Real Electron walk with the macOS Digital Color Meter is the recommended pre-merge check (the gradient bug was originally reported from a real Electron screenshot). Snap is a web Chromium renderer so it does not exercise the Electron native window background path, but that path now intentionally shows through nothing because the shell paints
bg-bg-baseopaquely.Screenshots or Recordings
Checklist
bug,enhancement,task,documentation. Type labels are author-added; the labeler bot does NOT assign them. Add the label in the GitHub UI, then tick this.app,ui,platform,harness,ci. The labeler bot assigns these on PR open based on changed paths. Confirm the bot's choice (or override if wrong), then tick this.P0,P1,P2,P3. The priority-triage bot suggests one on PR open. Confirm or override, then tick this.Pending,Approved by @<reviewer>, orNot required: <reason>(default isPending; "not required" is restricted to bot-authored low-risk PRs).dev, and my PR title and commit messages use Conventional Commits in English.Summary by CodeRabbit