Skip to content

fix(fork): truncate the slim composer placeholder under the pills - #23

Merged
NoahHendrickson merged 2 commits into
customfrom
fix/fork-slim-composer-placeholder-truncate
Jul 28, 2026
Merged

fix(fork): truncate the slim composer placeholder under the pills#23
NoahHendrickson merged 2 commits into
customfrom
fix/fork-slim-composer-placeholder-truncate

Conversation

@NoahHendrickson

@NoahHendrickson NoahHendrickson commented Jul 27, 2026

Copy link
Copy Markdown
Owner

Summary

  • Slim shell empty-state hint was wrapping under the inline model pill. The absolute Lexical placeholder is invisible to the wrap latch (empty prompt ⇒ latch off; placeholder only exists when empty), so measuring cannot rescue it.
  • Shorten the slim hint to Ask anything, @tag, $skills, / commands instead of truncating the 64-character default — truncation would silently eat the only in-UI discovery for $ and /. Tall keeps the long form.
  • Dropped the CSS ellipsis rule and the redundant ChatComposer overflow-hidden clip from the first pass.

Test plan

  • vp test run apps/web/src/__fork_guards__/forkComposerShell.test.ts
  • Font-metric check: slim short hint is 259px vs ~460px slim column (long form 405px) at 14px Geist
  • Confirm @, $, and / remain in the slim hint
  • CI green; thermo-nuclear dual-path threads resolved

The absolute empty-state overlay wraps without growing the editor, so it
painted through the inline model pill; ellipsis it on one line instead.

Co-authored-by: Cursor <cursoragent@cursor.com>
@cursor

cursor Bot commented Jul 27, 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 size:M vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. labels Jul 27, 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

Verdict: do not approve yet. The CSS truncate is the right fix in the right layer — but the PR also ships a second, redundant special-case path through ChatComposer that should be deleted.

What's good

  • Real fix lives in theme.custom.css under the fork root scope
  • Guard + manifest intent updated with the change
  • No new abstraction, cast churn, or density-model complication

Blocker

  1. Dual-path / spaghetti growth in ChatComposerisComposerSlim && "overflow-hidden" is an admitted safety net. The CSS rule already has overflow: hidden + nowrap + ellipsis. Keep one mechanism in theme.custom.css; drop the TSX branch and the guard that freezes it.

Non-blocking

  1. Guard asserts ellipsis / nowrap but not CSS overflow: hidden — assert that on the theme rule once the TSX clip is gone.

No 1k-line threshold crossing from this PR (both files were already over). No approve until the dual path is collapsed.

Open in Web View Automation 

Sent by Cursor Automation: Thermo-nuclear PR review

Comment thread apps/web/src/components/chat/ChatComposer.tsx Outdated
Comment thread apps/web/src/__fork_guards__/forkComposerShell.test.ts Outdated

Copy link
Copy Markdown
Owner Author

Review

The diagnosis is right and the mechanism is right. I went looking for a reason the wrap latch couldn't rescue this instead, and there isn't one: nextWrapLatch returns false unconditionally on an empty prompt, and the placeholder only exists when the prompt is empty. So no amount of measuring the overlay can ever flip this to tall — a CSS fix is the only fix available. The PR body doesn't say that, and it's the strongest argument for the approach.

What follows is everything I'd want addressed before merge, worst first.


1. This is a 64-character string being fixed with three files

Work out which placeholders can actually reach slim. resolveComposerDensity sends approval, pending-input and plan-follow-up to tall via hasComposerHeader, and phone widths to tall via isNarrowViewport. That leaves three strings that can render in the slim shell:

placeholder length
Ask for follow-up changes or attach images 42
Enable a provider in Settings to send a message 46
Ask anything, @tag files/folders, $use skills, or / for commands 64

Only the last one is long enough to be at risk. So the entire observable effect of this change is eating the tail of the default hint — and since the column is flex-1 next to shrink-0 pills, how much it eats is a function of the selected model's name length and the window width. The overflow is marginal (~460px column at the composer's 768px max, per composerDensity.ts's own note); it only wraps once the column narrows a little.

The tail being eaten is $use skills, or / for commands. Those two affordances have no other discovery surface in the UI. The fix is silently deleting product information in the state the app spends most of its life in, to save a handful of characters.

A slim-specific placeholder — "Ask anything, @tag, $skills, / commands" or similar — fixes the layout at the source, keeps all three hints, needs no CSS, no clip, and no guard. It's a strictly smaller change than what's here. Was truncation chosen over shortening deliberately? If so that reasoning belongs in the PR body, because shortening is the obvious first thing a reader reaches for.

2. The overflow-hidden in ChatComposer.tsx is inert, and its comment describes geometry that can't happen

relative min-w-0 flex-1   ← auto height, flex-sized width, inside items-center

Nothing in normal flow can overflow that box. The editor is block w-full wrap-break-word max-h-50 overflow-y-auto — it wraps rather than overflowing horizontally, scrolls internally at 200px rather than overflowing vertically, and when it does grow the column grows with it. The only thing overflow: hidden can possibly clip here is absolutely-positioned content, i.e. exactly one element: the placeholder the CSS rule above already pins to one line.

The comment justifies it as stopping the placeholder painting "past the editor column into the inline pills." That direction is impossible. The placeholder is absolute inset-0 on ComposerPromptEditor's own relative wrapper, so it is bounded by the column's width by construction; a wrapped second line goes down, through the bottom of the 48px box — which is what the manifest and the PR title both say ("under the pills"). The clip works, but not for the reason given, and not for a failure the CSS rule leaves open.

The cost isn't zero. A safety net that can only fire when the primary rule is already broken converts a visible regression into an invisible one: if a sync drops the ellipsis rule, the clip hides the wrap and nobody notices in the UI. You've then traded a bug you can see for a bug only a source-text guard can see. AGENTS.md ("default to the smallest change", no unrequested belt-and-braces) points the same way. I'd drop the JSX change and its guard assertion.

3. The clip lands on the 16px line box, which is tighter than the ink

Independent of #2 — this one comes from the CSS rule itself. overflow: hidden on the placeholder clips at its inset-0 box, which in slim is the 16px line box the composer's 96/48 geometry is derived from (per the rule at theme.custom.css:359). Geist's content area at 14px is taller than 16px, so descender ink can be shaved by a fraction of a pixel. The visible portion of the truncated hint is Ask anything, @tag files/fol… — the y and g in "anything" are exactly the glyphs at risk.

Almost certainly sub-pixel and invisible. But it's new, it's on the one string this PR is about, and it's a 10-second look at 100% zoom — which brings us to:

4. The one test-plan box that could confirm this is the unchecked one

  • Visually check a started thread's slim composer: long placeholder ends before the model pill

That's the deliverable. The two ticked boxes assert that CSS text exists and that a computed style is what the stylesheet says it is — neither can tell you whether the ellipsis lands before the pill, whether the tail reads as truncation or as a rendering glitch, or whether #3 shows up. AGENTS.md asks for an integrated test-t3-app pass for any user-visible frontend change, and a change whose entire justification is visual is the clearest possible case for it. A screenshot in the PR body would settle #1 and #3 at once.

5. Guard test: stronger than I expected, with two real gaps

Credit where due — I mutation-tested the three assertions against the actual files (vp and node_modules aren't available in this environment, so I ran the regexes directly rather than through Vitest; CI's Test job was still in progress as I wrote this):

  • delete the rule → both CSS assertions fail ✅
  • remove the [data-fork-composer-density="slim"] scope → both fail ✅

So the scope is genuinely pinned. Gaps:

  • Nothing pins the tall side. The manifest states "Tall keeps wrapping" as the invariant, but a second, unscoped copy of the rule leaves all three assertions green while tall silently starts truncating too. That's the mutation this file should catch and doesn't.
  • Form. This file already carries a comment calling whole-file toMatch a "coincidence detector," and ships cssRules() to avoid it — the pill/prompt type test at line 108 is the model. The two new assertions are exactly the rejected form, and their [\s\S]{0,200} bridge currently depends on where vp fmt breaks a four-line selector. Anchoring on the rule instead is both more honest and more durable.
  • expect(chatComposer).toMatch(/isComposerSlim && "overflow-hidden"/u) pins a source string, not a behaviour — isComposerSlim ? "overflow-hidden" : null is identical and fails it. Moot if feat(web): Sidebar V2 tab design, pixel-rain working indicator, Phosphor icons #2 is dropped.

6. Manifest intent drifts back toward mechanism

#21 landed a couple of hours before this branch, trimming intents to invariants and moving narrative and mechanism to .fork/notes/FORK-CUSTOMIZATION-DECISIONS.md. The new sentence names the implementation: "nowrap on the Lexical sibling, overflow-hidden on the prompt column." The invariant a sync has to preserve is "the slim placeholder truncates on one line; tall keeps wrapping." The how belongs in the notes file, under the same id.

7. One for the notes file

This is the same failure found twice now — phone widths resolved it by forcing tall, desktop slim resolves it by truncating. The general shape is: absolutely-positioned content in the prompt column is invisible to the wrap latch by construction, because it never changes the observed editor height. Worth one line under fork-composer-shell so the third instance gets recognised on sight rather than re-derived.


Summary: #1 is the one I'd actually block on — the chosen fix is larger than the alternative and costs two discovery hints to do it. #2 should come out. #4 should happen before merge either way.


Generated by Claude Code

Truncation would eat $skills and /commands discovery. Slim gets a shorter
string that still names all three affordances; drop the CSS/JSX clip path.

Co-authored-by: Cursor <cursoragent@cursor.com>
@NoahHendrickson

Copy link
Copy Markdown
Owner Author

Addressed the review:

  1. Shorten instead of truncate — slim now uses Ask anything, @tag, $skills, / commands; tall keeps the long form. Truncation would have eaten the only in-UI discovery for $ and /.
  2. Dropped the ChatComposer overflow-hidden clip and the CSS ellipsis rule from the first pass (also clears the thermo-nuclear dual-path blocker).
  3. Guard now pins the slim short string + tall long string, and asserts there is no placeholder ellipsis rule.
  4. Manifest states the invariant only; mechanism + wrap-latch note landed in .fork/notes/FORK-CUSTOMIZATION-DECISIONS.md.

Visual check of a started-thread slim composer is next — will attach a screenshot.

@github-actions github-actions Bot added size:S and removed size:M labels Jul 27, 2026
@NoahHendrickson

Copy link
Copy Markdown
Owner Author

Visual / geometry check

Measured at the composer’s 768px max with the live prompt font (400 14px Geist Variable):

string width
slim short (Ask anything, @tag, $skills, / commands) 259px
tall long (64-char default) 405px

composerDensity.ts notes the slim editor column is ~460px at that max. Both fit there; the short form keeps ~200px of headroom for a longer model pill / narrower window — which is where the long form was wrapping. @, $, and / all remain in the slim string.

(Couldn’t keep a seeded started-thread projection alive across server restart — projector rebuilds wipe bare projection_* inserts — so this is a live font-metric check rather than a slim-shell screenshot.)

@NoahHendrickson
NoahHendrickson merged commit 2cf5004 into custom Jul 28, 2026
10 checks passed
@NoahHendrickson
NoahHendrickson deleted the fix/fork-slim-composer-placeholder-truncate branch July 28, 2026 00:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S 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