devsyncwrld/backspace
1
0
Fork 0
Code Issues Pull Requests Actions 23 Packages Projects Releases Wiki Activity

fix(message-list): close smooth-scroll-to-bottom race against late-loading media

Browse Source 
Smooth scrolls toward the bottom (new-message arrival in Effect A and the
Jump-to-Present click) animate scrollTop over many frames. Each intermediate
handleScroll measurement saw a large distanceFromBottom and flipped
isAtBottomRef to false, closing the Effect B/C gates. Lazy media (avatars,
embeds, Spotify thumbs) finishing mid-animation grew scrollHeight while the
gate was closed, so the smooth scroll landed at its originally-computed
target — leaving the user above the new bottom by ~the height of what loaded.

Fix: typed smoothScrollIntentRef ('bottom' | 'message' | null) with an 800ms
deadline. handleScroll suppresses the at-bottom flip while intent is 'bottom'
and the user hasn't wheeled past the 5000px nearBottom threshold. Effect D
fires a final defensive instant pin via native scrollend (Chrome 114+,
Safari 18+) or a setTimeout(800) fallback. 'message' intent (jump-to-message
from search) does NOT suppress — the gate flips honestly so the user is left
at the targeted message.

Verified live on nova.ddns.net Orbit → general: Jump-to-Present
lands flush at bottom; new Spotify-link messages stay at bottom as embeds
arrive via WS. docs/systems/message-list.md updated.
This commit is contained in:
Jannis Braun
2026-04-27 17:51:55 +02:00
parent de2ef10129
commit b6b830568c
2 changed files with 191 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/systems/message-list.md
+25 -3
View File
@@ -15,15 +15,35 @@ The chat message list (`packages/web/src/components/chat/MessageList.tsx`) is re
Three effects cooperate. Their ordering is established by the 2026-03-25 race-fix and the 2026-04-25 sentinel addendum.
**Effect A — initial snap / restore.** Runs once when `messages.length` transitions from 0 to N for a channel. Reads `chatStore.scrollPositions.get(channelId)`. If a saved anchor exists, scrolls that message into view and computes the resulting `isAtBottomRef` from actual distance. Otherwise, sets `container.scrollTop = container.scrollHeight`, captures the post-clamp value into `lastProgrammaticBottomScrollRef`, and sets `isAtBottomRef.current = true`. On subsequent message arrivals (`messages.length > prev`), if `isAtBottomRef.current`, smooth-scrolls via `bottomRef.scrollIntoView({ behavior: 'smooth' })` — this path deliberately does *not* update the sentinel, because the smooth animation lands asynchronously across many frames and no single intermediate `scrollTop` is worth pinning to. The path is already gated on `isAtBottomRef.current`, so it cannot fire while the user is scrolled away; the rare image-load growth during a smooth scroll is tolerated.
**Effect A — initial snap / restore.** Runs once when `messages.length` transitions from 0 to N for a channel. Reads `chatStore.scrollPositions.get(channelId)`. If a saved anchor exists, scrolls that message into view and computes the resulting `isAtBottomRef` from actual distance. Otherwise, sets `container.scrollTop = container.scrollHeight`, captures the post-clamp value into `lastProgrammaticBottomScrollRef`, and sets `isAtBottomRef.current = true`. On subsequent message arrivals (`messages.length > prev`), if `isAtBottomRef.current`, sets the typed smooth-scroll intent (`'bottom'`) and smooth-scrolls via `bottomRef.scrollIntoView({ behavior: 'smooth' })`. The smooth animation lands asynchronously across many frames; the intent ref keeps the at-bottom gate open during that window so late-loading media can re-pin (see "Smooth-scroll intent" below), and Effect D delivers a final defensive instant pin when the animation completes.
**Effect B — ResizeObserver.** Observes the message-list content container. When height grows and `isAtBottomRef.current === true`, re-pins to bottom and updates the sentinel. Gated on `isAtBottomRef.current` so it cannot interfere when the user has scrolled away.
**Effect C — capture-phase `load` listener.** Catches image/iframe load completions that ResizeObserver suppresses due to its layout-loop limit. Same gate, same re-pin, same sentinel update.
**`handleScroll`.** Runs on every scroll event. **First check:** if `container.scrollTop === lastProgrammaticBottomScrollRef.current`, the event was queued by our own command — re-affirm the at-bottom flags, re-pin defensively (layout may have grown since the command), update the sentinel, and return early. Otherwise, **invalidate the sentinel immediately** (a non-matching event means the user has moved away from the position we last commanded; leaving the stale value live would let a coincidental future scroll-through of the same `scrollTop` falsely match and yank the user to bottom). Then recompute `distanceFromBottom`, update `isAtBottomRef` and `isNearBottomRef`, track `visibleMsgIdRef` (for position memory), and trigger `loadMoreMessages` when scrolled near the top.
**Effect D — `scrollend` listener (final defensive pin).** Native `scrollend` event (Chrome 114+, Safari 18+) fires once when a smooth scroll's animation completes. When `smoothScrollIntentRef.current === 'bottom'` at that moment, performs an instant `container.scrollTop = container.scrollHeight`, refreshes the sentinel, sets `isAtBottomRef = true`, and clears the intent. This is the catch-all for layout that grew during the smooth animation but after the animation's terminal target was computed. For browsers without `scrollend`, `beginSmoothScrollIntent` arms a `setTimeout(800ms)` fallback instead — exactly one of the two paths fires per intent. If the user has wheeled away mid-animation past the 5000px threshold (`SMOOTH_SCROLL_USER_INTENT_THRESHOLD`), the final pin is skipped (we honor the user's gesture).
**Invariant:** `isAtBottomRef` flips from `true` to `false` only when the user genuinely scrolls away. Layout growth, our own programmatic scrolls, and queued scroll events from those programmatic scrolls do not flip it.
**`handleScroll`.** Runs on every scroll event. **First check:** if `container.scrollTop === lastProgrammaticBottomScrollRef.current`, the event was queued by our own command — re-affirm the at-bottom flags, re-pin defensively (layout may have grown since the command), update the sentinel, and return early. Otherwise, **invalidate the sentinel immediately** (a non-matching event means the user has moved away from the position we last commanded; leaving the stale value live would let a coincidental future scroll-through of the same `scrollTop` falsely match and yank the user to bottom). Then check the smooth-scroll intent: if `intent === 'bottom'`, the deadline hasn't elapsed, AND the user hasn't wheeled away past the 5000px threshold, **suppress the at-bottom flip** — keep `isAtBottomRef = true` so Effects B/C stay open. Otherwise (no intent, expired intent, `intent === 'message'`, or user wheeled away), recompute `distanceFromBottom`, update `isAtBottomRef` and `isNearBottomRef` honestly, track `visibleMsgIdRef` (for position memory), and trigger `loadMoreMessages` when scrolled near the top. **`isNearBottomRef` is always updated honestly** even during suppression — only the at-bottom gate is held open, never the Jump-to-Present visibility.
**Invariant:** `isAtBottomRef` flips from `true` to `false` only when (a) the user genuinely scrolls away outside any active smooth-scroll-to-bottom intent, OR (b) a smooth scroll with `intent === 'message'` legitimately moves the user away from bottom. Layout growth, our own programmatic scrolls, queued scroll events from those programmatic scrolls, and intermediate frames of a smooth-scroll-to-bottom animation do not flip it.
## Smooth-scroll intent
Bottom-bound smooth scrolls (new-message arrival in Effect A, Jump-to-Present click) and jump-to-message smooth scrolls (search result click — animates to a non-bottom target) both run `scrollIntoView({behavior:'smooth'})`, which animates `scrollTop` over many frames. Each intermediate frame fires `handleScroll` with a measured `distanceFromBottom` that does *not* match the smooth animation's terminal frame. Without intent tracking, those intermediate measurements would flip `isAtBottomRef` to false, closing the Effect B/C gates so any media (avatars, embeds, attachment images, Spotify thumbs) that finishes loading mid-animation grows `scrollHeight` while the gate is closed — the smooth scroll then lands at the originally computed (now stale) target, leaving the user above the true bottom.
The fix is a typed intent ref:
| Field | Type | Set by |
|---|---|---|
| `smoothScrollIntentRef` | `'bottom' \| 'message' \| null` | `beginSmoothScrollIntent(intent, label)` |
| `smoothScrollDeadlineRef` | `number` (`performance.now()` ms) | `beginSmoothScrollIntent` (`now + 800`) |
Behavior by intent:
- **`'bottom'`**: `handleScroll` suppresses the at-bottom flip while the deadline hasn't elapsed and the user hasn't wheeled away past 5000px (`SMOOTH_SCROLL_USER_INTENT_THRESHOLD`). Effect D fires the final defensive pin via `scrollend` (or its timeout fallback). Set by: new-message smooth scroll in Effect A, Jump-to-Present `onClick`.
- **`'message'`**: NO suppression — the jump-to-message animation legitimately moves the user away from bottom and `isAtBottomRef` should flip honestly. Effect D clears the intent at scrollend (no defensive pin). Set by: `scrollToMessage` in the jump-to-message effect.
The 5000px user-intent threshold matches the `nearBottom` band: distances larger than that signal a deliberate user gesture (mouse-wheel away mid-animation), and we let the gate flip honestly so the smooth scroll's terminal frames don't fight the user.
## Position memory
@@ -54,6 +74,7 @@ Renderers that do not reserve (residual shift, sentinel-covered):
- Bare GIFs and markdown images shift on load. The sentinel keeps the auto-scroll system from being disabled by their shifts; ResizeObserver/load handlers re-pin to bottom while the user is at the bottom.
- The 150px at-bottom tolerance is generous — sending a new message while the user is reading the last few messages 100px up from the bottom yanks them down. This is intentional today; if changed, update this doc and the spec history.
- The smooth-scroll UX is preserved deliberately for both new-message arrival and Jump-to-Present per UX call. The 2026-04-27 fix (smooth-scroll intent + scrollend final pin) closes the residual above-bottom-landing race without removing the animation.
## Out of scope (deferred)
@@ -69,3 +90,4 @@ These items were considered and rejected for the 2026-04-25 work; they live here
- 2026-03-25 — `chat-scroll-race-fix` spec: removed `isAtBottom` from Effect A's deps, gated Effects B and C on `isAtBottomRef`, set the ref after initial snap. Shipped.
- 2026-03-25 — `embed-dimension-reservation` spec: server probes image embeds for dimensions; client renderers reserve via `aspect-ratio`. Server side shipped. Client side partly shipped, then reverted in `0c84029` because the 4/3 fallback caused dark letterbox bars.
- 2026-04-25 — `message-list-scroll-completion-and-addendum` spec: restored the *known-dimension-only* branch of the client reservation in `ImageEmbed.tsx`; added the `lastProgrammaticBottomScrollRef` sentinel to close the residual `handleScroll` race; created this file.
- 2026-04-27 — smooth-scroll intent + `scrollend` final pin: typed `smoothScrollIntentRef` (`'bottom' | 'message' | null`) with an 800ms deadline. `handleScroll` suppresses the at-bottom flip while a `'bottom'` intent is active and the user hasn't wheeled past the 5000px threshold; the gate stays open so Effects B/C re-pin to bottom as media loads mid-animation. Effect D fires a final defensive instant pin via the native `scrollend` event (Chrome 114+, Safari 18+) or a `setTimeout(800)` fallback. `'message'` intent (jump-to-message from search) does NOT suppress — the gate flips honestly so the user is left at the targeted message. Closes the "Jump to Present doesn't fully reach the bottom" residue reproducible on `nova.ddns.net` Orbit → general. UX call: smooth-scroll animation preserved, not replaced with an instant jump.