Skip to content

refactor(web): the markup card takes the measured clamp, and the fold gutter gets a name#556

Merged
gbasin merged 1 commit into
masterfrom
chore/clamp-sweep
Jul 16, 2026
Merged

refactor(web): the markup card takes the measured clamp, and the fold gutter gets a name#556
gbasin merged 1 commit into
masterfrom
chore/clamp-sweep

Conversation

@gbasin

@gbasin gbasin commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

Finishes the clamp unification from #545 — the two call sites and the gutter I'd declined there.

EntryQuoteCard adopts the clamp primitive

The markup card hand-rolled the last unmeasured clamp in the app: a static max-h box that always offered "Show all changes" and always drew a fade, whether or not anything was hidden below the cut. It also sat one position: static away from #544CriticMarkupView emits an sr-only span (position: absolute), which under a static clamp anchors above the clip, escapes it, and inflates the scroll height.

Adopting useClamp fixes both. The control and the fade now appear only when the diff is measured to overflow, and the box is relative so the clip holds. The change count never lived on the button alone, so nothing is lost when it hides — the header still reads "N changes".

Taking a second caller forced two latent bugs in the primitive into the open

Both are real, both would have shipped, and neither was caught by the existing suite:

  • The toggle could never have appeared in production. useClamp measured through a ref in an effect keyed on [canClamp, expanded]. EntryQuoteCard renders its clamped box only once useMarkupArtifact resolves, so the node attached on a later render, the effect never re-ran, and no ResizeObserver was ever created. The node is state now, so the effect runs when it lands.
  • ClampBoundary remounted every clamped subtree on every render. Built inline in the hook, it was a new component type each render, so React threw away everything beneath it — a whole message body — every time. Memoized on nested.

Each new test was A/B'd against the broken code first: red without the fix, green with it.

WorkFold gutter

Pure extraction — ml-[43px] max-w-[calc(100%-59px)] is byte-identical, now named STANDALONE_GUTTER with the derivation recorded (59 = 43 + 16, measured against a live thread) so the pair can't silently drift apart when only one is edited.

Deliberately not done

SessionCapabilitiesPopover truncates by item count (slice(0, 8) across six lists, +{hiddenCount} more, state keyed per-list in the parent), not by measured height. Adopting the clamp would mean rendering every item instead of eight, adding a controlled mode, and losing the exact count in the label — to dedup ~6 lines. Its line-clamp-2 descriptions have no toggle, so there's nothing to measure.

Verification

Full local gate green: pnpm check (lint + typecheck + 2534 unit tests), pnpm e2e (118 passed), mobile a11y, centaur-client build, web build:ci.

Known gap: the markup card's clamp is proven in jsdom, not a real browser — and jsdom fakes the very metrics the clamp reads. The same hook is browser-proven through MessageText via the existing "Show more grows a long agent reply" spec, and both bugs fixed here are React scheduling/identity issues rather than layout ones. e2e has no artifact-seeding helper and the card fetches its body over HTTP, so a real fixture is a larger build than this PR.

… gutter gets a name

EntryQuoteCard hand-rolled the last unmeasured clamp in the app: a static
`max-h` box that always offered "Show all changes" and always drew a fade,
whether or not anything was actually hidden below the cut. It also sat one
`position: static` away from #544 — CriticMarkupView emits an `sr-only` span,
which under a static clamp anchors above the clip, escapes it, and inflates
the scroll height. Adopting the clamp primitive fixes both: the control and
the fade now appear only when the diff is measured to overflow, and the box
is `relative` so the clip actually holds. The change count never lived on the
button alone, so nothing is lost when it hides — the header still reads "N
changes".

Taking a second caller forced two latent bugs in the primitive into the open:

  - It measured through a ref in an effect keyed on [canClamp, expanded], so a
    caller mounting its clamped element on a later render attached the node
    silently and was never measured at all. EntryQuoteCard renders only once
    its markup fetch resolves, so its toggle could never have appeared. The
    node is state now, and the effect runs when it lands.
  - ClampBoundary was rebuilt on every render, making it a new component type
    each time and remounting everything clamped inside it — a whole message
    body — on every render. It is memoized on `nested` now.

Both are pinned by tests that were A/B'd against the broken code first.

WorkFold's gutter pair is unchanged, just named and explained: the two values
are derived from each other (59 = 43 + 16) and silently drift apart when only
one is edited.

SessionCapabilitiesPopover is deliberately left alone; it truncates by item
count, not by measured height, and has no clamp to unify.
@gbasin
gbasin force-pushed the chore/clamp-sweep branch from 61771ca to 0651ff9 Compare July 16, 2026 21:53
@gbasin
gbasin added this pull request to the merge queue Jul 16, 2026
Merged via the queue into master with commit c21dbbb Jul 16, 2026
16 checks passed
@gbasin
gbasin deleted the chore/clamp-sweep branch July 16, 2026 22:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant