feat(web): restyle sidebar chrome and monochrome accents - #43
Conversation
Split Search, New thread, and Add project into labeled rows, put project scope behind a filter menu, drop the dark blue primary/ring for white, and align the shared topbar to 52px. Co-authored-by: Cursor <cursoragent@cursor.com>
|
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
Bot
added
vouch:trusted
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Thermo-nuclear code quality
The call-site cleanup is real: Add project moves into the action stack, ProjectScopeRow drops scopedProjectGroup / onAddProject, and the filter affordance is clearer. Theme + topbar changes are small and fenced.
What's blocking is the shape of the new chrome module:
- Code judo missed in
SidebarV2ChromeRows—NewThreadRowandAddProjectRoware copy-paste. Collapse to one private labeled-action helper; leave Search as the special case. - Guards freeze the wrong API — three row function names are string-locked even though only
ChromeActionRows+ProjectScopeRoware imported. Guard composition / labels / testids, not internal exports that should disappear. - Projects label re-spells
CHROME_CONTROLtypography — pull the 14px type into one token so the static header can't drift from the action rows.
No 1k-line regression here (SidebarV2 was already huge; this PR shrinks the call site). Fix the duplication and the guard surface before this is maintainable enough to land.
Sent by Cursor Automation: Thermo-nuclear PR review
| export function SidebarV2NewThreadRow(props: { | ||
| readonly newThreadShortcutLabel: string | null; | ||
| readonly newThreadDisabled: boolean; | ||
| readonly onNewThread: () => void; | ||
| }) { | ||
| return ( | ||
| <div className={CONTROL_ROW}> | ||
| <SidebarMenuButton | ||
| size="sm" | ||
| type="button" | ||
| className={cn("min-w-0 flex-1", CHROME_CONTROL, CHROME_ROW_ICON_TINT)} | ||
| onClick={props.onNewThread} | ||
| disabled={props.newThreadDisabled} | ||
| aria-label={ | ||
| props.newThreadShortcutLabel | ||
| ? `New thread (${props.newThreadShortcutLabel})` | ||
| : "New thread" | ||
| } | ||
| data-testid="sidebar-v2-new-thread" | ||
| > | ||
| <PlusCircleIcon className="size-4 shrink-0 text-sidebar-muted-foreground/80" /> | ||
| <span className="min-w-0 flex-1 truncate text-left">New thread</span> | ||
| </SidebarMenuButton> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| export function SidebarV2AddProjectRow(props: { readonly onAddProject: () => void }) { | ||
| return ( | ||
| <div className={CONTROL_ROW}> | ||
| <SidebarMenuButton | ||
| size="sm" | ||
| type="button" | ||
| className={cn("min-w-0 flex-1", CHROME_CONTROL, CHROME_ROW_ICON_TINT)} | ||
| onClick={props.onAddProject} | ||
| aria-label="Add project" | ||
| data-testid="sidebar-v2-add-project" | ||
| > | ||
| <FolderPlusIcon className="size-4 shrink-0 text-sidebar-muted-foreground/80" /> | ||
| <span className="min-w-0 flex-1 truncate text-left">Add project</span> | ||
| </SidebarMenuButton> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
i think there's a code-judo move here that makes this much simpler.
SidebarV2NewThreadRow and SidebarV2AddProjectRow are the same CONTROL_ROW + SidebarMenuButton + icon + label block with different props. Search earns its own shape (CommandDialogTrigger + Kbd); these two do not.
Collapse them into one private labeled-action helper (icon, label, aria-label, testid, onClick, disabled?) and call it twice from SidebarV2ChromeActionRows. Keep Search special. Don't mint two near-identical exported components for a presentation-only fork file.
| expect(rows).toContain("function SidebarV2SearchRow"); | ||
| expect(rows).toContain("function SidebarV2NewThreadRow"); | ||
| expect(rows).toContain("function SidebarV2AddProjectRow"); |
There was a problem hiding this comment.
this abstraction seems unnecessary — and the guard is cementing it.
Nothing outside this module imports SidebarV2SearchRow / NewThreadRow / AddProjectRow. The public seam is already SidebarV2ChromeActionRows + SidebarV2ProjectScopeRow. Locking the three internal function names as strings fights collapsing New thread / Add project into a helper.
Guard the composition instead: ActionRows call site, visible labels / testids, and the Projects filter affordance. Don't freeze an export surface nobody consumes.
| // pt-2 is the 8px gap between the action block and this header. | ||
| <SidebarGroup className="px-3 pt-2 pb-2"> | ||
| <div className={CONTROL_ROW}> | ||
| <span className="min-w-0 flex-1 truncate text-left text-[0.875rem] leading-4 font-normal text-sidebar-muted-foreground"> |
There was a problem hiding this comment.
this looks like a bespoke spelling of something CHROME_CONTROL already owns.
text-[0.875rem] leading-4 font-normal text-sidebar-muted-foreground is re-declared on the static Projects label while action rows get the same type via CHROME_CONTROL. Extract a shared type token (or reuse the control's type classes without the interactive chrome) so a 14px retune can't drift the header off the action rows.
ReviewStatic review only — this environment has no The 52px half of this PR is right, and I'll defend it below. The sidebar restructure is mostly good. Two things I'd hold the merge for: a filter that no longer shows it's on, and a global token change riding in a sidebar PR. 1. Blocking — an active project scope is now invisible
So once a user picks a project:
The single surviving cue is That's the hidden-mode bug CLAUDE.md's "a lying spinner, and a stale label" and "if you added a way in, add the way out and the way to see it" rules are aimed at. What softens it: scope is Cheapest fixes, any one of which closes it:
I'd take the first — it's a glyph state, no layout change, and it's the thing the eye is already on. 2. High — a global palette change is shipped inside a sidebar-chrome PR
Two consequences I'd want screenshots for before this lands: (a) (b) Per CLAUDE.md's one-concern rule this is arguably its own PR. If it stays here, the body and test plan need to say "the fork's dark accent goes monochrome app-wide," not "switches and focus underlines." Also: light mode keeps upstream's blue, which the manifest already declares intentional ( 3. Medium
4. Low
Verified — these hold upChecked rather than assumed:
Summary: fix (1) before merge — a filter with no on-state is a bug, not a style. Decide on (2): either split it out or re-scope the PR description and test plan to match what it actually changes. The rest is polish. Generated by Claude Code |
Show when a project scope is active on the funnel, collapse New thread / Add project into one labeled-action helper, share CHROME_TYPE with the Projects label, and restore the filter tooltip. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Addressed review feedback on
|
1 participant
Summary
--primary/--ringwhite app-wide (not sidebar-only): focus rings, switches, primary buttons/badges, and related accents go monochrome. Light mode keeps upstream blue.Test plan
vp test run apps/web/src/__fork_guards__/forkSidebarChrome.test.ts apps/web/src/__fork_guards__/forkSurfacePalette.test.ts apps/web/src/__fork_guards__/forkWorkspaceHeader.test.ts apps/web/src/__fork_guards__/sidebarV2ProjectGrouping.test.ts apps/web/src/__fork_guards__/sidebarV2RowActionHitArea.test.ts apps/web/src/__fork_guards__/phosphorIcons.test.ts --dir apps/webModel: Cursor Grok 4.5 · Cursor