fix(fork): clip the composer placeholder out of the prompt scrollport - #31
Conversation
The scrollport move (#28) stopped the editor's own ink from inflating a scroll container, but the placeholder sits beside the editor, outside its clip. At the fork's 14/16 line box the placeholder's line box is a pixel taller than its inset-0 box, and absolutely-positioned overflow propagates to the wrapper's scrollable overflow — so the wrapper painted a scrollbar exactly while the prompt was empty, and typing (which unmounts the placeholder) made it vanish. Clip the editor's siblings (placeholder and Lexical's zero-height spacer) the way the editor clips its own ink, in the same >=40rem media block. Verified live: empty prompt not scrollable, content under the 200px cap grows without a thumb, content past it scrolls on the wrapper, clearing returns to a clean empty state. 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. |
There was a problem hiding this comment.
Thermo-nuclear: not approved.
The empty-prompt phantom thumb diagnosis is right, but the fix adds a fourth near-duplicate selector in a media block that already styles editor + siblings via :is([data-testid="composer-editor"], [data-testid="composer-editor"] ~ div).
Code-judo: collapse the editor unclamp and the new sibling clip into one prompt-scoped :is(...) with max-height: none; overflow-y: hidden;, delete the sibling-only rule, and point the guard at that unified selector. That removes the new complexity instead of documenting it, and avoids further sprawl on an already >1k theme.custom.css.
Behavior can stay identical; the structure should get smaller before this merges.
Sent by Cursor Automation: Thermo-nuclear PR review
| :root[data-fork="noahhendrickson-t3code"] | ||
| [data-fork-composer-prompt] | ||
| [data-testid="composer-editor"] | ||
| ~ div { | ||
| overflow-y: hidden; | ||
| } |
There was a problem hiding this comment.
This fourth selector is unnecessary. The media block already uses :is([data-testid="composer-editor"], [data-testid="composer-editor"] ~ div) for the type ramp — reuse that shape for the clip.
Fold the editor unclamp and this sibling-only rule into one prompt-scoped :is(...):
:root[data-fork="noahhendrickson-t3code"]
[data-fork-composer-prompt]
:is([data-testid="composer-editor"], [data-testid="composer-editor"] ~ div) {
max-height: none;
overflow-y: hidden;
}max-height: none on the abspos spacer/placeholder is harmless (initial value; they are sized by inset). Delete this sibling-only rule and most of the comment; keep a short note that siblings must share the clip because abspos overflow bypasses the editor. Update the fork guard to assert the unified :is(...) rule instead of locking in a fourth near-duplicate.
Net: −1 rule, reuse the existing selector shape, and stop growing an already >1k theme.custom.css for a duplicate.
Review: the diagnosis is right, the primitive is wrongThe root-cause analysis here is excellent and I could not fault it — abspos scrollable overflow propagating to the nearest scroll container is exactly the mechanism, the media scoping is right, and below But 1.
|
| variant | computed overflow-x |
placeholder clientHeight |
wrapper scrollable (the #31 bug) |
|---|---|---|---|
| no fix (merged build) | visible |
16 | yes |
this PR — overflow-y: hidden |
auto |
1 | no |
overflow-y: clip |
visible |
16 | no |
A screenshot of the three side by side is unambiguous: under this PR's rule the placeholder text does not render at all — the 16px box is entirely consumed by a horizontal scrollbar. Under clip the text renders and the vertical phantom is still fixed. Both fixes stop the propagation equally (wrapper scrollHeight 17 → 16); only hidden pays for it with the other axis.
This is reachable in production, not theoretical. ChatComposer.tsx:3210 passes activePendingApproval?.detail straight through as the placeholder, and that is server/model-supplied text. A path, a URL, and a commit SHA all reproduced it (scrollWidth 604 / 623 / 536 against a 400px box) — Chromium does not take a break opportunity at / under default word-break. The prompt value is "" in the approval state, so the placeholder is exactly what is on screen when this fires.
Suggested change — one word:
:root[data-fork="noahhendrickson-t3code"]
[data-fork-composer-prompt]
[data-testid="composer-editor"]
~ div {
overflow-y: clip;
}clip clips without creating a scroll container, so the other axis stays visible and neither axis can ever paint a scrollbar or be scrolled programmatically. It is strictly the safer primitive for "this element is paint, not a scrollport," and it is already used in this codebase (index.css:1028, index.css:1528, overflow-clip in ui/tooltip.tsx and ui/popover.tsx), so it breaks no new browser-support ground.
Caveat, stated plainly: my repro used default scrollbar metrics in headless Chromium, not the running app. The fork styles ::-webkit-scrollbar globally (index.css:1076, --app-scrollbar-width: 6px), which forces classic space-reserving scrollbars — but that rule sets width only, so the horizontal bar's height is the UA default. Worth confirming live, the same way the original 17-vs-16 was measured. The computed-value change to overflow-x: auto is not environment-dependent regardless.
The editor's own overflow-y: hidden from #28 has the identical side effect, but it is whitespace-pre-wrap wrap-break-word so it cannot overflow horizontally, and changing that rule is #28's business — separate change, per the repo's no-mixed-fixes rule.
2. A wrapping placeholder is now silently truncated
Independent of which primitive is used, and inherent to clipping. Measured with a two-line placeholder in a 400px box: scrollHeight 34 against clientHeight 16. Before this PR the wrapper scrolled, so the second line was reachable; after, it is clipped with no affordance.
For "Ask anything, @tag files/folders, $use skills, or / for commands" at desktop widths this never fires. But activePendingApproval.detail is arbitrary prose, and the composer can be narrow while the viewport is ≥40rem. The manifest's justification — "the clip matches what typed text already gets… so paint is unchanged" — only reasons about the one-line case and is not true for the wrapping one. I would either accept it and say so in the manifest, or truncate the placeholder deliberately so it degrades to an ellipsis instead of a hard cut.
3. The guard's editor lookup now also matches the new rule
cssRules returns rules in source order and rules.find takes the first match. The editor predicate is inDesktopLineBoxMedia && includes("[data-fork-composer-prompt]") && includes('[data-testid="composer-editor"]') — and the new sibling selector satisfies all three. It resolves correctly today only because the unclamp rule happens to sit earlier in the file. Reorder the two blocks and the max-height: none assertion starts interrogating the sibling rule and fails for a reason that has nothing to do with the invariant.
Given this file already ate one bug from a predicate that was satisfied by the wrong thing (the @media prefix-match noted in the comment at forkComposerShell.test.ts:191), this seam deserves the same treatment:
const editor = rules.find(
(rule) =>
inDesktopLineBoxMedia(rule) &&
rule.selector.includes("[data-fork-composer-prompt]") &&
rule.selector.includes('[data-testid="composer-editor"]') &&
!/~\s*div/u.test(rule.selector),
);plus expect(siblings).not.toBe(editor) to pin that these are two distinct rules.
4. The guard pins the mechanism, not the invariant
expect(siblings!.body).toMatch(/overflow-y:\s*hidden/u) fails if anyone adopts clip — the guard would block its own fix. The invariant is "the editor's siblings do not contribute scrollable overflow to the wrapper," so /overflow-y:\s*(hidden|clip)/u expresses it without freezing the primitive.
5. ~ div now carries clipping, not just type — worth a watch note
I checked: every Lexical plugin in ComposerPromptEditor.tsx returns null, so today the seam matches only the placeholder and Lexical's spacer, exactly as the PR says. No live bug. But the type-ramp rule's ~ div sets font-size and leading — benign for anything that lands there — whereas this rule clips. The first plugin that renders a mention or command dropdown in place rather than through a portal gets clipped to the prompt's line box and the cause will be non-obvious. One line in the manifest's watch note is enough.
Question
With the type ramp already setting line-height: 16px on this exact ~ div selector, what makes the placeholder's line box 17px inside a 16px box? If it is sub-pixel rounding surfacing through scrollHeight, then the overhang is font- and zoom-dependent, the clip is the right remedy anyway, and the manifest's "its text line box is 17px" is worth recording as a measurement rather than a mechanism — otherwise the next reader will go looking for a leading bug that is not there.
Verdict: finding 1 is worth fixing before merge — it is a one-word change that trades a scroll container for a clip and removes a live regression path through the approval-state placeholder. 3 and 4 are cheap and keep this guard honest. 2 and 5 are judgement calls I would at least write down.
Not run locally: this container has no installed workspace deps, so I could not execute the guard suite — CI and your own run cover that. Everything above is from reading the diff plus the Chromium measurements described.
Generated by Claude Code
Review caught overflow-y: hidden computing the sibling's overflow-x to auto — a pure-paint element became a scroll box, and the approval-state placeholder is server-supplied text with no break opportunity, so the reserved horizontal scrollbar would consume the entire 16px line. Live measurement while applying the fix surfaced the adjacent case: with x left visible (overflow-y: clip alone), that same unbreakable text propagates sideways overflow into the wrapper, whose overflow-x computes to auto alongside its overflow-y: auto, and paints a 16px horizontal scrollbar there — a latent approval-state bug that exists on custom today. overflow: clip contains the siblings on both axes without creating a scroll container anywhere. Verified live: empty prompt not scrollable; a 1877px unbreakable placeholder in a 736px box renders its 16px line with zero reserved scrollbar height and zero sideways scroll range on the wrapper. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Review addressed in f2b06db — and it went one step further than the suggestion. Taken: Declined (Cursor): merging into the editor's 🤖 Generated with Claude Code |
The `editor` rule lookup matched on prompt scope + the composer-editor testid inside the >=40rem media — three predicates the sibling clip rule added in f2b06db satisfies as well. `rules.find` returned the unclamp rule only because it happens to sit earlier in theme.custom.css. Verified by swapping the two blocks: the guard failed with `expected '\n overflow: clip;\n ' to match /max-height:\s*none/u` — the unclamp assertion interrogating the clip rule, a failure with nothing to do with the invariant it names. With the `~ div` exclusion the same swap stays green, and `not.toBe(editor)` pins the two rules as distinct so a future collapse into one selector is caught rather than silently halving the coverage. Same class of wrong-thing-satisfied-the-predicate bug as the @media prefix match already recorded in this file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Addressed the remaining guard nit from the review (finding 3) in e1a91b4. The Confirmed it was live, not theoretical: swapping the two CSS blocks failed the suite with Findings 2 and 5 (wrapping-placeholder truncation; a watch note for the day a Lexical plugin renders in place rather than through a portal) stay as recorded judgement calls — neither changes behaviour today, and both are manifest prose rather than code. 27 tests in this file green, |


Follow-up to #28. The scrollport move stopped the editor's own ink from inflating a scroll container, but missed the editor's siblings.
The bug
On the merged build the composer showed a scrollbar while the prompt was empty, focused or not, and it disappeared on typing — the inverse of the phantom #28 fixed.
Root cause (measured live)
scrollHeight17 vsclientHeight16 on an empty prompt.overflow-y: hiddenclip. At the fork's 14/16 line box its text line box is 17px inside its 16pxinset-0box, and scrollable overflow of an absolutely-positioned descendant propagates to the wrapper scrollport.scrollHeightto 16, confirming it as the sole contributor.Fix
One rule in the existing
>=40remblock:[data-fork-composer-prompt] [data-testid="composer-editor"] ~ div { overflow-y: hidden }— the same general-sibling seam the type-ramp rule already uses (catches the placeholder and Lexical's zero-height spacer). The clip matches what typed text already gets from the editor's ownoverflow-y: hidden, so paint is unchanged. Below 40rem nothing changes.Guard extended in
forkComposerShell.test.ts(samecssRulesat-rule-ancestry style pinning the rule inside the media block); manifest intent updated.Verified against the running app
🤖 Generated with Claude Code