mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-27 12:13:30 +00:00
🪂 fix: Land Navigation Auto-Scroll on the Rendered Thread (#15014)
The "auto scroll to latest message" setting stopped taking readers to the newest message when opening a conversation, most visibly on long threads. `useMessageScrolling` fired its landing on the conversation id alone. That id reaches the hook a commit or more before the tree does, so `scrollIntoView` ran against the OUTGOING conversation's rows: it scrolled that thread to its end, and — having no dependency on the tree — never ran again once the requested thread mounted. The reader was left at whatever offset the old thread's bottom happened to be, which on a long thread is the top. Key the landing on the conversation that owns the RENDERED rows instead, using the same `messagesTree[0].conversationId` fallback `MessagesView` already uses to key the mount window, and land once per conversation so the tree identities a stream mints cannot haul back a reader who scrolled away. This is independent of the progressive row mounting: that window only ever grows upward from the newest row, so the end of the mounted content is already the end of the thread, and the landing needs no full mount to be correct. Measured against the real client (react-scan render tallies over a 10-message to 120-message navigation), render counts are unchanged at ~16k and the thread still mounts progressively; distance from the bottom on arrival goes 841px to 0. With progressive mounting disabled the same navigation landed 15421px from the bottom, confirming the anchoring was masking this rather than causing it. Also moves the `autoScroll` setting from Recoil to Jotai, keeping the same `autoScroll` localStorage key so a stored preference survives, and matching the `showThinking`/`smoothStreaming` atoms already served through `ToggleSwitch`. Claude-Session: https://claude.ai/code/session_01BDQSLdbwvtSqCmQSw7Nz91 Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
parent
7c71d6dc1a
commit
c8953b8f32
6 changed files with 178 additions and 10 deletions
|
|
@ -9,6 +9,7 @@ import { RowMountProvider, useProgressiveRowMount } from '~/hooks/Messages';
|
|||
import { MessagesViewProvider, useChatContext } from '~/Providers';
|
||||
import ScrollToBottom from '~/components/Messages/ScrollToBottom';
|
||||
import { steerOverlayHeightFamily } from '~/store/steer';
|
||||
import { autoScrollAtom } from '~/store/autoScroll';
|
||||
import { fontSizeAtom } from '~/store/fontSize';
|
||||
import MultiMessage from './MultiMessage';
|
||||
import MessageNav from './MessageNav';
|
||||
|
|
@ -117,7 +118,7 @@ function MessagesViewContent({
|
|||
|
||||
const { index, latestMessageDepth } = useChatContext();
|
||||
const isSubmitting = useRecoilValue(store.isSubmittingFamily(index));
|
||||
const autoScroll = useRecoilValue(store.autoScroll);
|
||||
const autoScroll = useAtomValue(autoScrollAtom);
|
||||
/** Re-arm from the conversation that owns the RENDERED tree: the Recoil
|
||||
* conversation id lags the route during warm-cache navigation, and keying
|
||||
* off it would first mount the new tree unwindowed, then narrow it after
|
||||
|
|
|
|||
|
|
@ -43,6 +43,7 @@ import SharedLinks from '../SettingsTabs/Data/SharedLinks';
|
|||
import ImageResize from '../SettingsTabs/Chat/ImageResize';
|
||||
import { showThinkingAtom } from '~/store/showThinking';
|
||||
import ProviderKeys from '../SettingsTabs/ProviderKeys';
|
||||
import { autoScrollAtom } from '~/store/autoScroll';
|
||||
import Avatar from '../SettingsTabs/Account/Avatar';
|
||||
import About from '../SettingsTabs/About/About';
|
||||
import ApiKeys from '../SettingsTabs/ApiKeys';
|
||||
|
|
@ -352,7 +353,7 @@ export const registry: SettingEntry[] = [
|
|||
section: 'conversations',
|
||||
labelKey: 'com_nav_auto_scroll',
|
||||
Component: toggleControl({
|
||||
stateAtom: store.autoScroll,
|
||||
stateAtom: autoScrollAtom,
|
||||
localizationKey: 'com_nav_auto_scroll',
|
||||
switchId: 'autoScroll',
|
||||
}),
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
import React from 'react';
|
||||
import { RecoilRoot } from 'recoil';
|
||||
import { Provider, createStore } from 'jotai';
|
||||
import { act, fireEvent, render, screen } from '@testing-library/react';
|
||||
import type { TConversation, TMessage } from 'librechat-data-provider';
|
||||
import {
|
||||
|
|
@ -35,6 +36,7 @@ jest.mock('../messageLayout', () => ({
|
|||
|
||||
import useMessageScrolling from '../useMessageScrolling';
|
||||
import { reconcileMessageContentLayout } from '../messageLayout';
|
||||
import { autoScrollAtom } from '~/store/autoScroll';
|
||||
|
||||
const mockReconcileMessageContentLayout = reconcileMessageContentLayout as jest.Mock;
|
||||
|
||||
|
|
@ -588,3 +590,111 @@ describe('useMessageScrolling resize reconciliation', () => {
|
|||
expect(mockScrollToBottom).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('useMessageScrolling navigation landing', () => {
|
||||
const treeFor = (conversationId: string): TMessage[] => [
|
||||
{ ...message, conversationId } as TMessage,
|
||||
];
|
||||
|
||||
const harness = (
|
||||
store: ReturnType<typeof createStore>,
|
||||
conversationId: string,
|
||||
messagesTree: TMessage[] | null,
|
||||
) => (
|
||||
<RecoilRoot>
|
||||
<Provider store={store}>
|
||||
<MessagesViewContext.Provider
|
||||
value={createContextValue({
|
||||
isSubmitting: false,
|
||||
conversation: { ...conversation, conversationId } as TConversation,
|
||||
conversationId,
|
||||
})}
|
||||
>
|
||||
<ScrollingHarness messagesTree={messagesTree} />
|
||||
</MessagesViewContext.Provider>
|
||||
</Provider>
|
||||
</RecoilRoot>
|
||||
);
|
||||
|
||||
function renderLanding(
|
||||
conversationId: string,
|
||||
messagesTree: TMessage[] | null,
|
||||
autoScroll = true,
|
||||
) {
|
||||
const store = createStore();
|
||||
store.set(autoScrollAtom, autoScroll);
|
||||
const view = render(harness(store, conversationId, messagesTree));
|
||||
return {
|
||||
...view,
|
||||
rerenderWith: (nextId: string, nextTree: TMessage[] | null) =>
|
||||
view.rerender(harness(store, nextId, nextTree)),
|
||||
};
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
MockResizeObserver.reset();
|
||||
MockIntersectionObserver.reset();
|
||||
mockScrollToBottom.mockClear();
|
||||
mockScrollToBottom.cancel.mockClear();
|
||||
(global as unknown as { ResizeObserver: typeof MockResizeObserver }).ResizeObserver =
|
||||
MockResizeObserver;
|
||||
(
|
||||
global as unknown as { IntersectionObserver: typeof MockIntersectionObserver }
|
||||
).IntersectionObserver = MockIntersectionObserver;
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
(global as unknown as { ResizeObserver: typeof ResizeObserver | undefined }).ResizeObserver =
|
||||
originalResizeObserver;
|
||||
(
|
||||
global as unknown as { IntersectionObserver: typeof IntersectionObserver | undefined }
|
||||
).IntersectionObserver = originalIntersectionObserver;
|
||||
});
|
||||
|
||||
it('waits for the opened conversation to own the rendered rows', () => {
|
||||
/** The id reaches the hook commits before the tree does. Landing on the
|
||||
* outgoing thread spends the one scroll this navigation gets, and leaves
|
||||
* the reader at the top of the thread they actually asked for. */
|
||||
renderLanding('conversation-2', treeFor('conversation-1'));
|
||||
|
||||
expect(mockScrollToBottom).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('lands once the rendered tree names the opened conversation', () => {
|
||||
const { rerenderWith } = renderLanding('conversation-2', treeFor('conversation-1'));
|
||||
expect(mockScrollToBottom).not.toHaveBeenCalled();
|
||||
|
||||
rerenderWith('conversation-2', treeFor('conversation-2'));
|
||||
|
||||
expect(mockScrollToBottom).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('does not re-land on the tree identities a stream mints', () => {
|
||||
const { rerenderWith } = renderLanding('conversation-2', treeFor('conversation-2'));
|
||||
expect(mockScrollToBottom).toHaveBeenCalledTimes(1);
|
||||
|
||||
/** Every delta writes a fresh array; re-landing on those would haul back a
|
||||
* reader who deliberately scrolled up. */
|
||||
rerenderWith('conversation-2', treeFor('conversation-2'));
|
||||
rerenderWith('conversation-2', treeFor('conversation-2'));
|
||||
|
||||
expect(mockScrollToBottom).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('lands again for the next conversation opened', () => {
|
||||
const { rerenderWith } = renderLanding('conversation-2', treeFor('conversation-2'));
|
||||
expect(mockScrollToBottom).toHaveBeenCalledTimes(1);
|
||||
|
||||
rerenderWith('conversation-3', treeFor('conversation-2'));
|
||||
expect(mockScrollToBottom).toHaveBeenCalledTimes(1);
|
||||
|
||||
rerenderWith('conversation-3', treeFor('conversation-3'));
|
||||
expect(mockScrollToBottom).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it('stays put when the setting is off', () => {
|
||||
renderLanding('conversation-2', treeFor('conversation-2'), false);
|
||||
|
||||
expect(mockScrollToBottom).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,11 +1,11 @@
|
|||
import { useRef, useCallback, useEffect } from 'react';
|
||||
import { useRecoilValue } from 'recoil';
|
||||
import { useAtomValue } from 'jotai';
|
||||
import { Constants } from 'librechat-data-provider';
|
||||
import type { TMessage } from 'librechat-data-provider';
|
||||
import { useMessagesConversation, useMessagesSubmission } from '~/Providers';
|
||||
import { reconcileMessageContentLayout } from './messageLayout';
|
||||
import useScrollToRef from '~/hooks/useScrollToRef';
|
||||
import store from '~/store';
|
||||
import { autoScrollAtom } from '~/store/autoScroll';
|
||||
|
||||
const resizeFollowThreshold = 120;
|
||||
|
||||
|
|
@ -26,7 +26,7 @@ const prefersReducedMotion = () =>
|
|||
window.matchMedia('(prefers-reduced-motion: reduce)').matches;
|
||||
|
||||
export default function useMessageScrolling(messagesTree?: TMessage[] | null) {
|
||||
const autoScroll = useRecoilValue(store.autoScroll);
|
||||
const autoScroll = useAtomValue(autoScrollAtom);
|
||||
|
||||
const scrollableRef = useRef<HTMLDivElement | null>(null);
|
||||
const contentRef = useRef<HTMLDivElement | null>(null);
|
||||
|
|
@ -48,6 +48,9 @@ export default function useMessageScrolling(messagesTree?: TMessage[] | null) {
|
|||
const lastScrollTopRef = useRef(-1);
|
||||
const wasSubmittingRef = useRef(false);
|
||||
const suppressNextResizeFollowRef = useRef(false);
|
||||
/** The conversation whose newest message has already been landed on, so the
|
||||
* stream deltas that follow cannot re-take a reader who scrolled away. */
|
||||
const landedConversationRef = useRef<string | null>(null);
|
||||
const { conversation, conversationId } = useMessagesConversation();
|
||||
const { setAbortScroll, isSubmitting, abortScroll } = useMessagesSubmission();
|
||||
|
||||
|
|
@ -411,15 +414,59 @@ export default function useMessageScrolling(messagesTree?: TMessage[] | null) {
|
|||
};
|
||||
}, [isSubmitting, messagesTree, scrollToBottom, abortScroll, followBottom]);
|
||||
|
||||
/**
|
||||
* Land on the newest message when a conversation is opened.
|
||||
*
|
||||
* Keyed on the conversation whose rows are actually MOUNTED, not on the id
|
||||
* alone. The id reaches this hook a commit or more before the tree does, and
|
||||
* firing on it scrolled the outgoing thread to its end and then never ran
|
||||
* again — the reader was left wherever that put them, which on a long thread
|
||||
* is the top. Waiting for the rendered tree to name the same conversation
|
||||
* costs nothing and is the only moment `messages-end` is where the reader
|
||||
* expects it.
|
||||
*
|
||||
* One landing per conversation: the tree's identity changes on every stream
|
||||
* delta and cache reconcile, and re-running on those would drag a reader who
|
||||
* has scrolled away back to the bottom.
|
||||
*/
|
||||
useEffect(() => {
|
||||
if (!messagesEndRef.current || !scrollableRef.current) {
|
||||
if (!autoScroll) {
|
||||
/** Switching the setting off releases the landing, so switching it back
|
||||
* on while the same conversation is open honours it again. */
|
||||
landedConversationRef.current = null;
|
||||
return;
|
||||
}
|
||||
|
||||
if (scrollToBottom && autoScroll && conversationId !== Constants.NEW_CONVO) {
|
||||
scrollToBottom();
|
||||
if (conversationId == null || conversationId === Constants.NEW_CONVO) {
|
||||
return;
|
||||
}
|
||||
}, [autoScroll, conversationId, scrollToBottom]);
|
||||
|
||||
if (!scrollToBottom || !messagesEndRef.current || !scrollableRef.current) {
|
||||
return;
|
||||
}
|
||||
|
||||
/** Rows are gated by the progressive mount window during a first commit,
|
||||
* but that window only ever grows UPWARD from the newest row, so the end
|
||||
* of the mounted content is already the end of the thread. */
|
||||
if (!messagesTree?.length) {
|
||||
return;
|
||||
}
|
||||
|
||||
/** Same fallback `MessagesView` uses to key the mount window: a tree whose
|
||||
* rows carry no conversation id is taken to be this conversation's, so a
|
||||
* locally-built thread still lands instead of waiting forever. */
|
||||
const renderedConversationId = messagesTree[0]?.conversationId ?? conversationId;
|
||||
if (renderedConversationId !== conversationId) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (landedConversationRef.current === conversationId) {
|
||||
return;
|
||||
}
|
||||
|
||||
landedConversationRef.current = conversationId;
|
||||
scrollToBottom();
|
||||
}, [autoScroll, conversationId, messagesTree, scrollToBottom]);
|
||||
|
||||
return {
|
||||
conversation,
|
||||
|
|
|
|||
10
client/src/store/autoScroll.ts
Normal file
10
client/src/store/autoScroll.ts
Normal file
|
|
@ -0,0 +1,10 @@
|
|||
import { createStorageAtom } from './jotai-utils';
|
||||
|
||||
const DEFAULT_AUTO_SCROLL = false;
|
||||
|
||||
/**
|
||||
* Whether opening a conversation lands the reader on its newest message.
|
||||
* Persisted under the same `autoScroll` key the Recoil atom used, so a stored
|
||||
* preference survives the migration.
|
||||
*/
|
||||
export const autoScrollAtom = createStorageAtom<boolean>('autoScroll', DEFAULT_AUTO_SCROLL);
|
||||
|
|
@ -41,7 +41,6 @@ function isSmallViewport(): boolean {
|
|||
|
||||
const localStorageAtoms = {
|
||||
// General settings
|
||||
autoScroll: atomWithLocalStorage('autoScroll', false),
|
||||
sidebarExpanded: atomWithLocalStorage(
|
||||
'unifiedSidebarExpanded',
|
||||
!isSmallViewport(),
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue