fix(web): collapse the composer when it is not focused - #7446
fix(web): collapse the composer when it is not focused#7446chrisdeeming wants to merge 1 commit into
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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
vouch:trusted
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
There was a problem hiding this comment.
Two findings on the resting-composer change. Extending the collapsed state from mobile to every viewport is fine in principle, but the editor is hidden with display:none while it stays mounted, and nothing in the imperative focus paths un-collapses it — so desktop focus entry points silently stop working. The collapsed draft text also now uses a token that is identical to the placeholder token in the shipped palettes.
Posted via Macroscope — UI Consistency
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This PR extends mobile composer collapse behavior to desktop, introducing new user-facing functionality with complex focus/blur reconciliation logic. Multiple review comments identified bugs in the implementation that required fixes. The scope and complexity warrant human review. You can add or adjust custom eligibility rules. Learn more. |
164e54b to
690627c
Compare
690627c to
01ebc6b
Compare
There was a problem hiding this comment.
Collapse now applies on desktop, so keyboard-driven composer surfaces that only exist inside the collapsed (hidden / unmounted) subtree lose their entry point. Two spots below.
Posted via Macroscope — UI Consistency
01ebc6b to
c84ebfd
Compare
There was a problem hiding this comment.
Reviewed the resting-composer collapse for UI consistency. The focus entry points now route through expandComposerThenFocus, which resolves the earlier "collapsed forever on desktop" problem. Two findings remain in the collapsed row, which is now the default resting state on desktop rather than a mobile-only view.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
UI consistency review: one issue found in the resting-composer collapse work. The earlier findings (menus opening against an unmounted subtree, and Tab entry into the collapsed row) look correctly addressed by hasOpenMenu and the onFocus/data-chat-composer-collapsed-controls split.
Posted via Macroscope — UI Consistency
c84ebfd to
3e02768
Compare
3e02768 to
6f1ab32
Compare
There was a problem hiding this comment.
Reviewed the desktop resting-composer change. The previously flagged issues (focus entry expanding the editor, keyboard Tab entry, draft vs. placeholder tone, sm:px-4 alignment, keybinding-opened menus and the stash pulse holding the composer open) all look resolved, and every editor focus entry point now routes through expandComposerThenFocus. One minor desktop affordance gap remains on the collapsed row.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
Now that this row is the resting state on desktop, hovering the composer shows the default arrow cursor: Tailwind v4's preflight sets no cursor on button, and the shared controls opt in explicitly (Button uses cursor-pointer, ComposerPrimaryActions uses enabled:cursor-pointer). The expanded composer shows a text caret over the same area, so the cursor flips on focus. cursor-text here keeps the resting row reading as an input.
| "min-w-0 flex-1 cursor-text truncate rounded-sm bg-transparent p-0 text-left text-[14px] focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring", |
Posted via Macroscope — UI Consistency
6f1ab32 to
f80fc34
Compare
There was a problem hiding this comment.
Reviewed the resting-composer collapse for UI consistency. The earlier gaps (imperative focus entry points, tab-in, menu/pulse hold-open, sm:px-4, draft vs placeholder tone, focus ring) all look resolved. Two remaining items are specific to the collapsed row now being the desktop resting state, where it was previously mobile-only: it drops the composer's typography settings, and its buttons have no pointer cursor.
Posted via Macroscope — UI Consistency
647ad5f to
b1744d3
Compare
b1744d3 to
4ce8dff
Compare
There was a problem hiding this comment.
One finding: the new isBusy term keeps the composer permanently expanded while an approval or pending user-input request is outstanding, which makes the collapsed approval / pending-input variants below unreachable and changes existing mobile behavior. Details inline.
Posted via Macroscope — UI Consistency
| // Every in-flight signal, not just a running turn: the footer owns the | ||
| // Stop control, the send spinner and the "Preparing worktree" label. | ||
| isBusy: phase === "running" || isSendBusy || isConnecting || isPreparingWorktree, |
There was a problem hiding this comment.
isBusy folds in phase === "running", but the session status stays running for the whole time an approval or user-input request is outstanding — the server never dispatches a session change for request.opened / user-input.requested (and a provider waiting state is mapped to running), and derivePhase reads nothing but session.status. So whenever activePendingApproval or pendingUserInputs is non-empty, shouldCollapseRestingComposer returns false.
That contradicts the comment just above ("Approvals and pending inputs … render their own collapsed variants") and makes those variants dead: isComposerCollapsed && activePendingApproval (2870) and isComposerCollapsed && pendingUserInputs.length > 0 (2887) can no longer render. Those rows shipped on mobile before this PR, where the old rule (isMobileViewport && !forceExpandedOnMobile && !isComposerFocused) had no busy term — so a narrow viewport now keeps the full approval panel plus editor plus toolbar mounted at rest instead of the compact row, and showMobilePendingAnswerActions (1300) is pinned on for the same reason.
Excluding outstanding requests from the busy gate keeps the Stop/spinner rule for real in-flight turns while restoring the collapsed request rows:
| // Every in-flight signal, not just a running turn: the footer owns the | |
| // Stop control, the send spinner and the "Preparing worktree" label. | |
| isBusy: phase === "running" || isSendBusy || isConnecting || isPreparingWorktree, | |
| // Every in-flight signal, not just a running turn: the footer owns the | |
| // Stop control, the send spinner and the "Preparing worktree" label. A | |
| // pending request is excluded: the session stays "running" while it waits, | |
| // and it has its own collapsed row carrying the actions. | |
| isBusy: | |
| (phase === "running" || isSendBusy || isConnecting || isPreparingWorktree) && | |
| activePendingApproval === null && | |
| pendingUserInputs.length === 0, |
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
This was the best catch of the review — fixed in 09638f1 with your suggestion.
I verified the chain rather than taking it on trust: orchestrationSessionStatusFromRuntimeState maps waiting to running (ProviderRuntimeIngestion.ts:273-291), and derivePhase reads nothing but session.status (session-logic.ts:1710-1722). So phase really does stay running for the whole time a request is outstanding, and my isBusy term made the collapsed approval and pending-input rows unreachable — rows that shipped on mobile before this PR. That is a regression I introduced, and the comment above the flag was wrong as a result.
isBusy now excludes outstanding requests, so real in-flight turns still keep Stop and the spinner mounted while those variants render again.
There was a problem hiding this comment.
Sorry, I'm unable to act on this request because you do not have permissions within this repository.
4ce8dff to
09638f1
Compare
09638f1 to
a1d9c55
Compare
There was a problem hiding this comment.
One consistency gap: the collapsed pending-user-input row now renders on desktop but did not get the pointer/focus treatment the sibling collapsed prompt row received in this PR.
Posted via Macroscope — UI Consistency
| )} | ||
| onPointerDown={(event) => event.preventDefault()} | ||
| onClick={expandMobileComposer} | ||
| onClick={expandComposer} |
There was a problem hiding this comment.
This collapsed "Write custom answer" row is now a desktop resting state too (gate at 2911 is isComposerCollapsed, and isBusy excludes pending inputs), but it kept the mobile-only styling while the sibling collapsed prompt row below gained cursor-pointer and a focus-visible ring. On a pointer device the row highlights on hover (hover:bg-background/80 on the wrapper) yet shows the default arrow — and in the multi-select branch the ComposerPrimaryActions button next to it carries enabled:cursor-pointer, so the cursor flips mid-row. Suggest matching the other collapsed expander:
- "min-w-0 flex-1 truncate bg-transparent py-1.5 text-left text-sm",
+ "min-w-0 flex-1 cursor-pointer truncate rounded-sm bg-transparent py-1.5 text-left text-sm focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring",Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
Fixed in 64abe7e as suggested — the "Write custom answer" row now carries cursor-pointer, rounded-sm and the focus-visible ring, matching the sibling collapsed expander. The cursor no longer flips mid-row against the adjacent ComposerPrimaryActions button.
There was a problem hiding this comment.
Sorry, I'm unable to act on this request because you do not have permissions within this repository.
a1d9c55 to
64abe7e
Compare
There was a problem hiding this comment.
One finding: the collapsed pending-answer row is now a desktop resting state but still renders composer prompt text in the interface font, unlike the sibling collapsed prompt row that was aligned to the editor's typography tokens.
Posted via Macroscope — UI Consistency
| type="button" | ||
| className={cn( | ||
| "min-w-0 flex-1 truncate bg-transparent py-1.5 text-left text-sm", | ||
| "min-w-0 flex-1 cursor-pointer truncate rounded-sm bg-transparent py-1.5 text-left text-sm focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring", |
There was a problem hiding this comment.
This row now renders at rest on desktop too (gate at 2946 is isComposerCollapsed), and it shows activePendingProgress.customAnswer — the exact value the editor renders (3241). The editor wrapper types that text with the appearance tokens ([font-family:var(--font-composer,…)] [font-size:var(--font-size-prompt,0.875rem)], ComposerPromptEditor.tsx:1751), which the Prompt font / Prompt font size settings write (12–20px, optional custom family). Hardcoding text-sm in the interface font means a parked custom answer is shown at the wrong size/family and re-typesets on expand for anyone who changed those settings — the same contract the sibling collapsed prompt row below now honours (3015). Suggest using the same tokens here.
| "min-w-0 flex-1 cursor-pointer truncate rounded-sm bg-transparent py-1.5 text-left text-sm focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring", | |
| "min-w-0 flex-1 cursor-pointer truncate rounded-sm bg-transparent py-1.5 text-left [font-family:var(--font-composer,var(--font-sans))] [font-size:var(--font-size-prompt,0.875rem)] [@media(max-width:39.999rem)_and_(pointer:coarse)]:[font-size:max(var(--font-size-prompt,1rem),16px)] focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring", |
Posted via Macroscope — UI Consistency
64abe7e to
61d1cbe
Compare
There was a problem hiding this comment.
One finding on the collapsed prompt row: it stays interactive in a state where the editor cannot take focus, so the new expand/reconcile handshake flashes the composer open and shut.
Posted via Macroscope — UI Consistency
| "min-w-0 flex-1 cursor-pointer truncate rounded-sm bg-transparent p-0 text-left [font-family:var(--font-composer,var(--font-sans))] [font-size:var(--font-size-prompt,0.875rem)] [@media(max-width:39.999rem)_and_(pointer:coarse)]:[font-size:max(var(--font-size-prompt,1rem),16px)] focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring", | ||
| (activePendingProgress ? activePendingProgress.customAnswer : prompt.trim()) | ||
| ? "text-foreground" | ||
| : "text-placeholder", | ||
| )} | ||
| onPointerDown={(event) => event.preventDefault()} | ||
| onClick={expandMobileComposer} | ||
| onClick={expandComposer} | ||
| // Tabbing here is a request to start typing, so expand and let | ||
| // the caret land in the editor rather than parking on a button. | ||
| onFocus={expandComposer} | ||
| aria-label="Expand composer" |
There was a problem hiding this comment.
When projectSelectionRequired is set (local draft with no project), the editor is rendered disabled (line 3271), so Lexical's root is contenteditable="false" and focusAt's rootElement.focus() is a no-op. Clicking this row now expands the composer, misses focus on all five attempts, and the release frame reconciles by collapsing it again — on desktop the resting composer flashes open and shuts and can't be opened at all (previously it just stayed expanded, and collapse never applied here outside mobile).
The sibling send button already opts out of that state through collapsedComposerPrimaryActionDisabled; disabling this button too keeps the row consistent with the disabled editor and with the opacity-75 the surface already shows.
| "min-w-0 flex-1 cursor-pointer truncate rounded-sm bg-transparent p-0 text-left [font-family:var(--font-composer,var(--font-sans))] [font-size:var(--font-size-prompt,0.875rem)] [@media(max-width:39.999rem)_and_(pointer:coarse)]:[font-size:max(var(--font-size-prompt,1rem),16px)] focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring", | |
| (activePendingProgress ? activePendingProgress.customAnswer : prompt.trim()) | |
| ? "text-foreground" | |
| : "text-placeholder", | |
| )} | |
| onPointerDown={(event) => event.preventDefault()} | |
| onClick={expandMobileComposer} | |
| onClick={expandComposer} | |
| // Tabbing here is a request to start typing, so expand and let | |
| // the caret land in the editor rather than parking on a button. | |
| onFocus={expandComposer} | |
| aria-label="Expand composer" | |
| "min-w-0 flex-1 truncate rounded-sm bg-transparent p-0 text-left [font-family:var(--font-composer,var(--font-sans))] [font-size:var(--font-size-prompt,0.875rem)] [@media(max-width:39.999rem)_and_(pointer:coarse)]:[font-size:max(var(--font-size-prompt,1rem),16px)] enabled:cursor-pointer focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring", | |
| (activePendingProgress ? activePendingProgress.customAnswer : prompt.trim()) | |
| ? "text-foreground" | |
| : "text-placeholder", | |
| )} | |
| disabled={projectSelectionRequired} | |
| onPointerDown={(event) => event.preventDefault()} | |
| onClick={expandComposer} | |
| // Tabbing here is a request to start typing, so expand and let | |
| // the caret land in the editor rather than parking on a button. | |
| onFocus={expandComposer} | |
| aria-label="Expand composer" |
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
Correct, and this was a regression from the retry loop I added for the previous finding — it turned "stays expanded" into "flashes open and shut", which is worse. Fixed in db1b04e.
Two layers, since the click path is not the only way in:
- The expand button is now
disabledunder the same three conditions that disable the editor (isConnecting || isComposerApprovalState || projectSelectionRequired), withenabled:cursor-pointerso the affordance matches. That mirrors how the sibling send button already opts out. expandComposerThenFocusbails before starting the handshake when the editor is disabled. There are six entry points into it (imperative handle, type-to-focus, prompt replacement, stash restore, context insertion), and any of them could otherwise hit the same flash — the guard covers all of them, not just the click.
Verified the happy path is unaffected: on a normal thread the row is enabled and still expands 50px → 144px with the caret landing in the editor.
There was a problem hiding this comment.
Sorry, I'm unable to act on this request because you do not have permissions within this repository.
61d1cbe to
db1b04e
Compare
db1b04e to
ae79b5f
Compare
There was a problem hiding this comment.
One consistency finding on the collapsed (resting) composer row, which this PR promotes from a mobile-only state to the desktop resting state.
Posted via Macroscope — UI Consistency
| // Exempt from focus-driven expansion: expanding would unmount | ||
| // this button while the keyboard user is standing on it. | ||
| data-chat-composer-collapsed-controls="true" | ||
| className="flex size-8 shrink-0 items-center justify-center rounded-full bg-message-action text-message-action-foreground enabled:cursor-pointer hover:bg-message-action-hover focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring disabled:pointer-events-none disabled:opacity-30" |
There was a problem hiding this comment.
Now that the resting row is a desktop state, this hand-rolled send button occupies the same slot as the shared one but does not look like it. The send control in ComposerPrimaryActions.tsx:222-268 is the same 32px circle on desktop, yet it carries shadow-xs + enabled:shadow-message-action/24, enabled:inset-shadow-[0_1px_--theme(--color-white/16%)], hover:scale-105 and active: feedback, and draws a 14px glyph at strokeWidth="1.8"; this copy is flat with a 16px glyph at strokeWidth="2" (line 3075). Clicking the row swaps one for the other in place, so on desktop the send affordance visibly changes weight and gains a shadow/hover-scale the moment the composer expands. The aria-label also stays "Send message" where the shared button reports sendDisabledReason / "Environment disconnected".
Suggest lifting the shared send button's visual (classes + glyph) into one small component and rendering it here too, instead of maintaining a second divergent copy.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
Agreed, and took the suggested route rather than patching the copy — fixed in 52f0e82.
New ComposerSendButton component; both the expanded footer and the collapsed resting row render it, so there is no second divergent copy to drift. The label is derived by a shared composerSendButtonLabel() helper, so the collapsed button now reports sendDisabledReason / "Environment disconnected" / "Connecting" / "Preparing worktree" / "Sending" instead of a static "Send message". Extracting it also left Spinner, StageBackdropButtonArt, useSidebarStageBackdropVariant and useEnvironmentIdentificationMode unused in ComposerPrimaryActions, which are now removed.
Measured both states in the browser at 1440px and they are byte-identical: 32x32, 14px glyph at stroke-width 1.8, 0.15s transition, same shadow and same label. Clicking the row no longer changes the send affordance.
Note the two electronPasskeys.test.ts typecheck errors on this branch are pre-existing — I confirmed they reproduce on a clean upstream/main checkout.
There was a problem hiding this comment.
Sorry, I'm unable to act on this request because you do not have permissions within this repository.
ae79b5f to
52f0e82
Compare
github-actions
Bot
added
size:XL
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 52f0e82. Configure here.
The resting composer reserved 144px on every thread: a 70px minimum editor that never shrank when empty, plus a 48px toolbar row. That chrome stayed mounted while you were only reading, leaving T3 Code with a noticeably smaller transcript than comparable apps. Collapse it to a single ~50px row whenever it is unfocused. A drafted message stays visible there, muted and truncated, and staged attachments keep the composer open since the row renders text only. Narrow viewports already behaved this way; this drops the width gate so web, desktop and mobile share one rule.
52f0e82 to
49d0019
Compare
|
Superseded by #7855. |
1 participant
What Changed
The composer collapses to a single ~50px row whenever it is not focused, instead of always reserving 144px.
Why
The resting composer reserved a 70px minimum editor that never shrank when empty, plus a 48px toolbar row. That chrome stayed mounted while you were only reading, giving T3 Code a noticeably smaller transcript than comparable apps. Collapsing on blur returns ~94px to the thread.
UI Changes
Same thread at 1440x900, unfocused.
Before
Empty composer, then with a parked draft. Both 144px with the full toolbar.
After
Empty composer, then with a parked draft. Both 50px; the draft shows muted and truncated.
Checklist
Written by Claude Opus 5 via Claude Code.
Note
Medium Risk
Large interaction surface in ChatComposer (focus, blur, collapse, popups, stash, imperative focus APIs); regressions could affect typing, sending, and keyboard shortcuts across web/desktop/mobile.
Overview
The chat composer now collapses to a single resting row when unfocused on desktop as well as mobile, not only on narrow viewports. Parked draft text still shows truncated in that row; attachments, in-flight image compression, running turns (Stop/spinner), plan follow-up chrome, and transient UI (stash menu, model picker, stash pulse) keep the full composer open via new
shouldCollapseRestingComposerincomposerFooterLayout.ts.Focus and expand behavior is reworked:
expandComposerThenFocusretries editor focus across animation frames and reconciles collapse if focus never lands; blur into any Base UI floating layer (dialogs, sheets, menus, etc.) defers collapse andfocusin/pointerdownlisteners re-run the check when focus leaves popups. Stash (⌘S) and restore paths avoid stealing focus or flashing expand when the editor is disabled.A shared
ComposerSendButton(withcomposerSendButtonLabel) unifies send styling, busy spinner, and aria labels in the expanded footer and collapsed row; the collapsed send can submit without expanding the composer.Reviewed by Cursor Bugbot for commit 49d0019. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Collapse the
ChatComposerwhen it loses focus across all viewportsshouldCollapseRestingComposerutility in composerFooterLayout.tsFLOATING_LAYER_SELECTORand theisInsideFloatingLayercheck in ChatComposer.tsx; a globalfocusin/pointerdowncapture re-runs the deferred check once focus settles outside all popupsexpandComposerThenFocusretries editor focus up to 5 animation frames and skips expansion when the editor is disabled, avoiding premature collapseComposerSendButtoncomponent in ComposerSendButton.tsx with correct busy indicator, backdrop styling, and accessible labeling; replaces the inline send button in ComposerPrimaryActions.tsxdata-chat-composer-mobile-collapsedis replaced bydata-chat-composer-collapsed; in-flight image compressions now count as attachments for collapse gating, and stash-by-shortcut restores focus to the pre-stash element if it was outside the composerMacroscope summarized 49d0019.