Skip to content

fix(ui): restore sidebar/thread divider, unify border alpha to 0.08#845

Merged
Astro-Han merged 2 commits into
devfrom
claude/sidebar-divider-unify
May 22, 2026
Merged

fix(ui): restore sidebar/thread divider, unify border alpha to 0.08#845
Astro-Han merged 2 commits into
devfrom
claude/sidebar-divider-unify

Conversation

@Astro-Han

@Astro-Han Astro-Han commented May 22, 2026

Copy link
Copy Markdown
Owner

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

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

  • 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.
  • 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.
  • 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.
  • 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).
  • I linked the related issue, or stated in Summary why there is no issue.
  • I described the review focus and any meaningful risks.
  • I replaced the example block in How To Verify with the real verification steps and the key result for each.
  • I did not introduce unrelated refactors, dependencies, generated files, or file changes beyond the stated scope.
  • (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.
  • (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.
  • I reviewed the final diff for unrelated changes and suspicious dependency changes.
  • I am targeting dev, and my PR title and commit messages use Conventional Commits in English.

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 Change Stack

DESIGN.md L226/L228 calls for a 1px --border-weaker hairline between
the three shell columns, but layout.tsx L2483 had no border-right on
<aside>. After the light/dark surface collapse (#834, #841) that
divider became the only signal carrying the boundary: dark sidebar and
canvas both resolve to #1A1917 (zero color delta), 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>.
2. Unify dark --border-base / --border-weak / --border-weaker from
   rgba(255,255,255,0.06) to rgba(255,255,255,0.08), matching the
   light side at 0.08 black. The 0.06 vs 0.08 drift was unjustified
   at this scale (visually indistinguishable in side-by-side preview)
   and the bumped value lets the dark hairline carry the boundary
   without becoming the only signal that's too faint to see.

The token bump is global to dark and affects every dark border: tool
cards, popovers, code fences, button outlines. Snap diff shows they
read slightly more defined, still calm.

Verify:
  cd packages/ui && bun test theme-parity  # 200 pass / 403 expects
  cd packages/app && bun test shell-frame-contract  # 9 pass / 95 expects
  bun --cwd packages/{ui,app,opencode} run typecheck  # clean
  bun run snap app-shell sidebar session-turn-changes sidebar-divider
@coderabbitai

coderabbitai Bot commented May 22, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

This PR increases the opacity of dark-mode border tokens (--border-base, --border-weak, --border-weaker) from alpha 0.06 to 0.08, applies the updated weaker border style to the sidebar divider, adds theme parity assertions to ensure consistency across CSS and media-query paths, and introduces a Playwright snapshot test to visually verify the rendered sidebar boundary in both light and dark themes.

Changes

Sidebar Border Styling

Layer / File(s) Summary
Dark-mode border token opacity increase
packages/ui/src/styles/theme.css, packages/ui/src/theme/themes/pawwork.json, packages/ui/test/theme-parity.test.ts
Dark border tokens --border-base, --border-weak, and --border-weaker are updated from rgba(255, 255, 255, 0.06) to rgba(255, 255, 255, 0.08) in both the :root[data-color-scheme="dark"] selector and the @media (prefers-color-scheme: dark) mirror in CSS, and in the pawwork theme JSON overrides. A new test suite asserts all three tokens normalize to 0.08 alpha consistently across light, dark attribute, and dark media blocks.
Sidebar border rendering and snapshot
packages/app/src/pages/layout.tsx, packages/app/e2e/snap/sidebar-divider.snap.ts
The sidebar <aside> element adds the border-r border-border-weaker class to render the right-hand divider using the updated token. A Playwright snapshot test captures the sidebar↔thread column boundary seam in both light and dark themes, validates both images are non-empty, and composes them into a grid snapshot for visual verification.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • Astro-Han/pawwork#841: Both PRs modify the same border hairline tokens (--border-base, --border-weak, --border-weaker) in packages/ui/src/styles/theme.css and packages/ui/src/theme/themes/pawwork.json.
  • Astro-Han/pawwork#834: Both PRs update dark-mode border "hairline" tokens via the same theme files, affecting the sidebar divider styling verified by the new snapshot test.
  • Astro-Han/pawwork#565: Updates other border tokens in the shared token system and applies border styling in the layout, creating related visual changes through the same theming mechanism.

Poem

🐰 A border so fine, now slightly more bright,
Dark shadows refined to just the right light,
The sidebar stands proud with its subtle dividing seam,
Pixels aligned in snapshot dreams! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main changes: restoring a sidebar/thread divider and unifying border alpha values across themes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The pull request description is comprehensive and follows the template structure with all required sections completed.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/sidebar-divider-unify

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added app Application behavior and product flows ui Design system and user interface P2 Medium priority labels May 22, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested priority: P2 (includes user-path files (packages/app/src/pages/layout.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.

@Astro-Han Astro-Han added the bug Something isn't working label May 22, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a visible boundary between the sidebar and the main thread by adding a right border to the sidebar layout and increasing the alpha transparency of border tokens in dark mode from 0.06 to 0.08. It also adds a new E2E snapshot test and a theme parity test to ensure visual consistency. Feedback on the E2E test suggests refining the screenshot clipping logic to use the sidebar's vertical offset and adopting stricter fallback values for viewport dimensions to prevent false positives.

Comment thread packages/app/e2e/snap/sidebar-divider.snap.ts Outdated
@github-actions

github-actions Bot commented May 22, 2026

Copy link
Copy Markdown

Perf delta summary

Comparator: pass

Profile / Scenario interaction median interaction worst long task max tbt frame gap p95 frame gap max jank count cls status
default / session-streaming-long 40 -> 48 (+8) 64 -> 64 (0) 0 -> 0 (0) 0 -> 0 (0) 16.8 -> 16.8 (0) 33.4 -> 33.4 (0) 0 -> 0 (0) 0 -> 0 (0) pass

@Astro-Han

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented May 22, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

… preview anchor

Crop y/height were taken from y:0 + viewport height, which sliced the titlebar
above the sidebar into the frame instead of the sidebar↔thread seam. Anchor
both to the sidebar bounding box so the crop sits inside the sidebar's vertical
extent.

Also reframe the comment: this snap is a human-eye preview anchor, not a
regression gate. Token regressions are locked by theme-parity; the snap only
asserts the PNG buffer is non-empty.
@Astro-Han Astro-Han merged commit 962e07b into dev May 22, 2026
28 checks passed
@Astro-Han Astro-Han deleted the claude/sidebar-divider-unify branch May 22, 2026 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

app Application behavior and product flows bug Something isn't working P2 Medium priority ui Design system and user interface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant