feat(MessageList,ChatContainer): allow disabling auto-scroll - #239
Draft
chelentos wants to merge 8 commits into
Draft
feat(MessageList,ChatContainer): allow disabling auto-scroll#239chelentos wants to merge 8 commits into
chelentos wants to merge 8 commits into
Conversation
Adds an autoScroll option (default true) to useSmartScroll and useVirtualStickToBottom so consumers can disable automatic pinning to the bottom while keeping user-scroll tracking and the imperative scrollToBottom / prepend-restore paths working.
…guard Address review findings on the auto-scroll gating tests: - assert the behavior argument (instant vs smooth) on the four positive useSmartScroll cases, so a dropped 'smooth' is caught; - add a disabled-to-enabled toggle test per hook (holding every other prop fixed) to prove autoScroll is read via ref rather than through an effect dependency array.
useSmartScroll has a dedicated effect that scrolls to bottom on any status transition, gated by autoScroll like the other four triggers. The prior autoScroll docs (README, JSDoc, HOOKS.md) enumerated mount / new message / streaming / viewport resize but omitted it.
…ocument re-enable timing MessageList/README.md and HOOKS.md claimed a consumer could disable auto-scroll and still force-scroll via scrollToBottom; it still early-returns under the user-scrolled-up guard, and MessageList/useVirtualStickToBottom never expose it at all. Also note in the autoScroll JSDoc (MessageList, ChatContainer) that re-enabling only takes effect at the next scroll trigger, not immediately - otherwise a consumer wiring a "follow along" toggle would see it do nothing on an idle conversation. Shortened the ChatContainer README's autoScroll row so it no longer forces prettier to re-align the whole props table, and moved the detail into a new Auto-scroll section with a usage example.
…ions
useVirtualStickToBottom's pinToBottom re-applies scrollToRow across several animation frames, with
its own autoScrollRef re-check inside step(). No existing test advanced requestAnimationFrame, so
that per-frame guard had zero coverage - deleting it left all tests green. Add a test that captures
the scheduled rAF callback, disables autoScroll mid-flight, and invokes the callback directly to
prove the guard is load-bearing (verified: fails when the guard is removed, passes when restored).
Also tighten the four positive pin assertions from bare toHaveBeenCalled() to the exact
{index, align, behavior} argument object, matching the stricter style already used in the
useSmartScroll tests.
Contributor
|
🚀 Prerelease version published! Install this PR version: npm i --save-dev @gravity-ui/aikit@2.19.2-beta.8bf8370c62216e1d8db334845ec2cb7fba2eef05.0 |
|
Preview is ready. |
Contributor
|
🎭 Component Tests Report is ready. |
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
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.
Summary
Adds a single
autoScroll?: booleanprop (defaulttrue) toMessageListandChatContainerthat switches off automatic scroll-to-bottom in the message list.Omitting the prop is a strict no-op. Receipts, since that is the only question that matters for a published component library:
useVirtualStickToBottomthe whole gate is!autoScrollRef.current ||at the head of the two guards already insidepinToBottom, so with the default it evaluates tofalse || <original condition>.behaviorargument is unchanged —'instant'on mount / resize / mutation,'smooth'onstatusandmessagesCount, andscrollToRow's{align: 'end', behavior: 'instant'}in the virtualized path. The tests assert the argument objects, not just that a call happened, so a future swap would fail.autoScrollToBottom, which is auseCallbackover auseCallbackwith[]deps — referentially stable for the component's lifetime, so no effect can re-fire that did not before. There is a test per hook that fails if the flag is ever added to a dependency array.Motivation
Both scroll hooks pin the viewport to the bottom on five triggers: first mount, a message being appended, a
statustransition, streaming content growth, and a viewport resize. They already yield once the user scrolls up — but that guard is only armed by a real scroll event.So the common case is not covered: a reader who never touches the wheel while a long answer streams in still gets pulled to the bottom as the content grows under them. Today there is no way out short of forking
MessageList.API
autoScrollis a top-levelChatContainerprop and is excluded fromMessageListConfig, so it cannot be set in two places — matching howshowActionsOnHover,transformOptionsandmdxPropsare already handled.Why a boolean rather than per-trigger control
A per-trigger object (
{initial, onNewMessage, onStreaming}) was implemented first and deliberately dropped. Separating the triggers turned out to create three distinct bug classes, all of them consequences of separability rather than of implementation quality:initialandonNewMessagecompete for the same moment whenever history loads asynchronously — the list mounts empty, so the initial pin has nothing to scroll to, and the arriving history gets claimed byonNewMessage.{initial: true, onNewMessage: false}would open at the top;{initial: false}would scroll anyway.ResizeObserver.observe()delivers a callback immediately with the current size. That is not a resize, so it has to be suppressed too — but only when the resize trigger is independently switchable.A single flag read at fire time is stateless and has none of them. Widening
booleantoboolean | AutoScrollConfiglater is a non-breaking type extension, so nothing is foreclosed if a concrete per-trigger need turns up.Behavior worth knowing
autoScroll={false}a chat with history opens scrolled to the top, not at the last message. The switch is all-or-nothing.useScrollPreservation, and the prepend/anti-jump viewport preservation in the virtualized list — that last one is anti-jump behavior, not auto-scroll.Testing
21 new unit tests across the two hooks (
npm run test:unit: 302 passing). Both negative-test groups were mutation-checked — the gate was temporarily removed to confirm the tests actually fail without it, including the per-framestep()re-check in the virtualized hook, which is only reachable by advancingrequestAnimationFrame.No Playwright snapshots: the feature is behavioural and introduces no new static visual state.
Two things reviewers may trip over
docs/HOOKS.mdcorrection (drive-by). TheuseSmartScrollblock documented a signature that has never existed in this repo — options{threshold, enabled}returning{ref, isAtBottom, scrollToBottom}. The real hook takes{isStreaming, messagesCount, status}and returns{containerRef, scrollToBottom}. Corrected here since the block had to be touched anyway. The adjacentuseScrollPreservationblock is wrong in the same way; left alone as out of scope, happy to fix it in a follow-up.llms-full.txtcontains one hunk unrelated to this change. The file is generated bynpm run generate:llmsfrom the root README anddocs/*.md, and the checked-in copy was stale: regenerating it after thedocs/HOOKS.mdedit also picked up a CSS-variable row sourced fromdocs/THEMING.md.THEMING.mditself is untouched by this branch.🤖 Generated with Claude Code