diff --git a/docs/systems/message-list.md b/docs/systems/message-list.md index 6e2b03e5..94159ca7 100644 --- a/docs/systems/message-list.md +++ b/docs/systems/message-list.md @@ -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. diff --git a/packages/web/src/components/chat/MessageList.tsx b/packages/web/src/components/chat/MessageList.tsx index 3bb4e140..89f6f1f0 100644 --- a/packages/web/src/components/chat/MessageList.tsx +++ b/packages/web/src/components/chat/MessageList.tsx @@ -61,6 +61,23 @@ export function MessageList({ channelId, jumpToMessageId, onJumpComplete }: Mess const [isAtBottom, setIsAtBottom] = useState(true); const isAtBottomRef = useRef(true); const lastProgrammaticBottomScrollRef = useRef(null); + // Smooth-scroll intent tracking. While a smooth scroll is animating toward the bottom, + // intermediate `handleScroll` measurements would otherwise see a large `distanceFromBottom` + // and flip `isAtBottomRef` to false — closing the ResizeObserver/load-handler gate so + // late-loading media (avatars, embeds, images, Spotify thumbs) growing `scrollHeight` + // mid-animation never triggers a re-pin. The smooth scroll then lands at the originally + // computed (now stale) target, leaving the user above the true bottom. + // 'bottom' = animating toward the bottom, suppress at-bottom flip during the window. + // 'message' = jump-to-message animation, do NOT suppress (the user is legitimately moving away). + // null = no animation in progress. + const smoothScrollIntentRef = useRef<'bottom' | 'message' | null>(null); + const smoothScrollDeadlineRef = useRef(0); + const smoothScrollFallbackTimerRef = useRef | null>(null); + // 5000px = same threshold as `nearBottom`. If the user wheels away mid-animation, their + // distance jumps well past this, and we let the at-bottom flag flip honestly so the + // smooth scroll's terminal frames don't fight a deliberate user gesture. + const SMOOTH_SCROLL_USER_INTENT_THRESHOLD = 5000; + const SMOOTH_SCROLL_DEADLINE_MS = 800; const [isLoadingMore, setIsLoadingMore] = useState(false); const showInitialSkeleton = useDelayedLoading(isLoading && messages.length === 0); const showPaginationSkeleton = useDelayedLoading(isLoadingMore); @@ -69,6 +86,76 @@ export function MessageList({ channelId, jumpToMessageId, onJumpComplete }: Mess const visibleMsgIdRef = useRef(null); const ackTimerRef = useRef>(); + // Final defensive pin after a bottom-bound smooth scroll completes. + // Runs from either the native `scrollend` handler (preferred) or the timeout fallback + // (browsers without scrollend support). Whichever fires first clears the intent and + // cancels its counterpart. + const finalizeBottomSmoothScroll = useCallback(() => { + if (smoothScrollIntentRef.current !== 'bottom') { + // Already cleared (e.g. user wheeled away and we let the gate flip honestly, + // or the scrollend fired for an unrelated user-driven scroll). + return; + } + const container = containerRef.current; + if (!container) { + smoothScrollIntentRef.current = null; + smoothScrollDeadlineRef.current = 0; + if (smoothScrollFallbackTimerRef.current) { + clearTimeout(smoothScrollFallbackTimerRef.current); + smoothScrollFallbackTimerRef.current = null; + } + return; + } + const distanceFromBottom = container.scrollHeight - container.scrollTop - container.clientHeight; + const userScrolledAway = distanceFromBottom >= SMOOTH_SCROLL_USER_INTENT_THRESHOLD; + if (!userScrolledAway) { + container.scrollTop = container.scrollHeight; + lastProgrammaticBottomScrollRef.current = container.scrollTop; + isAtBottomRef.current = true; + setIsAtBottom(true); + isNearBottomRef.current = true; + setIsNearBottom(true); + } + smoothScrollIntentRef.current = null; + smoothScrollDeadlineRef.current = 0; + if (smoothScrollFallbackTimerRef.current) { + clearTimeout(smoothScrollFallbackTimerRef.current); + smoothScrollFallbackTimerRef.current = null; + } + }, []); + + // Set the smooth-scroll intent and arm the final-pin path. Pick exactly one signal + // (native scrollend if supported, timeout otherwise) — the scrollend listener itself + // is registered persistently in a separate effect; here we only arm the timeout fallback + // when scrollend is unavailable so they don't double-fire. + const beginSmoothScrollIntent = useCallback((intent: 'bottom' | 'message') => { + smoothScrollIntentRef.current = intent; + smoothScrollDeadlineRef.current = performance.now() + SMOOTH_SCROLL_DEADLINE_MS; + if (smoothScrollFallbackTimerRef.current) { + clearTimeout(smoothScrollFallbackTimerRef.current); + smoothScrollFallbackTimerRef.current = null; + } + const hasScrollend = typeof window !== 'undefined' && 'onscrollend' in window; + if (intent === 'bottom' && !hasScrollend) { + smoothScrollFallbackTimerRef.current = setTimeout(() => { + smoothScrollFallbackTimerRef.current = null; + finalizeBottomSmoothScroll(); + }, SMOOTH_SCROLL_DEADLINE_MS); + } + // For 'message' intent: there is no defensive final pin (the target is not the bottom), + // but the intent ref must still be cleared once the animation ends. Use a timeout in all + // cases for 'message' — the scrollend listener also clears it, whichever fires first. + if (intent === 'message') { + smoothScrollFallbackTimerRef.current = setTimeout(() => { + smoothScrollFallbackTimerRef.current = null; + if (smoothScrollIntentRef.current === 'message') { + smoothScrollIntentRef.current = null; + smoothScrollDeadlineRef.current = 0; + } + }, SMOOTH_SCROLL_DEADLINE_MS); + } + }, [finalizeBottomSmoothScroll]); + // Permission check: DM channels always allow history; space channels check READ_MESSAGE_HISTORY const channelPerms = useSpaceStore((s) => s.channelPermissions.get(channelId)); const isDm = isDmChannel(channelId); @@ -162,10 +249,11 @@ export function MessageList({ channelId, jumpToMessageId, onJumpComplete }: Mess }); } else if (messages.length > prev && isAtBottomRef.current) { // New messages arrived while at bottom — smooth scroll + beginSmoothScrollIntent('bottom'); bottomRef.current?.scrollIntoView({ behavior: 'smooth' }); } // eslint-disable-next-line react-hooks/exhaustive-deps -- isAtBottomRef read via ref intentionally - }, [messages.length, channelId]); + }, [messages.length, channelId, beginSmoothScrollIntent]); // Auto-scroll when content height grows (embeds/images loading) while near bottom const hasMessages = messages.length > 0; @@ -202,6 +290,52 @@ export function MessageList({ channelId, jumpToMessageId, onJumpComplete }: Mess return () => content.removeEventListener('load', handleMediaLoad, true); }, [hasMessages, channelId]); + // Effect 7 — `scrollend` listener (Chrome 114+, Safari 18+). + // Fires once per smooth-scroll animation completion. When a 'bottom' intent is in + // flight, do a final defensive instant pin: layout may have grown between the + // smooth-scroll command and its terminal frame (lazy-loaded media, late embeds), + // and the smooth animation will have stopped at the originally computed target. + // For browsers without scrollend, the timeout fallback armed in + // `beginSmoothScrollIntent` handles the same final pin. + useEffect(() => { + const container = containerRef.current; + if (!container) return; + if (typeof window === 'undefined' || !('onscrollend' in window)) return; + + const handleScrollEnd = () => { + const intent = smoothScrollIntentRef.current; + if (intent === 'bottom') { + finalizeBottomSmoothScroll(); + } else if (intent === 'message') { + // No defensive pin (target is not bottom), but clear the intent so the next + // bottom-bound smooth scroll's suppression works correctly. + smoothScrollIntentRef.current = null; + smoothScrollDeadlineRef.current = 0; + if (smoothScrollFallbackTimerRef.current) { + clearTimeout(smoothScrollFallbackTimerRef.current); + smoothScrollFallbackTimerRef.current = null; + } + } + }; + + container.addEventListener('scrollend', handleScrollEnd); + return () => container.removeEventListener('scrollend', handleScrollEnd); + }, [hasMessages, channelId, finalizeBottomSmoothScroll]); + + // Cleanup: on channel switch / unmount, clear any in-flight smooth-scroll intent + // (we don't want a 'bottom' intent armed on the previous channel to suppress the + // first user scroll on the new channel). + useEffect(() => { + return () => { + smoothScrollIntentRef.current = null; + smoothScrollDeadlineRef.current = 0; + if (smoothScrollFallbackTimerRef.current) { + clearTimeout(smoothScrollFallbackTimerRef.current); + smoothScrollFallbackTimerRef.current = null; + } + }; + }, [channelId]); + // Jump-to-message: scroll to target and highlight useEffect(() => { if (!jumpToMessageId) return; @@ -209,6 +343,7 @@ export function MessageList({ channelId, jumpToMessageId, onJumpComplete }: Mess const scrollToMessage = () => { const el = document.getElementById(`msg-${jumpToMessageId}`); if (el) { + beginSmoothScrollIntent('message'); el.scrollIntoView({ behavior: 'smooth', block: 'center' }); el.classList.add('search-highlight'); setTimeout(() => el.classList.remove('search-highlight'), 2000); @@ -230,18 +365,21 @@ export function MessageList({ channelId, jumpToMessageId, onJumpComplete }: Mess }); }); }); - }, [jumpToMessageId, channelId, loadMessagesAround, onJumpComplete]); + }, [jumpToMessageId, channelId, loadMessagesAround, onJumpComplete, beginSmoothScrollIntent]); const handleScroll = useCallback(async () => { const container = containerRef.current; if (!container) return; + const sentinelBefore = lastProgrammaticBottomScrollRef.current; + const sentinelMatch = container.scrollTop === sentinelBefore; + // Sentinel: if scrollTop equals our last programmatic bottom-scroll value, this event // was queued by our own command. Layout may have grown between the command and the // event firing, but our intent is "stay at bottom" — do not let a post-growth distance // measurement flip the at-bottom flags. Re-pin defensively (content may have grown // again) and update the sentinel. See docs/systems/message-list.md (Auto-scroll model). - if (container.scrollTop === lastProgrammaticBottomScrollRef.current) { + if (sentinelMatch) { isAtBottomRef.current = true; setIsAtBottom(true); isNearBottomRef.current = true; @@ -260,11 +398,29 @@ export function MessageList({ channelId, jumpToMessageId, onJumpComplete }: Mess // Check scroll position relative to bottom const distanceFromBottom = container.scrollHeight - container.scrollTop - container.clientHeight; // "at bottom" = within 150px — used for auto-scrolling on new messages - const atBottom = distanceFromBottom < 150; - setIsAtBottom(atBottom); - isAtBottomRef.current = atBottom; + const atBottomMeasured = distanceFromBottom < 150; // "near bottom" = within 5000px — used for "Jump to Present" button visibility const nearBottom = distanceFromBottom < 5000; + + // Smooth-scroll-to-bottom suppression: while a smooth animation we initiated is + // animating toward the bottom, intermediate frames report large `distanceFromBottom`. + // Honoring those would flip `isAtBottomRef` to false and close the + // ResizeObserver/load-handler gates — preventing any late-loading media (avatars, + // embeds, attachment images, Spotify thumbs) growing scrollHeight mid-animation + // from re-pinning. The smooth scroll then lands at the originally computed (now + // stale) target. Suppress the flip ONLY for 'bottom' intent — 'message' intent + // (jump-to-message) legitimately moves the user away from bottom, so let the gate + // flip honestly there. Also let the gate flip if the user has wheeled away well + // past the near-bottom band (5000px), which signals a deliberate user gesture + // overriding our animation. + const intent = smoothScrollIntentRef.current; + const intentActive = intent === 'bottom' && performance.now() < smoothScrollDeadlineRef.current; + const userScrolledAway = distanceFromBottom >= SMOOTH_SCROLL_USER_INTENT_THRESHOLD; + const suppressBottomFlip = intentActive && !userScrolledAway; + + const atBottom = suppressBottomFlip ? true : atBottomMeasured; + setIsAtBottom(atBottom); + isAtBottomRef.current = atBottom; setIsNearBottom(nearBottom); isNearBottomRef.current = nearBottom; @@ -384,7 +540,10 @@ export function MessageList({ channelId, jumpToMessageId, onJumpComplete }: Mess {!isNearBottom && messages.length > 0 && (