Chat: fix lost stick-to-bottom on card-heavy answers (/getting-started)#1504
Closed
michaelassraf wants to merge 2 commits into
Closed
Chat: fix lost stick-to-bottom on card-heavy answers (/getting-started)#1504michaelassraf wants to merge 2 commits into
michaelassraf wants to merge 2 commits into
Conversation
…/getting-started) Co-Authored-By: Claude Opus 4.8 <[email protected]>
Co-Authored-By: Claude Opus 4.8 <[email protected]>
📝 WalkthroughWalkthrough
ChangesChat bottom-follow behavior
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ChatMessageList
participant ResizeObserver
participant ScrollContainer
User->>ChatMessageList: send turn or click jump-to-latest
ChatMessageList->>ScrollContainer: scrollToBottom
ResizeObserver->>ChatMessageList: report content or scroller resize
ChatMessageList->>ScrollContainer: reassert scrollToBottom
User->>ChatMessageList: upward wheel or scroll gesture
ChatMessageList->>ChatMessageList: clear followBottomRef
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@openframe-frontend-core/src/components/chat/chat-message-list.tsx`:
- Around line 278-367: The follow-bottom useEffect must rerun when loading
finishes. Add isLoading to the dependency array of the effect containing
reassert, ResizeObserver, and scroll listeners, preserving the existing
early-return behavior and setup once the real list refs are available.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: acc643b0-fcf7-43bf-974c-672dd5dde9dc
📒 Files selected for processing (2)
openframe-frontend-core/src/components/chat/__tests__/chat-message-list.test.tsxopenframe-frontend-core/src/components/chat/chat-message-list.tsx
Comment on lines
+278
to
+367
| useEffect(() => { | ||
| if (!autoScroll) return | ||
| const scroller = scrollRef.current | ||
| const content = contentRef.current | ||
| if (!scroller || !content) return | ||
|
|
||
| // Re-assert on ANY geometry change. Deliberately WITHOUT | ||
| // `ignoreEscapes`: the library must stay free to hand control back | ||
| // on a real gesture mid-animation. Re-asserting is precisely what | ||
| // re-sets its `isAtBottom`, so a silently-lost lock recovers on the | ||
| // next resize instead of staying broken for the rest of the turn. | ||
| // Cheap to call repeatedly — the library returns the in-flight | ||
| // promise when an animation with the same behaviour is running. | ||
| const measure = () => { | ||
| const distance = scroller.scrollHeight - scroller.scrollTop - scroller.clientHeight | ||
| setAtBottom(distance <= BOTTOM_THRESHOLD_PX) | ||
| } | ||
|
|
||
| const reassert = () => { | ||
| if (followBottomRef.current) void scrollToBottom({ animation: 'smooth' }) | ||
| // Measured on every geometry change, follow armed or not: a | ||
| // sibling growing below the scroller moves the bottom without | ||
| // any scroll event, and the button has to notice. | ||
| measure() | ||
| } | ||
|
|
||
| const ro = new ResizeObserver(reassert) | ||
| ro.observe(content) // tokens, entity cards, images | ||
| ro.observe(scroller) // streaming loader / source chips below it | ||
|
|
||
| // --- user-intent disarm --------------------------------------- | ||
| // Only a deliberate move AWAY from the bottom releases the lock. | ||
| // Resize-derived scroll events are NOT gestures, which is exactly | ||
| // the distinction the library's heuristic gets wrong. | ||
| const disarm = () => { | ||
| followBottomRef.current = false | ||
| } | ||
| const onWheel = (e: WheelEvent) => { | ||
| if (e.deltaY < 0) disarm() | ||
| } | ||
| const onKeyDown = (e: KeyboardEvent) => { | ||
| if (e.key === 'ArrowUp' || e.key === 'PageUp' || e.key === 'Home') disarm() | ||
| } | ||
| let touchY: number | null = null | ||
| const onTouchStart = (e: TouchEvent) => { | ||
| touchY = e.touches[0]?.clientY ?? null | ||
| } | ||
| const onTouchMove = (e: TouchEvent) => { | ||
| const y = e.touches[0]?.clientY | ||
| if (y == null || touchY == null) return | ||
| if (y > touchY) disarm() // finger travels DOWN → content moves up | ||
| touchY = y | ||
| } | ||
| // Scrollbar drag emits neither wheel nor touch — a scrollTop | ||
| // DECREASE while the pointer is held down is the same intent. | ||
| let pointerDown = false | ||
| let lastScrollTop = scroller.scrollTop | ||
| const onPointerDown = () => { | ||
| pointerDown = true | ||
| } | ||
| const onPointerUp = () => { | ||
| pointerDown = false | ||
| } | ||
| const onScroll = () => { | ||
| const st = scroller.scrollTop | ||
| if (pointerDown && st < lastScrollTop) disarm() | ||
| lastScrollTop = st | ||
| measure() | ||
| } | ||
| measure() | ||
|
|
||
| scroller.addEventListener('wheel', onWheel, { passive: true }) | ||
| scroller.addEventListener('keydown', onKeyDown) | ||
| scroller.addEventListener('touchstart', onTouchStart, { passive: true }) | ||
| scroller.addEventListener('touchmove', onTouchMove, { passive: true }) | ||
| scroller.addEventListener('pointerdown', onPointerDown, { passive: true }) | ||
| scroller.addEventListener('scroll', onScroll, { passive: true }) | ||
| window.addEventListener('pointerup', onPointerUp, { passive: true }) | ||
|
|
||
| return () => { | ||
| ro.disconnect() | ||
| scroller.removeEventListener('wheel', onWheel) | ||
| scroller.removeEventListener('keydown', onKeyDown) | ||
| scroller.removeEventListener('touchstart', onTouchStart) | ||
| scroller.removeEventListener('touchmove', onTouchMove) | ||
| scroller.removeEventListener('pointerdown', onPointerDown) | ||
| scroller.removeEventListener('scroll', onScroll) | ||
| window.removeEventListener('pointerup', onPointerUp) | ||
| } | ||
| }, [autoScroll, scrollRef, contentRef, scrollToBottom]) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP --type=tsx -C3 '<ChatMessageList\b' | rg -nC3 'isLoading'
rg -nP -C3 'isLoading' openframe-frontend-core/src/components/chat/chat-message-list.tsxRepository: flamingo-stack/openframe-oss-lib
Length of output: 667
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="openframe-frontend-core/src/components/chat/chat-message-list.tsx"
echo "== file size =="
wc -l "$file"
echo
echo "== hook and loading branch context =="
sed -n '1,220p' "$file" | cat -n | sed -n '1,220p'
echo
sed -n '220,420p' "$file" | cat -n | sed -n '220,420p'
echo
sed -n '540,640p' "$file" | cat -n | sed -n '540,640p'Repository: flamingo-stack/openframe-oss-lib
Length of output: 11762
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="openframe-frontend-core/src/components/chat/chat-message-list.tsx"
echo "== lines 250-390 =="
sed -n '250,390p' "$file" | cat -n
echo
echo "== search for effects depending on isLoading / loading transition =="
rg -n "isLoading|autoScroll|scrollToBottom|followBottomRef|atBottom|ResizeObserver|useEffect\(" "$file"Repository: flamingo-stack/openframe-oss-lib
Length of output: 10628
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="openframe-frontend-core/src/components/chat/chat-message-list.tsx"
sed -n '460,620p' "$file" | cat -nRepository: flamingo-stack/openframe-oss-lib
Length of output: 8629
Add isLoading to the follow-effect deps
On the loading render, this effect returns before scrollRef.current/contentRef.current exist, and it won’t run again when the skeleton swaps to the real list because isLoading is missing from the dependency array. That leaves the ResizeObserver and scroll listeners unmounted on the initial load path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@openframe-frontend-core/src/components/chat/chat-message-list.tsx` around
lines 278 - 367, The follow-bottom useEffect must rerun when loading finishes.
Add isLoading to the dependency array of the effect containing reassert,
ResizeObserver, and scroll listeners, preserving the existing early-return
behavior and setup once the real list refs are available.
This was referenced Jul 20, 2026
Contributor
Author
|
Superseded — absorbed into #1506 rather than merged separately. Both commits were cherry-picked clean onto
No conflict with that branch's Three fixes were made on top while absorbing it:
Also worth recording: the reported symptom on Still needs the real-device check this PR's description flagged, once the lib release lands. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Running a card-heavy slash command in the Mingo panel (reported on
/getting-started) loses stick-to-bottom: the thread stops following and the rest of the answer lands below the fold.Measured live: the scroller ended 4081px, then 6354px, then 18619px above the bottom across three runs.
Root cause
ChatMessageListdelegates follow-the-bottom entirely touse-stick-to-bottom, which follows only while its internalisAtBottomflag is true. This list's layout loses that flag silently in two ways:contentRef, which lives inside the scroller. The streaming loader unmounting and the host's source-chip row mounting are siblings below the scroller, so they change itsclientHeight, moving the bottom, with no event the library can observe.scrollTopdown, and the library's scroll handler reads that as a user scroll-up and escapes the lock permanently. ItsresizeDifferenceguard is asetTimeout(..., 1)race, hence the intermittency./getting-startedhits both hard:presentation: 'list'produces a dozen[card://onboarding_guide:...]markers, i.e. fetch-mode cards with 1200x630 covers that keep resolving for seconds after the stream ends. Verified from the raw SSE frames that this turn carries noscrollAnchor, so the deliberate top-anchor path is not involved.Fix
The list now owns the intent instead of trusting the library's flag:
followBottomRef.scrollToBottom. That call also re-sets the library'sisAtBottom, so a lost lock self-heals on the next resize rather than staying broken for the rest of the turn.scrollTopdecrease while the pointer is held (scrollbar drag). Resize-derived scrolls never count, which is exactly the distinction the library gets wrong.scrollAnchor: 'top'turn disarms explicitly, since it deliberately parks at the message top.Deletes the old per-
<img>load-handler hack: same defect class, and the ResizeObserver covers every growth source, not just images present at send time.Testing
chat-message-list.test.tsx: the 4 existing plus 6 new regressions covering late async growth, gesture release, downward-wheel non-release,autoScroll={false}, and the top-anchor carve-out.tsc --noEmitclean for the changed file.npm installfirst died on a transientETARGETfor@radix-ui/[email protected]; the retry is running and a live/getting-startedcheck plus a device pass are still outstanding before merge.Follow-up
Hub pins an exact lib version, so shipping needs a lib release and a hub pin bump.
Generated with Claude Code
Update: industry-standard audit
Re-checked this against what shipping AI chats do in 2026.
Validated. The container-resize blind spot is a known, still-open upstream bug: use-stick-to-bottom#40, "scroll position drifts when scroll container height changes (flex sibling resize)" — "The library's ResizeObserver only monitors the content element, not the scroll container itself... Proposed fix: implement a second ResizeObserver attached to the scroll container." We resolve to 1.1.6 (latest), so it is unfixed upstream, and this PR implements precisely the remedy the issue proposes. Vercel AI Elements'
<Conversation>still uses the same library, so the reference implementation shares the defect.Not claimed as standard. The "intent lock armed on send, released only on a real gesture" is not a shipped primitive anywhere reviewed (assistant-ui, LibreChat, Open WebUI, AI Elements). It is a deliberate local choice: resize-derived scrolls are not gestures, which is exactly the distinction the library's heuristic gets wrong.
Known divergence, accepted. The market leaders (ChatGPT, Claude.ai, Gemini) anchor the sent message to the TOP and do not chase the answer; assistant-ui codifies this as
turnAnchor="top"(which flipsautoScrolltofalse). This repo already owns that primitive asscrollAnchor: 'top'indisplay-action-response.ts, applied to display-action answers only. Extending it to slash-command list answers was considered and explicitly declined for now: bottom-follow is the chosen behavior here. Revisit if reading long listings from the end proves annoying.Added in this PR: the jump-to-bottom button, which AI Elements ships as
ConversationScrollButtonand which is the practical escape hatch for WCAG 2.2.2 / F16. Its visibility is derived from real geometry rather than the library'sisAtBottom, because a staletrueon that flag would hide the button in exactly the situation it exists for. Clicking it re-arms the follow lock.Deferred (not in this PR):
prefers-reduced-motiongating on the spring re-assert (this repo already honors the query inuse-scripted-stream.ts:25), and anaria-liveregion for the answer body (today only the streaming loader has one).Summary by CodeRabbit
New Features
Bug Fixes