Skip to content

feat(fork): restyle Sidebar V2 cards to the Figma card-v2 layout - #36

Merged
NoahHendrickson merged 4 commits into
customfrom
fork/sidebar-v2-card-layout
Jul 30, 2026
Merged

feat(fork): restyle Sidebar V2 cards to the Figma card-v2 layout#36
NoahHendrickson merged 4 commits into
customfrom
fork/sidebar-v2-card-layout

Conversation

@NoahHendrickson

@NoahHendrickson NoahHendrickson commented Jul 29, 2026

Copy link
Copy Markdown
Owner

Problem

The Sidebar V2 thread cards didn't match the Figma card-v2 design (t3-fork, node 113-3999). The headline change — the status indicator leading the card's title line instead of trailing it — landed in the first commit, but the metrics around it drifted: cards were 2px too tall, sat 5px apart instead of 2, groups sat 19px apart instead of 16, titles were medium-weight where the design draws Regular, the wrong statuses receded (Working instead of Done), and the fork-added trailing controls (group-header plus, card check/snooze, shelf chevrons) sat off the vertical axis the chrome icons and runtime glyphs share.

Fix

  • Card geometry: 16px rows in a 54px card (px-1 py-2, gap-1.5), 2px between cards via the list ul's gap (rows carry no padding), 16px between groups, 16px of headroom above the list. contain-intrinsic-size hints follow (54/75). The row gap and the 8px title gap are a deliberate live retune of Figma's literal 4px: the prompt sits on the header label's 36px axis, with the repo/meta indent (pl-6) following. The working rain keeps its 18px grid by overflowing 1px into the card padding rather than growing the row.
  • Card content: titles are Regular for every status; Done + Idle recede at rest (hover restores), Working stays foreground per the component set; the model label is caption 11/15 on whichever line it lands.
  • Group headers: open folder glyph, foreground label, 2px to the first card.
  • Trailing axis: the offset table in sidebarV2TrailingColumn.ts was derived from a false premise — that the chrome rows and list rows end at the same edge (they're 4px apart). Every offset is now re-derived from the real insets against one axis 24px in from the panel edge, which lines up the header plus, card actions, slim-row actions, and shelf chevrons with the chrome buttons and runtime glyphs.
  • Chrome rows: control rows at 28px with a left-aligned scope label.
  • Also repairs a stale row-count helper in SidebarV2ThreadCardMeta.test.tsx that was failing 5/6 on the first commit, and updates the guard tests and .fork/customizations.yaml to pin the new metrics.

Verified live against a seeded worktree dev environment by the maintainer, iterating on alignment with screenshots; 195 fork guard/custom tests pass and typecheck is clean.

Work by Claude Fable 5 via Claude Code, with maintainer-driven review in the live app.

Review updates

a636e5c lands the maintainer review: the panel --muted-foreground lift is gone (chrome keeps its --sidebar-muted-foreground brightness; receded card titles render again, guard pins the exact hex and the absence of the override), cards read one Done/Idle recede rule for surface and title while slim shelves keep upstream's, row-action focus rings clear the row clip (ring-offset-1), the stale chrome-column comments are rewritten, the 24px always-on floor is documented as the design-wide stance, and the deleted meta rationale is restored. Nits: redundant conjunct dropped, elapsed fade gated on actions existing, meta rows counted by testid, slim intrinsic-size 36px, offset table documents its empty entries. 8949bd5 deduplicates the card-height number into the card comment and gives the gap retune its reason.

🤖 Generated with Claude Code

NoahHendrickson and others added 2 commits July 29, 2026 15:11
Align chrome, group headers, and thread cards to the Figma card-v2
metrics so status, folder, and search icons share one left axis.

Co-authored-by: Cursor <cursoragent@cursor.com>
Cards now measure exactly what the design draws: 16px rows in a 52px
card, 2px between cards, 16px between groups, 16px of list headroom.
Titles are regular weight with Done and Idle receding instead of
Working; group headers use an open folder with a foreground label; the
model label is caption-sized on either line. The trailing column's
offsets are re-derived from the real row edges — chrome ends 12px in
where the list ends 8 — which puts the header plus, card actions, slim
actions and shelf chevrons on the same axis as the chrome icons and
runtime glyphs. Chrome control rows sit at 28px with a left-aligned
scope label. Also repairs the stale row-count helper in
SidebarV2ThreadCardMeta.test.tsx.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@cursor

cursor Bot commented Jul 29, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XL labels Jul 29, 2026

@cursor cursor 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.

Thermo-nuclear code quality review — not approving.

This PR does real structural good: moving status out of the trailing actions cell deletes the opacity/stacking hit-path bug, and the trailing-axis re-derivation is the right kind of fix. That is not enough for the approval bar.

The remaining problems are about ownership of the layout model. The whole PR is geometry, and the geometry is still a comment protocol scattered across SidebarV2.tsx, meta, status, and the trailing-offset table — already contradicting itself (52 vs 54). Until that model is one module the call sites consume, this will keep drifting the next time someone tunes a gap.

Ordered by priority:

  1. Card geometry has no owner; intentional Figma drift is unexplained
  2. Leading status dispatcher left in the 3k-line megacomponent
  3. Empty-string trailing offsets are identity slots pinned by guards

No approval until the geometry is a single derived model (and preferably the status mark composition leaves SidebarV2.tsx).

Open in Web View Automation 

Sent by Cursor Automation: Thermo-nuclear PR review

Comment thread apps/web/src/components/SidebarV2.tsx
Comment thread apps/web/src/components/SidebarV2.tsx
Comment thread apps/web/src/custom/SidebarV2StatusIndicator.tsx Outdated
Comment thread apps/web/src/custom/sidebarV2TrailingColumn.ts
Comment thread apps/web/src/custom/SidebarV2ThreadCardMeta.tsx

Copy link
Copy Markdown
Owner Author

Review — aggressive pass

The geometry work is genuinely good. I re-derived the whole trailing axis independently and it holds: chrome pe-3 + half a 24px button = 24; a card reaches the same inset via list pad 8 + px-1; the header and the slim right-0 overlay sit on the bare 8px edge and spend me-1; px-2.5 + half a 12px chevron = 24. Leading edges line up too — status glyph at 12 / title at 36 / group label at 36 (24px folder box + gap-1) / meta pl-6 at 36. Calling the old derivation false rather than patching around it was the right call, and contain-intrinsic-size 54/75 checks out against 8 + 16 + 6 + 16 + 8 and + 6 + 15.

So the layout is right. What follows is everything else.


1. The palette change is undisclosed, wider than its own rationale, and it eats the feature this PR ships

theme.custom.css:133-134 lifts both --muted-foreground and --sidebar-muted-foreground to #e0e0e0 inside [data-sidebar-version="v2"]. This is not mentioned anywhere in the PR body, which is titled "card metrics".

The stated reason is "Search / All projects / project headers". Every one of those reads --sidebar-muted-foreground (CHROME_CONTROL, CHROME_ROW_ICON_TINT, the header's /80 folder). None of them reads --muted-foreground. The sidebar-scoped token alone buys the entire stated benefit. The other line is pure collateral — and it lands squarely on the card:

  • sidebarV2RowPolicy.ts now says, in this PR, "colour alone carries the receded/forward distinction."
  • The receded title is text-muted-foreground; the forward one is text-foreground (#f1f3f7).
  • Before: #a3a3a3 vs #f1f3f72.27:1.
  • After: #e0e0e0 vs #f1f3f71.19:1.

One line of this PR asserts colour is now the sole carrier of the signal; another line in the same PR reduces that signal to ~1.19:1, which is not visible. Done and Idle titles are, in practice, no longer receded at all. Same compression hits the meta lines: text-muted-foreground/70 over #1e1e1e goes from ≈#7b7b7b to ≈#a6a6a6, so title→branch hierarchy flattens too.

The guard added at forkSurfacePalette.test.ts:174 pins the wrong invariant:

expect(relativeLuminance(muted)).toBeGreaterThan(relativeLuminance("#a3a3a3"));

Open-ended in the direction that causes the damage — #f1f3f7 passes this test. Every other assertion in that file pins an exact hex (toContain("--sidebar-control-surface: #303030")). This one should too, and it should additionally floor the distance to --foreground, since that gap is the feature.

Ask: drop the --muted-foreground line, keep --sidebar-muted-foreground, and re-check what actually looked soft. If both are genuinely needed, it belongs in its own PR with its own before/after, not folded into a metrics change.

2. Two contradictory recede policies now live in one component

threadCardTitleRecedes was inverted to Done/Idle. shouldRecede at SidebarV2.tsx:520 was not touched — it is still (status === "ready" || status === "working"), and it is still what threadRowSurfaceClassName({ recedes: shouldRecede }) consumes at line 763.

Net result per card: a Working row's surface recedes (text-sidebar-muted-foreground/75 inherited) while its title is forced forward; a Done row does the exact opposite. Anything in the card that doesn't set its own colour follows the surface, and the hover branch differs between the two.

Worse, the comment at SidebarV2.tsx:514-519 still reads:

"Working and read-ready rows recede (inbox-zero: a running agent isn't your problem yet); approval and input no longer do..."

That is a verbatim statement of the policy this PR reverses, sitting directly above the variable that still implements it. Either shouldRecede follows the component set too, or the comment has to say out loud that surface and title deliberately diverge and why.

3. Focus-ring clearance on the settle/snooze buttons went from 4px to 0

SIDEBAR_V2_ICON_BUTTON_CLASS is focus-visible:ring-2 + ring-offset-2 — 4px of box-shadow outside the button. threadRowSurfaceClassName puts overflow-hidden on the row, so the clip boundary is the row's border box.

  • Before: card px-3 (12px) with cardActions: "-me-1" → button's right edge 8px inside the row → ring reaches 4px inside. Fine.
  • After: card px-1 (4px) with cardActions: "" → button's right edge 4px inside the row → the ring's outer edge lands exactly on the clip boundary.

focus-within:overflow-visible on the wrapper (added precisely so this ring survives) does nothing about the row's own overflow-hidden two levels up. Zero tolerance, and rounded-md on the row makes the corners worse. Please tab to Settle on a card and confirm the ring is intact; if it isn't, the cheapest fix is overflow-clip-margin on the row or dropping ring-offset-2 to ring-offset-1 here.

4. Chrome trailing buttons shrank 32px → 24px on fine pointers

SidebarV2ChromeRows.tsx:86, size-8size-6. TOUCH_TARGET is pointer-fine:hidden, so mouse users get 24px flat.

sidebarV2TrailingColumn.ts argues the opposite case, in this repo, about this exact size:

"24px is exactly the WCAG 2.5.8 floor, with no margin, and a shared box whose whole argument is 'these were too small to hit' cannot ship the one always-on control at it."

These chrome buttons are always-on — not hover-gated like the row actions. If Figma says 24, fine, but this PR should either say why the module's own rule doesn't apply here or amend the rule. Silently landing on the far side of an argument the codebase already wrote down is the thing that makes it un-relitigable later.

5. The false premise this PR fixes survives verbatim, five lines from the corrected value

sidebarV2TrailingColumn.ts now says the old derivation was "4px wrong at the source." SidebarV2ChromeRows.tsx:64-76, unmodified, still says:

"...a 16px icon centred in a 32px button... Squaring the icons costs the button 4px of its own right edge... so both columns end at the same place and only the 4px is left."

The button is 24px now, chromeRow is "", and "both columns end at the same place" is the exact claim the PR calls wrong. The stale copy sits immediately above const CONTROL_ROW, which consumes the corrected constant. Whoever reads this file next will believe it.

6. Cards drop unread emphasis; slim rows keep it

isDone resolves to unread. So a card with an outcome you have never seen now dims identically to a fully-handled idle thread — only the dot colour separates them. Meanwhile the slim branch at SidebarV2.tsx:798-800 still does isUnread ? "text-muted-foreground" : "text-muted-foreground/70", so the two variants of the same list now encode unread-ness oppositely. Compounded by finding 1, where the dimming is invisible anyway. If the component set really wants Done receded, slim should follow — or the divergence needs a sentence.

7. ~35 lines of load-bearing rationale deleted with no code change

SidebarV2ThreadCardMeta.tsx loses the whole worktree-mark-replaces-branch-mark argument (including why the mark gates on hasWorktree and not branch), the dev-server-pulse note, and the "semantic tokens, not emerald/red literals — a 400-weight green on white would not stay legible" note. The corresponding code is unchanged. That's exactly the class of comment CLAUDE.md asks for and exactly the class of note whose absence causes someone to "simplify" the predicate or swap the tokens back. Restore them; they cost nothing in a layout PR.


Nits

  • SidebarV2.tsx:787topStatus?.tone === "done" && topStatus.mark === "dot" — the second conjunct can never be false; tone: "done" is produced by exactly one branch, and it's the dot branch. Either drop it or say it's guarding a future woke-done.
  • SidebarV2.tsx:1054-1071 — the gate is settlementSupported || showSnoozeButton || status === "working", but the elapsed span keeps an unconditional group-hover/v2-row:opacity-0. On a working row where neither action is offered, hovering blanks the timer and puts nothing in its place. Gate the fade on the same condition that renders the actions.
  • SidebarV2ThreadCardMeta.test.tsx:225split(a).length + split(b).length - 2 is a clever substring count standing in for a row count, and the comment claims robustness it doesn't have (any cn()/tailwind-merge reordering of REPO_ROW silently zeroes the second term and the test still "passes" with the wrong number). A data-testid on the row divs would be shorter and actually load-bearing.
  • SidebarV2.tsx:831 (pre-existing, but adjacent) — the slim li declares contain-intrinsic-size:auto_34px for an h-9 (36px) row. While you're pinning the card at exactly 54/75, its neighbour lies by 2px on every skipped row.
  • Three of five entries in SIDEBAR_V2_TRAILING_OFFSET are now "", and the guard asserts empty strings. That's fine and arguably the point (the module is now the derivation, not the correction) — but it's worth a line at the top saying so, or the next reader deletes the "empty" entries.
  • Commit e2fe860 ships SidebarV2ThreadCardMeta.test.tsx failing 5/6 and e5d243a repairs it. Squash on merge, or custom gets a red bisect point.

Bottom line: the metrics and the axis re-derivation are solid and I'd take them as-is. Findings 1 and 2 are blocking — between them, the receded-title behaviour this PR re-specifies doesn't actually render, and the component holds two opposite definitions of "receded". 3 needs a browser check. 5 and 7 are documentation debt the PR created and should pay before merge.


Generated by Claude Code

NoahHendrickson and others added 2 commits July 29, 2026 16:50
The palette line that lifted --muted-foreground to #e0e0e0 is gone —
only the sidebar-scoped chrome token is lifted now, so receded card
titles and the title→branch hierarchy render again; the guard pins the
exact hex and the absence of the panel override. Cards read one recede
rule for surface and title (Done/Idle, via cardRecedes) while the slim
shelves keep upstream's, and both the code comment and manifest say so.
Row-action focus rings drop to ring-offset-1 so they clear the row's
overflow-hidden instead of landing exactly on it. The stale chrome
comments (32px buttons, "both columns end at the same place") are
rewritten to the corrected axis model, the 24px always-on floor is
documented as the design-wide stance, and the meta component gets back
the worktree/dev-pulse/semantic-token rationale that was deleted with
no code change. Nits: redundant mark conjunct dropped, the elapsed
timer only fades when hover actions will replace it, meta rows carry a
testid the row-count test uses instead of class sniffing, slim rows'
intrinsic-size hint matches their real 36px, and the offset table says
its empty entries are load-bearing.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The rain module's comment restated the card's drawn height and went
stale when the row gap was retuned (52 → 54); the card comment in
SidebarV2 owns the number now and the rain note just points there. The
gap-1.5 divergence from Figma's 4px carries its reason: with the prompt
moved to the header label's 36px axis, the literal 4px read cramped
between two 16px text rows.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@NoahHendrickson

Copy link
Copy Markdown
Owner Author

All findings landed in a636e5c + 8949bd5. Point by point:

1 (blocking) — palette: You're right on every count, including the guard critique. The --muted-foreground: #e0e0e0 line is gone from the panel block; --sidebar-muted-foreground keeps the lift, which covers every stated beneficiary (all chrome consumers read the sidebar token). Receded titles are back to upstream's #a3a3a3 vs #f1f3f7, and the meta hierarchy returns with them. The guard now pins #e0e0e0 exact (matching the file's other assertions) and asserts the panel block does not declare --muted-foreground at all — the absence is the invariant, and no brighter-than floor survives for #f1f3f7 to sneak through.

2 (blocking) — two recede policies: Cards now compute one cardRecedes (Done/Idle, from the same topStatus the mark renders from) and feed it to both threadRowSurfaceClassName and the title; shouldRecede survives for the slim shelves only. The stale comment above it now says exactly that, and the rowPolicy "two recede rules" note names where each applies.

3 — focus ring: Confirmed by derivation: button edge 4px inside the row's overflow-hidden (flush with card px-1; me-1 on bare-edge rows), and ring-2 + offset-2 is exactly 4px of shadow — outer edge on the clip boundary. Dropped to ring-offset-1 on SIDEBAR_V2_ICON_BUTTON_CLASS (1px clearance, derivation in the comment); the chrome buttons are unclipped and keep offset-2. overflow-clip-margin doesn't apply — it requires overflow: clip, and the row uses hidden.

4 — 24px chrome buttons: Amended the rule rather than silently contradicting it: the module's 2.5.8 note now says the 24px floor is the design-wide stance (Figma draws every icon-only control at it, always-on chrome included) with margin coming from the coarse-pointer child, and TRAILING_BUTTON carries a note pointing at the same stance.

5 — stale premise: The CONTROL_ROW comment is rewritten: these rows define the axis, the list rows explicitly do not share their right edge, and the "both columns end at the same place" claim is called out as the 4px-wrong premise. The 32px-button arithmetic is gone.

6 — unread emphasis: Kept as deliberate divergence, now written down in the rowPolicy note and the manifest: the card delegates unread to the Done dot per the component set; slim rows keep upstream's brightness encoding. With finding 1 fixed the card's dimming is visible again, so the dot + dim combination reads.

Nits: redundant mark === "dot" conjunct dropped (with a comment saying why it could never fail); elapsed fade now gated on settlementSupported || showSnoozeButton so a working row with no actions keeps its timer on hover; meta rows carry data-testid="sidebar-v2-card-line" and the test counts that instead of class-sniffing; slim li hint corrected to its real 36px; the offset table preamble says the empty entries are load-bearing derivations. On the red bisect point: agreed — squash-merge this PR.

Also from the Cursor threads: the rain module's stale "52px" claim is deduplicated into the card comment (single owner for the number), the PR body now states the retuned 54/75, and the gap-1.5 divergence from Figma carries its reason at the divergence. Cursor's geometry-module / component-extraction refactors are declined for this PR per change-scope, with reasons on each thread — reasonable follow-ups if you want them.

195 guard/custom tests pass, typecheck clean, CI green on a636e5c; 8949bd5 is comment-only and re-running now.

🤖 Generated with Claude Code

@NoahHendrickson
NoahHendrickson merged commit 7b128cd into custom Jul 30, 2026
10 checks passed
@NoahHendrickson
NoahHendrickson deleted the fork/sidebar-v2-card-layout branch July 30, 2026 02:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XL vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant