Skip to content

fix(web): a clamped reply's footnotes stop inflating the channel's scroll height#544

Merged
gbasin merged 1 commit into
masterfrom
fix/clamp-abspos-scroll-leak
Jul 16, 2026
Merged

fix(web): a clamped reply's footnotes stop inflating the channel's scroll height#544
gbasin merged 1 commit into
masterfrom
fix/clamp-abspos-scroll-leak

Conversation

@gbasin

@gbasin gbasin commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

What

#testing scrolled to a blank void. The transcript's scroll container reported 7481px while the content only laid out to 4497px, so the auto-scroll-to-bottom landed in ~2940px of empty space with no messages visible — just the "↑ Agent answered" pill pointing back up at the real content.

Reported by Gary against https://atrium.garybasin.com/c/d4dbf59b-f82d-4bcb-bca9-cba174205d6e.

Root cause

ThreadCompactReply clamps its preview to three lines with line-clamp-3, which hides the remainder via overflow: hidden. But overflow only clips descendants whose containing-block chain runs through the clamping box, and the clamp was position: static.

The trigger was an agent reply containing a "Markdown feature sampler" — GFM footnotes render <section class="footnotes"> opening with <h3 class="sr-only">Footnotes</h3>, and Tailwind's sr-only is position: absolute. With every ancestor between it and the fold wrapper static, that 1×1px heading resolved its containing block to the fold (.relative.mt-2.space-y-2.pl-1) above the clamp, escaped the clip entirely, kept its static position ~3000px down inside the hidden markdown, and dragged the whole transcript's scroll height with it.

Every other absolutely positioned element in that markdown (code-block copy buttons, task-list checkbox markers) anchors to a relative wrapper inside the clamp and is correctly contained. The sr-only heading was the sole escapee.

Fix

Mark the clamp relative so it becomes the containing block and clips the heading like the rest of the collapsed content.

Verification

Measured against the live channel through the prod tunnel:

Experiment Scroll height
As shipped 7481px
position: relative on the clamps 4540px
Reverted 7481px
Re-applied 4540px
Hiding only the sr-only h3 4540px

Content height is 4497px, so 4540 is honest. Screenshot after the fix shows the channel scrolled to real messages.

  • pnpm check green (lint + migrations + typecheck + all unit suites)
  • pnpm e2e: 113 passed. Two unrelated specs (inbox.spec.ts:64, chat.spec.ts:439) failed under concurrent load from other sessions and pass isolated in 8.8s — neither touches clamps or thread replies.

Note for reviewers

This is a general hazard, not a one-off: any line-clamp wrapping rendered markdown leaks this way, and footnotes are simply the first content to ship an sr-only absolute element. jsdom has no layout, so the regression test asserts the CSS contract (the clamp carries relative) rather than the symptom; it fails without this change.

There is a parallel in-flight refactor extracting a shared ClampedBlock component. When that lands, relative needs to move onto ClampedBlock's collapsed content div — the test here queries rendered DOM, so it should fail loudly rather than let the fix be dropped silently.

…roll height

A channel whose transcript contained a long agent reply with GFM footnotes
scrolled to a blank void: the scroll container reported 7481px while the
content only laid out to 4497px, so landing at the bottom showed ~2940px of
empty space and no messages.

ThreadCompactReply clamps its preview with `line-clamp-3`, which hides the rest
with `overflow: hidden`. But overflow only clips descendants whose containing
block chain runs through the clamping box, and the clamp was `position: static`.
GFM footnotes open with `<h3 class="sr-only">Footnotes</h3>`; Tailwind's sr-only
is `position: absolute`. With every ancestor up to the fold static, that 1x1px
heading resolved its containing block to the fold wrapper ABOVE the clamp,
escaped the clip, kept its static position ~3000px down inside the hidden
markdown, and dragged the transcript's scroll height along with it.

Marking the clamp `relative` makes it the containing block, so the heading is
clipped like the rest of the collapsed content. Verified against the live
channel: toggling `position: relative` on the clamp moved the scroll height
7481 -> 4540 -> 7481 -> 4540, and hiding that single sr-only heading alone
reproduced the same 7481 -> 4540 drop.

jsdom has no layout, so the regression test asserts the CSS contract (the clamp
carries `relative`) rather than the symptom. It fails without this change.
@gbasin
gbasin enabled auto-merge July 16, 2026 19:06
@gbasin
gbasin added this pull request to the merge queue Jul 16, 2026
Merged via the queue into master with commit 3c12f51 Jul 16, 2026
16 checks passed
@gbasin
gbasin deleted the fix/clamp-abspos-scroll-leak branch July 16, 2026 19:25
gbasin added a commit that referenced this pull request Jul 16, 2026
… 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.
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