feat(web): redesign chat composer to the new Figma layout - #41
Conversation
The composer now matches the updated t3-fork Figma (node 71-4121) plus Noey's live-review adjustments: filled 24px context chips with the Auto chip's 4px-radius container, a 40px prompt surface with body/md type and a full muted-foreground placeholder, a 20px control row with per-mode accent chips (auto green, full access yellow, auto-accept edits purple, supervised red), text-only mode toggles, square 4px hover fills, a centred context-window ring, and the send button pinned top-right as the prompt wraps. The old tall/slim density machinery and wrap latch are deleted; the effort trigger drops upstream's dot separator. Co-Authored-By: Claude Fable 5 <noreply@anthropic.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 — request changes
Net deletion is good, and killing the wrap latch / tall-slim machinery is real simplification. That does not clear the bar: this still deepens last-resort fenced edits in a 3.3k upstream file, leaves a density concept that no longer earns its keep, and pushes more attribute/exclusion-chain CSS into an already-over-1k theme file.
Must address before approve
- Stop growing
fork-composer-shellfences inChatComposer— extract acustom/shell (or accept full ownership via override) so layout composition lives outside the hot upstream file. - Delete the density attribute/
"base"leftover entirely; keep collapsed as a boolean/mobile concern only. - Replace mode-chip / control-geometry styling that grows via
:not()exclusion chains with component-owned styles or a typed mode→token map. - Scope the TraitsPicker label join so Settings does not silently inherit a composer-only design call.
- Drop or justify
ComposerContextRow; fix the intent (20px) vs CSS (24px) drift.
Also: stale comment ~2678–2680 still claims model/effort pills live inside the box beside send — contradicts the new layout. Delete it. ComposerControlRowProps.right JSDoc still describes worktree/branch.
Net-negative lines are not the same as structural cleanup. The code-judo move is still available: one shell module in custom/, almost no fences left in ChatComposer.
Sent by Cursor Automation: Thermo-nuclear PR review
| data-chat-composer-form="true" | ||
| > | ||
| {/* fork:begin fork-composer-shell — see .fork/customizations.yaml#fork-composer-shell */} | ||
| {contextStrip ? <ComposerContextRow>{contextStrip}</ComposerContextRow> : null} |
There was a problem hiding this comment.
this is still last-resort fencing inside a 3.3k hot file — and this PR adds more of it. .fork/AGENTS.md preference order is custom/** → override → theme → fences. the code-judo move: extract a custom/ComposerShell (context slot + surface slot + control row) and leave ChatComposer with one small fence that renders that shell. do not keep sprinkling fork-composer-shell through imports, FooterModeControls, density, placeholder, and layout. shadowing the whole composer is only viable if you intend to own it; extracting the shell is cleaner than either more fences or a full 3.3k override.
| isNarrowViewport: isMobileViewport, | ||
| }); | ||
| const isComposerSlim = composerDensity === "slim"; | ||
| const composerDensity = isComposerCollapsedMobile ? "collapsed" : "base"; |
There was a problem hiding this comment.
density is dead as a model and should be deleted, not renamed. "base" is never styled; only "collapsed" changes radius. this PR killed tall/slim/latch correctly, then preserved a vestigial attr + ternary + guard pin for a concept that no longer exists. use a boolean (data-fork-composer-collapsed) or style the existing mobile collapsed path directly — no density enum, no "base" string, no guard asserting the leftover.
| :is(button, [data-slot="button"]):not([data-fork-composer-action]) { | ||
| height: 24px; | ||
| min-height: 24px; | ||
| :is(button, [data-slot="button"]):not([data-fork-composer-action]):not( |
There was a problem hiding this comment.
this exclusion chain is the wrong growth model. every new control type becomes another :not(...) on a shared geometry selector, and the dark color rule already drifts (status excluded from geometry but not from the dark label color). prefer component-owned classes / CSS variables on the controls themselves so the shared rule does not need to know every exception by name.
| // whole restyle and its value picks the mode's accent color; | ||
| // the popup keeps its icons and descriptions. | ||
| <ComposerSelectControl | ||
| data-fork-composer-mode-chip={props.runtimeMode} |
There was a problem hiding this comment.
putting the whole accent restyle on data-fork-composer-mode-chip={runtimeMode} forces ~80 lines of near-duplicate CSS mode blocks and the exclusion-chain tax above. better: a tiny custom mode-chip wrapper (or a typed mode→token map applied as classes/vars) that owns height, radius, wash, and hover. attribute soup in theme.custom.css is not a design system.
| /* fork:begin fork-composer-shell — see .fork/customizations.yaml#fork-composer-shell */ | ||
| /* A plain space, not upstream's " · ": the composer's effort chip reads | ||
| "High 1M". Settings uses the same trigger and stays consistent. */ | ||
| return { label: labels.join(" "), showFastModeIcon: fastModeEnabled }; |
There was a problem hiding this comment.
this is a shared helper; Settings uses the same trigger. a composer-only typography call (" · " → " ") should not silently restyle Settings. pass a join/separator (or a display override) from the composer call site, or keep upstream join and format only in the composer shell.
| } | ||
|
|
||
| /** Repository and worktree controls that sit above the prompt surface. */ | ||
| export const ComposerContextRow = memo(function ComposerContextRow({ |
There was a problem hiding this comment.
ComposerContextRow is a memoized div + data-* + pb-2. that does not earn a named export, a prop interface, or a guard that pins the wrapper. put data-fork-composer-context-row on the call-site element (or fold into ComposerShell) and delete the identity component.
| hue; light mode keeps the wash and drops each label to its dark Tailwind | ||
| shade so it stays readable. Hover deepens the wash rather than falling | ||
| back to the ghost variant's gray. */ | ||
| :root[data-fork="noahhendrickson-t3code"] { |
There was a problem hiding this comment.
theme.custom.css was already over 1k and this PR adds ~200 more lines of composer attribute CSS. if mode tokens stay in CSS, extract the composer block into a dedicated fork stylesheet (or co-locate with the shell module) instead of letting the global theme file keep absorbing layout. also: four nearly-identical mode hue blocks should collapse to a token table, not copy-paste.
| Which controls sit where is the main design contract. BranchToolbar's | ||
| worktree and branch chips sit above the surface as filled chips sharing | ||
| one selected-surface fill with no hairline, 4px apart, on the Auto chip's | ||
| container at the drawn 4px inline padding (20px tall, 4px radius) at 12px |
There was a problem hiding this comment.
intent still says context chips are "20px tall"; CSS comment + rules implement 24px ("four pixels taller (Noey's call)"). pick one number and make intent, CSS, and guards agree — drift here is how the next sync "fixes" the wrong thing.
Code review —
|
| Intent still claims | Guard deleted |
|---|---|
| "The prompt wrapper owns the 12.5rem scroll cap" | moves the desktop prompt scrollport onto the wrapper, not the editor |
| "explicit drag-over outline" | repaints the drag-over state the surface rule would otherwise swallow |
| "the placeholder/spacer overflow containment remains in place" | resizes the placeholder with the prompt, via a general sibling combinator |
| provider icon dropped in the composer only | drops the provider icon from the model pill, in the composer only |
| surface selected by name | paints the surface by name, so an added sibling cannot inherit the box |
| — | frees the prompt from upstream's three-line minimum, hides only the left slot's separators, takes the drawn colours in dark and defers to upstream in light |
Those CSS rules all still exist in theme.custom.css. They're just unguarded now — which is exactly the state .fork/AGENTS.md says the guard suite exists to prevent ("an upstream sync that silently drops the customization turns CI red instead of passing quietly"). The scrollport and provider-icon-scoping guards in particular encode bugs that were already found once.
And the replacements assert source text, not outcomes. .fork/AGENTS.md is explicit that a guard asserts the outcome. These don't:
expect(chatComposer).toContain('className="flex min-w-0 items-center gap-6"');
expect(chatComposer).toContain('"relative p-2"');
expect(chatComposer).toMatch(
/<ComposerControlRow\s+left=\{activePendingApproval \? null : composerModeControls\}\s+right=\{composerModelAndStatusControls\}/u,
);That last one pins a local variable name and the exact prop order. Add one utility class, run vp fmt in a way that reflows the JSX, or rename composerModeControls — CI goes red with zero behavior change. Meanwhile findings #2 and #3 above sail straight through, and #2 is literally inside the string being matched. The guard reads the gate and asserts nothing about what the gate should cover. That's the worst of both: high false-positive rate, no true-positive coverage.
The CSS guards (cssRules-based, checking selectors and declarations) are the good pattern here and should be the model — assert the rendered contract, not the source that produces it.
7. 🟠 The manifest contradicts the CSS on chip height
.fork/customizations.yaml L1138:
…on the Auto chip's container at the drawn 4px inline padding (20px tall, 4px radius) at 12px type…
theme.custom.css says height: 24px / min-height: 24px, the PR body says "24px tall", and the guard's own comment says "the Auto chip's container four pixels taller — 24px". The manifest is the fork's source of truth for intent; it's the one place that's wrong.
8. 🔵 Stale prop doc in fork-owned code
ComposerControlRow.tsx L7–12 still documents right as:
Where the message runs: the worktree/branch context strip. Absent on a project with no git repo, which leaves the row left-aligned rather than collapsing it — the left controls still apply.
right is now the model/effort/meter group and is always passed, so both sentences are wrong. The right?: optional and the {right ? … : null} branch are also dead now — worth either restoring a real absent case or dropping the optionality.
On the placeholder — not a defect, but worth a decision
I checked node 71-4121 rather than assume. The design genuinely reads "Ask anything,", trailing comma and all, so the implementation is faithful and I'm not flagging it as a typo.
But it's a comma-terminated sentence fragment shipping to users, and it drops @/$//discoverability — including fromaria-placeholder`, since that's fed the same string, so screen-reader users lose it too. The fork previously cared about this enough to keep a shortened hint in the slim shell specifically so those three sigils stayed discoverable. Worth taking back to design as a copy bug rather than reproducing it pixel-faithfully.
Things I checked that are fine
--color-emerald-700/-amber-700/-violet-700/-red-700all resolve — Tailwind v4's default palette is available andindex.cssalready uses them.- The
[data-testid="composer-editor"] ~ divplaceholder rule is safe: the only following siblings inside Lexical's wrapper are the empty<span/>spacer and the placeholder div;data-chat-composer-mobile-pending-actionsis outside that container. data-chat-composer-footerhas no remaining references anywhere in the repo.isComposerFooterCompactstill measures correctly — it observescomposerFormRef, not the removed footer element, soCompactComposerControlsMenustill works.:not([data-fork-composer-status] button)raises the geometry rule to (0,4,2), comfortably above the mode-chip rule's (0,3,1). The exclusion-by-name comment is accurate.
Summary: #1 breaks CI, #2 and #3 are real behavioral regressions in states the redesign didn't intend to touch, and #6 removes the safety net that would have caught #2 on the next upstream sync. The layout and the density-machinery deletion are solid work — please gate right on approval, restore the mobile pending-answer suppression, fix or update the traits test, and rebuild the guards around outcomes.
Generated by Claude Code
|
Following up on finding #1 in my review above — CI has now completed on
All three failures are the ones predicted, in Two things worth drawing out of this: It's the only thing failing. The third failure is the argument in finding #4, stated by the test itself. Generated by Claude Code |
Code review — request changesReviewed Blocking
left={activePendingApproval ? null : composerModeControls}
right={composerModelAndStatusControls}Changing model/effort mid-approval retargets the provider for a turn already awaiting a decision. Gate the interactive right-side controls the same way; the context meter can stay as a readout.
High
Medium
Nits / already covered elsewhere
VerdictRequest changes. Land the layout once (1) CI is green without weakening the shared traits contract, (2) approval + mobile pending gates are restored for the relocated controls, and (3) guards again pin the outcomes the intent claims — especially the gates this PR broke. Reviewed by Cursor Cloud agent (run) — Cursor Grok 4.5. |
| flip modes for a run whose approval is still unresolved. BranchToolbar | ||
| is deliberately not gated on approval; upstream showed it too. */} | ||
| {/* Modes sit below-left; context usage, model and effort sit below-right. */} | ||
| <ComposerControlRow |
There was a problem hiding this comment.
Blocking — approval gate incomplete after relocation. Model/effort used to unmount with the in-box footer during activePendingApproval. They now live on right, but only left is gated. Changing model/effort mid-approval retargets the provider for a turn still awaiting a decision. Gate the interactive right-side controls the same way; keep the context meter if you want a readout.
| /* fork:begin fork-composer-shell — see .fork/customizations.yaml#fork-composer-shell */ | ||
| /* A plain space, not upstream's " · ": the composer's effort chip reads | ||
| "High 1M". Settings uses the same trigger and stays consistent. */ | ||
| return { label: labels.join(" "), showFastModeIcon: fastModeEnabled }; |
There was a problem hiding this comment.
Blocking — CI red + Settings leak. This shared helper now joins with " ", which fails TraitsPicker.test.ts (3 cases) and also restyles Settings’ traits trigger. "High Thinking On" loses the trait boundary the " · " separator was carrying. Please don’t just update the expected strings — pass a composer-only separator/override and keep " · " as the default for other callers.
| Which controls sit where is the main design contract. BranchToolbar's | ||
| worktree and branch chips sit above the surface as filled chips sharing | ||
| one selected-surface fill with no hairline, 4px apart, on the Auto chip's | ||
| container at the drawn 4px inline padding (20px tall, 4px radius) at 12px |
There was a problem hiding this comment.
Medium — intent/CSS drift. This says context chips are 20px tall; theme.custom.css implements 24px (“four pixels taller (Noey's call)”), and the PR body/guards agree with 24. Manifest is the fork source of truth — update this to 24px (or change the CSS).
| hasComposerHeader, which resolves density to tall, so the | ||
| right conjunct can never be false when the left is true. */} | ||
| {isComposerSlim ? ( | ||
| {!activePendingApproval ? ( |
There was a problem hiding this comment.
Blocking — duplicate mobile primary actions. When showMobilePendingAnswerActions is true, the absolute block below still mounts, and this inline slot no longer has the old hidden sm:flex suppression. Phones get two Next/Submit clusters for one question. Restore !showMobilePendingAnswerActions (or equivalent) here.
| expect(chatComposer).toContain("<ComposerContextRow>{contextStrip}</ComposerContextRow>"); | ||
| expect(chatComposer).toContain('data-fork-composer-box="true"'); | ||
| expect(chatComposer).toMatch( | ||
| /<ComposerControlRow\s+left=\{activePendingApproval \? null : composerModeControls\}\s+right=\{composerModelAndStatusControls\}/u, |
There was a problem hiding this comment.
High — guard pins the incomplete gate as a string. This asserts the exact JSX that leaves right ungated during approval (finding above), so a real regression is locked in as “correct.” Prefer an outcome-level contract: interactive model/effort controls must not render while activePendingApproval is set.
|
Addressed the review feedback in 98a258c:
Local verification: 199 focused tests passed (including all 181 fork guards), web typecheck passed, fork-owned lint reported no warnings, and targeted formatting passed. |
Empty-draft hero text was still stacked just above the input after the composer was docked. Render the greeting as its own centered layer. Co-authored-by: Cursor <cursoragent@cursor.com>
1 participant
Problem
The chat composer didn't match the updated t3-fork Figma (node 71-4121): an earlier attempt got the bones right but deviated on type sizes, chip treatments, and control styling, and still carried the old tall/slim density machinery.
Fix
Reworked the
fork-composer-shellcustomization to the current design plus adjustments made during live review:#2a2a2achips, 24px tall, 4px radius, 4px inline padding (2px optical correction on the folder glyph), no hairline, workspace caret dropped.muted-foregroundplaceholder, muted#474747focus recolour, send button pinned top-right as the prompt wraps.composerDensity.ts,useComposerPromptWrapLatch.ts, and the density switch; the surface now grows only with the editor.Manifest intent, watch list (
TraitsPicker.tsxadded), and the guard suite updated to assert the new outcomes — 178 guard tests, web typecheck, and.fork/lint-owned.mjsall pass.Before/after screenshots to follow in a comment.
Built with Claude Fable 5 on Claude Code.
🤖 Generated with Claude Code