diff --git a/client/src/components/Chat/Messages/MessagesView.tsx b/client/src/components/Chat/Messages/MessagesView.tsx index ed76987ccd..8166a5d9ce 100644 --- a/client/src/components/Chat/Messages/MessagesView.tsx +++ b/client/src/components/Chat/Messages/MessagesView.tsx @@ -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 diff --git a/client/src/components/Nav/Settings/registry.tsx b/client/src/components/Nav/Settings/registry.tsx index 8a81a7f40d..2c08d8fb74 100644 --- a/client/src/components/Nav/Settings/registry.tsx +++ b/client/src/components/Nav/Settings/registry.tsx @@ -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', }), diff --git a/client/src/hooks/Messages/__tests__/useMessageScrolling.spec.tsx b/client/src/hooks/Messages/__tests__/useMessageScrolling.spec.tsx index cc0771b60b..0330e1583c 100644 --- a/client/src/hooks/Messages/__tests__/useMessageScrolling.spec.tsx +++ b/client/src/hooks/Messages/__tests__/useMessageScrolling.spec.tsx @@ -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, + conversationId: string, + messagesTree: TMessage[] | null, + ) => ( + + + + + + + + ); + + 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(); + }); +}); diff --git a/client/src/hooks/Messages/useMessageScrolling.ts b/client/src/hooks/Messages/useMessageScrolling.ts index 03cce40878..cd5ae9df63 100644 --- a/client/src/hooks/Messages/useMessageScrolling.ts +++ b/client/src/hooks/Messages/useMessageScrolling.ts @@ -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(null); const contentRef = useRef(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(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, diff --git a/client/src/store/autoScroll.ts b/client/src/store/autoScroll.ts new file mode 100644 index 0000000000..e5e1fa8b2e --- /dev/null +++ b/client/src/store/autoScroll.ts @@ -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('autoScroll', DEFAULT_AUTO_SCROLL); diff --git a/client/src/store/settings.ts b/client/src/store/settings.ts index 67e3c39b7e..109e63e447 100644 --- a/client/src/store/settings.ts +++ b/client/src/store/settings.ts @@ -41,7 +41,6 @@ function isSmallViewport(): boolean { const localStorageAtoms = { // General settings - autoScroll: atomWithLocalStorage('autoScroll', false), sidebarExpanded: atomWithLocalStorage( 'unifiedSidebarExpanded', !isSmallViewport(),