From e47736f723a741058db43b8f9f9bdd106409a947 Mon Sep 17 00:00:00 2001 From: martincupela Date: Mon, 12 Oct 2026 09:33:07 +0200 Subject: [PATCH 1/6] fix(VirtualizedMessageList): keep the last message fully visible in short lists Virtuoso reads a bare index as align: 'start', so both the initial anchor and scrollToBottom parked the last message's top at the viewport top. Where that message is taller than the list -- a floating panel, a sidebar, a compact mobile view -- its bottom was clipped by the difference, and the jump-to-latest button landed in the same place rather than at the bottom. The hardcoded atBottomThreshold of 100px compounded it: on a ~200px list that is half the viewport, so the list called itself at the bottom while part of a message was still below the fold. That flag hides the jump-to-latest button, suppresses the new-message notification, and marks the channel read, so a message could be marked read that had never been fully shown, with no control left to reach it. Both thresholds now scale with the measured scroller height, and marking read additionally requires the list to be at its true bottom rather than merely inside the threshold. MessageList keeps its existing behaviour: the stricter mark-read gate is opt-in and only VirtualizedMessageList passes it. Co-Authored-By: Claude Opus 5 --- .../MessageList/VirtualizedMessageList.tsx | 23 +++- .../__tests__/VirtualizedMessageList.test.tsx | 123 ++++++++++++++++-- .../hooks/VirtualizedMessageList/index.ts | 1 + .../useScrollerViewport.ts | 91 +++++++++++++ .../hooks/__tests__/useMarkRead.test.tsx | 30 +++++ .../MessageList/hooks/useMarkRead.ts | 14 +- 6 files changed, 265 insertions(+), 17 deletions(-) create mode 100644 src/components/MessageList/hooks/VirtualizedMessageList/useScrollerViewport.ts diff --git a/src/components/MessageList/VirtualizedMessageList.tsx b/src/components/MessageList/VirtualizedMessageList.tsx index 4bdd6bf64e..40156f8443 100644 --- a/src/components/MessageList/VirtualizedMessageList.tsx +++ b/src/components/MessageList/VirtualizedMessageList.tsx @@ -23,6 +23,7 @@ import { useUnreadMessagesNotificationVirtualized, } from './hooks/VirtualizedMessageList'; import { useMarkRead } from './hooks/useMarkRead'; +import { useScrollerViewport } from './hooks/VirtualizedMessageList/useScrollerViewport'; import { NewMessageNotification as DefaultNewMessageNotification } from './NewMessageNotification'; import { MessageListMainPanel as DefaultMessageListMainPanel } from './MessageListMainPanel'; import type { GroupStyle, ProcessMessagesParams, RenderedMessage } from './utils'; @@ -188,7 +189,10 @@ function calculateInitialTopMostItemIndex( return { align: 'center', index } as const; } } - return messages.length - 1; + // A bare index aligns the row's top to the viewport top, which leaves the last message + // hanging below the fold whenever it is taller than the list. Anchoring its end keeps the + // list opening at true bottom regardless of how short the viewport is. + return { align: 'end', index: messages.length - 1 } as const; } const VirtualizedMessageListWithContext = ( @@ -381,7 +385,12 @@ const VirtualizedMessageListWithContext = ( setNewMessagesNotification, } = useNewMessageNotification(processedMessages, client.userID, hasMoreNewer); + const { handleScrollerRef, isAtTrueBottom, threshold } = useScrollerViewport(); + useMarkRead({ + // Virtuoso's at-bottom flag carries a threshold, so on a short list it reads true while part + // of the last message is still clipped. Marking a channel read needs the stricter signal. + isAtTrueBottom, isMessageListScrolledToBottom, messageListIsThread: !!threadList, wasMarkedUnread: !!channelUnreadUiState?.first_unread_message_id, @@ -394,7 +403,12 @@ const VirtualizedMessageListWithContext = ( } if (virtuoso.current) { - virtuoso.current.scrollToIndex(processedMessages.length - 1); + // `align: 'end'` for the same reason as `calculateInitialTopMostItemIndex`: without it a + // last message taller than the viewport lands with its bottom clipped. + virtuoso.current.scrollToIndex({ + align: 'end', + index: processedMessages.length - 1, + }); } setNewMessagesNotification(false); @@ -523,9 +537,9 @@ const VirtualizedMessageListWithContext = ( /> atBottomStateChange={atBottomStateChange} - atBottomThreshold={100} + atBottomThreshold={threshold} atTopStateChange={atTopStateChange} - atTopThreshold={100} + atTopThreshold={threshold} className='str-chat__message-list-scroll' components={{ EmptyPlaceholder, @@ -591,6 +605,7 @@ const VirtualizedMessageListWithContext = ( key={messageSetKey} overscan={overscan} ref={virtuoso} + scrollerRef={handleScrollerRef} style={{ overflowX: 'hidden' }} totalCount={processedMessages.length} {...overridingVirtuosoProps} diff --git a/src/components/MessageList/__tests__/VirtualizedMessageList.test.tsx b/src/components/MessageList/__tests__/VirtualizedMessageList.test.tsx index 0354d77513..690fa5536a 100644 --- a/src/components/MessageList/__tests__/VirtualizedMessageList.test.tsx +++ b/src/components/MessageList/__tests__/VirtualizedMessageList.test.tsx @@ -17,20 +17,48 @@ import { VirtualizedMessageList } from '../VirtualizedMessageList'; import { Chat } from '../../Chat'; import { Channel } from '../../Channel'; +import { useVirtualizedMessageListContext, WithComponents } from '../../../context'; -vi.mock('react-virtuoso', async () => { - const { Virtuoso } = await import('react-virtuoso'); - const { forwardRef } = await import('react'); +// Records the props VML hands to Virtuoso and routes the imperative handle through a spy, so the +// scroll target is assertable. The overrides keep the real Virtuoso renderable under jsdom, which +// reports every element as 0px. +const { scrollToIndexSpy, virtuosoProps } = vi.hoisted(() => ({ + scrollToIndexSpy: vi.fn(), + virtuosoProps: [] as Record[], +})); + +vi.mock('react-virtuoso', async (importOriginal) => { + const { Virtuoso } = await importOriginal(); + const { forwardRef, useImperativeHandle, useRef } = await import('react'); return { - Virtuoso: forwardRef((props: any, ref: any) => ( - - )), + Virtuoso: forwardRef((props: any, ref: any) => { + virtuosoProps.push(props); + const innerRef = useRef(null); + useImperativeHandle( + ref, + () => ({ + autoscrollToBottom: (...args: any[]) => + innerRef.current?.autoscrollToBottom(...args), + scrollBy: (...args: any[]) => innerRef.current?.scrollBy(...args), + scrollIntoView: (...args: any[]) => innerRef.current?.scrollIntoView(...args), + scrollTo: (...args: any[]) => innerRef.current?.scrollTo(...args), + scrollToIndex: (...args: any[]) => { + scrollToIndexSpy(...args); + return innerRef.current?.scrollToIndex(...args); + }, + }), + [], + ); + return ( + + ); + }), }; }); @@ -100,6 +128,77 @@ describe('VirtualizedMessageList', () => { expect(virtualList).toBeInTheDocument(); expect(virtualList).toContainElement(emptyStateText); }); + + // A bare index means align: 'start', which parks the last row's top at the viewport top and + // clips its bottom whenever that row is taller than the list -- visible in a short panel. + // jsdom has no layout, so these assert the scroll target rather than the clipping. + describe('scroll anchoring', () => { + let scrollToBottom: (() => Promise) | undefined; + + // scrollToBottom is published on a context VML opens around its own subtree, so the probe + // has to sit inside it. MessageListMainPanel is the outermost overridable slot in there. + const MainPanelProbe = ({ children }: React.PropsWithChildren) => { + scrollToBottom = useVirtualizedMessageListContext().scrollToBottom; + return
{children}
; + }; + + const MESSAGE_COUNT = 20; + + const renderList = async () => { + const user1 = generateUser(); + const user2 = generateUser(); + const mockedChannel = generateChannel({ + members: [generateMember({ user: user1 }), generateMember({ user: user2 })], + messages: Array.from({ length: MESSAGE_COUNT }, (_v, i) => + generateMessage({ id: `anchor-msg-${i}`, user: i % 2 ? user1 : user2 }), + ), + }); + const client = await getTestClientWithUser({ id: 'id' }); + useMockedApis(client, [getOrCreateChannelApi(mockedChannel)]); // eslint-disable-line react-hooks/rules-of-hooks + const channel = client.channel('messaging', mockedChannel.channel.id); + await channel.watch(); + + await act(() => { + render( + + + + + + + , + ); + }); + return virtuosoProps[virtuosoProps.length - 1]; + }; + + beforeEach(() => { + virtuosoProps.length = 0; + scrollToBottom = undefined; + }); + + it('anchors the initial scroll to the end of the last item', async () => { + const props = await renderList(); + expect(props.initialTopMostItemIndex).toEqual({ + align: 'end', + index: props.totalCount - 1, + }); + }); + + it('anchors scrollToBottom to the end of the last item', async () => { + const props = await renderList(); + + await act(async () => { + await scrollToBottom?.(); + }); + + expect(props.totalCount).toBe(MESSAGE_COUNT); + expect(scrollToIndexSpy).toHaveBeenCalledWith({ + align: 'end', + index: MESSAGE_COUNT - 1, + }); + }); + }); }); describe('usePrependedMessagesCount', () => { diff --git a/src/components/MessageList/hooks/VirtualizedMessageList/index.ts b/src/components/MessageList/hooks/VirtualizedMessageList/index.ts index 99d4112e16..3d790524a4 100644 --- a/src/components/MessageList/hooks/VirtualizedMessageList/index.ts +++ b/src/components/MessageList/hooks/VirtualizedMessageList/index.ts @@ -6,3 +6,4 @@ export * from './usePrependMessagesCount'; export * from './useScrollToBottomOnNewMessage'; export * from './useShouldForceScrollToBottom'; export * from './useUnreadMessagesNotificationVirtualized'; +export * from './useScrollerViewport'; diff --git a/src/components/MessageList/hooks/VirtualizedMessageList/useScrollerViewport.ts b/src/components/MessageList/hooks/VirtualizedMessageList/useScrollerViewport.ts new file mode 100644 index 0000000000..ba8d6b6e63 --- /dev/null +++ b/src/components/MessageList/hooks/VirtualizedMessageList/useScrollerViewport.ts @@ -0,0 +1,91 @@ +import { useCallback, useEffect, useRef, useState } from 'react'; + +/** + * Virtuoso's at-bottom and at-top thresholds are absolute pixels, so a value tuned for a + * full-height list means something very different in a short floating panel. These bounds keep the + * derived threshold proportional to the viewport while staying within what Virtuoso behaves well + * with: its own default is 4px, and 100px is what a full-height list effectively used. + */ +const THRESHOLD_RATIO = 0.1; +const MIN_THRESHOLD = 4; +const MAX_THRESHOLD = 100; + +/** Distance from the true bottom, in pixels, still counted as "the last message is fully shown". */ +const TRUE_BOTTOM_EPSILON = 4; + +const clamp = (value: number, min: number, max: number) => + Math.min(Math.max(value, min), max); + +export type ScrollerViewport = { + /** Threshold proportional to the viewport, for Virtuoso's at-bottom / at-top detection. */ + threshold: number; + /** Callback for Virtuoso's `scrollerRef` prop. */ + handleScrollerRef: (element: HTMLElement | Window | null) => void; + /** + * Whether the list is at its true bottom, within a few pixels. Unlike Virtuoso's thresholded + * at-bottom flag this does not go true while part of the last message is still below the fold, + * which is what makes it safe to gate marking a channel read. + */ + isAtTrueBottom: boolean; +}; + +/** + * Tracks the scroll container's height and whether it sits at its true bottom, so thresholds can + * scale with the rendered list rather than assuming a full-height one. + */ +export const useScrollerViewport = (): ScrollerViewport => { + // State rather than a ref: the element arrives from Virtuoso's scrollerRef callback after + // mount, and the observers below have to attach at that point. + const [scroller, setScroller] = useState(null); + // Until the scroller is measured, keep the behaviour a full-height list already had. + const [viewportHeight, setViewportHeight] = useState(MAX_THRESHOLD / THRESHOLD_RATIO); + const [isAtTrueBottom, setIsAtTrueBottom] = useState(true); + const frameRef = useRef(null); + + const handleScrollerRef = useCallback((element: HTMLElement | Window | null) => { + // Virtuoso types this as HTMLElement | Window; the message list always scrolls an element. + setScroller(element instanceof HTMLElement ? element : null); + }, []); + + useEffect(() => { + if (!scroller) return; + + const measure = () => { + frameRef.current = null; + setViewportHeight(scroller.clientHeight); + setIsAtTrueBottom( + scroller.scrollHeight - scroller.scrollTop - scroller.clientHeight <= + TRUE_BOTTOM_EPSILON, + ); + }; + + const scheduleMeasure = () => { + if (frameRef.current !== null) return; + frameRef.current = requestAnimationFrame(measure); + }; + + scroller.addEventListener('scroll', scheduleMeasure, { passive: true }); + + // Matches useScrollLocationLogic: the panel can be resized without the list scrolling, but + // environments without ResizeObserver (jsdom) still get the scroll-driven measurements. + let resizeObserver: ResizeObserver | undefined; + if (typeof ResizeObserver !== 'undefined') { + resizeObserver = new ResizeObserver(scheduleMeasure); + resizeObserver.observe(scroller); + } + measure(); + + return () => { + scroller.removeEventListener('scroll', scheduleMeasure); + resizeObserver?.disconnect(); + if (frameRef.current !== null) cancelAnimationFrame(frameRef.current); + frameRef.current = null; + }; + }, [scroller]); + + return { + handleScrollerRef, + isAtTrueBottom, + threshold: clamp(viewportHeight * THRESHOLD_RATIO, MIN_THRESHOLD, MAX_THRESHOLD), + }; +}; diff --git a/src/components/MessageList/hooks/__tests__/useMarkRead.test.tsx b/src/components/MessageList/hooks/__tests__/useMarkRead.test.tsx index 6f7a8209d4..2d0fa8edc2 100644 --- a/src/components/MessageList/hooks/__tests__/useMarkRead.test.tsx +++ b/src/components/MessageList/hooks/__tests__/useMarkRead.test.tsx @@ -242,6 +242,36 @@ describe('useMarkRead', () => { } expect(markRead).not.toHaveBeenCalled(); }); + + // A thresholded at-bottom flag reads true while the last message is still partly below the + // fold, which would mark a channel read on a message nobody has seen. Same fixture as the + // first case in this block, which is the isAtTrueBottom: true counterpart. + it('should not mark channel read while the last message is not fully shown', async () => { + const channelData = unreadLastMessageChannelData(); + const { + channels: [channel], + client, + } = await initClientWithChannels({ + channelsData: [channelData], + customUser: channelData.read[0].user, + }); + + await render({ + channel, + client, + params: { + ...shouldMarkReadParams, + isAtTrueBottom: false, + }, + }); + + if (scenario === visibilityChangeScenario) { + await act(() => { + document.dispatchEvent(new Event('visibilitychange')); + }); + } + expect(markRead).not.toHaveBeenCalled(); + }); }); describe('on message.new', () => { diff --git a/src/components/MessageList/hooks/useMarkRead.ts b/src/components/MessageList/hooks/useMarkRead.ts index c2c76a00f1..0eea59f74d 100644 --- a/src/components/MessageList/hooks/useMarkRead.ts +++ b/src/components/MessageList/hooks/useMarkRead.ts @@ -16,6 +16,13 @@ const hasReadLastMessage = (channel: Channel, userId: string) => { type UseMarkReadParams = { isMessageListScrolledToBottom: boolean; messageListIsThread: boolean; + /** + * Whether the last message is fully shown. Lists whose at-bottom flag carries a threshold -- + * it can read true while part of the last message is still below the fold -- pass this so a + * channel is never marked read on a message nobody has seen. Omitted, only the at-bottom flag + * applies. + */ + isAtTrueBottom?: boolean; wasMarkedUnread?: boolean; }; @@ -23,12 +30,15 @@ type UseMarkReadParams = { * Takes care of marking a channel read. The channel is read only if all the following applies: * 1. the message list is not rendered in a thread * 2. the message list is scrolled to the bottom - * 3. the channel was not marked unread by the user + * 3. the last message is fully shown, where the caller can tell + * 4. the channel was not marked unread by the user * @param isMessageListScrolledToBottom * @param messageListIsThread + * @param isAtTrueBottom * @param wasChannelMarkedUnread */ export const useMarkRead = ({ + isAtTrueBottom = true, isMessageListScrolledToBottom, messageListIsThread, wasMarkedUnread, @@ -48,6 +58,7 @@ export const useMarkRead = ({ !wasMarkedUnread && !messageListIsThread && isMessageListScrolledToBottom && + isAtTrueBottom && client.user?.id && !hasReadLastMessage(channel, client.user.id); @@ -99,6 +110,7 @@ export const useMarkRead = ({ }, [ channel, client, + isAtTrueBottom, isMessageListScrolledToBottom, markRead, messageListIsThread, From bdca90d12f77d1a065f8eaac3a24591728e4834b Mon Sep 17 00:00:00 2001 From: martincupela Date: Mon, 12 Oct 2026 10:36:42 +0200 Subject: [PATCH 2/6] fix(VirtualizedMessageList): keep viewport measurement honest under overrides Two gaps in the previous commit, from review on the v15 port (#3318). An integrator passing additionalVirtuosoProps.scrollerRef replaced the callback the viewport measurement depends on, leaving the thresholds at their full-height defaults and isAtTrueBottom permanently true. scrollerRef is now applied after the spread and composed with the caller's. Content can also grow without the scroller resizing or scrolling -- a bubble whose avatar or image lands late -- which moves the true bottom without firing either event the hook observes. totalListHeightChanged now triggers a fresh measurement, likewise composed with the caller's. The third review finding does not apply here: this branch already carries isAtTrueBottom in the mark-read effect's dependencies. Co-Authored-By: Claude Opus 5 --- .../MessageList/VirtualizedMessageList.tsx | 32 +++++++++++++++++-- .../useScrollerViewport.ts | 27 ++++++++++++---- 2 files changed, 51 insertions(+), 8 deletions(-) diff --git a/src/components/MessageList/VirtualizedMessageList.tsx b/src/components/MessageList/VirtualizedMessageList.tsx index 40156f8443..b2c0dc2f68 100644 --- a/src/components/MessageList/VirtualizedMessageList.tsx +++ b/src/components/MessageList/VirtualizedMessageList.tsx @@ -385,7 +385,34 @@ const VirtualizedMessageListWithContext = ( setNewMessagesNotification, } = useNewMessageNotification(processedMessages, client.userID, hasMoreNewer); - const { handleScrollerRef, isAtTrueBottom, threshold } = useScrollerViewport(); + const { handleScrollerRef, isAtTrueBottom, remeasure, threshold } = + useScrollerViewport(); + + // Composed with whatever the integrator passes, and applied after the `additionalVirtuosoProps` + // spread: these two callbacks are what keep the thresholds and the true-bottom check honest, so + // an override must not be able to silently switch the measurement off. + const { + scrollerRef: scrollerRefFromProps, + totalListHeightChanged: totalListHeightChangedFromProps, + } = overridingVirtuosoProps; + + const scrollerRef = useCallback( + (element: HTMLElement | Window | null) => { + handleScrollerRef(element); + scrollerRefFromProps?.(element); + }, + [handleScrollerRef, scrollerRefFromProps], + ); + + // Content can grow without the scroller resizing or scrolling -- a bubble whose avatar or image + // lands late -- which moves the true bottom without firing either event the hook observes. + const totalListHeightChanged = useCallback( + (height: number) => { + remeasure(); + totalListHeightChangedFromProps?.(height); + }, + [remeasure, totalListHeightChangedFromProps], + ); useMarkRead({ // Virtuoso's at-bottom flag carries a threshold, so on a short list it reads true while part @@ -605,10 +632,11 @@ const VirtualizedMessageListWithContext = ( key={messageSetKey} overscan={overscan} ref={virtuoso} - scrollerRef={handleScrollerRef} style={{ overflowX: 'hidden' }} totalCount={processedMessages.length} {...overridingVirtuosoProps} + scrollerRef={scrollerRef} + totalListHeightChanged={totalListHeightChanged} {...(scrollSeekPlaceHolder ? { scrollSeek: scrollSeekPlaceHolder } : {})} {...(defaultItemHeight ? { defaultItemHeight } : {})} /> diff --git a/src/components/MessageList/hooks/VirtualizedMessageList/useScrollerViewport.ts b/src/components/MessageList/hooks/VirtualizedMessageList/useScrollerViewport.ts index ba8d6b6e63..4e99c02611 100644 --- a/src/components/MessageList/hooks/VirtualizedMessageList/useScrollerViewport.ts +++ b/src/components/MessageList/hooks/VirtualizedMessageList/useScrollerViewport.ts @@ -21,6 +21,12 @@ export type ScrollerViewport = { threshold: number; /** Callback for Virtuoso's `scrollerRef` prop. */ handleScrollerRef: (element: HTMLElement | Window | null) => void; + /** + * Schedules a fresh measurement. Content can grow without the scroller resizing or scrolling -- + * a message bubble whose avatar or image lands late -- which changes how far the list is from + * its true bottom without firing either event we observe. + */ + remeasure: () => void; /** * Whether the list is at its true bottom, within a few pixels. Unlike Virtuoso's thresholded * at-bottom flag this does not go true while part of the last message is still below the fold, @@ -41,6 +47,16 @@ export const useScrollerViewport = (): ScrollerViewport => { const [viewportHeight, setViewportHeight] = useState(MAX_THRESHOLD / THRESHOLD_RATIO); const [isAtTrueBottom, setIsAtTrueBottom] = useState(true); const frameRef = useRef(null); + // Assigned by the effect below; held in a ref so `remeasure` can stay stable for callers. + const measureRef = useRef<() => void>(() => undefined); + + const remeasure = useCallback(() => { + if (frameRef.current !== null) return; + frameRef.current = requestAnimationFrame(() => { + frameRef.current = null; + measureRef.current(); + }); + }, []); const handleScrollerRef = useCallback((element: HTMLElement | Window | null) => { // Virtuoso types this as HTMLElement | Window; the message list always scrolls an element. @@ -51,7 +67,6 @@ export const useScrollerViewport = (): ScrollerViewport => { if (!scroller) return; const measure = () => { - frameRef.current = null; setViewportHeight(scroller.clientHeight); setIsAtTrueBottom( scroller.scrollHeight - scroller.scrollTop - scroller.clientHeight <= @@ -59,10 +74,8 @@ export const useScrollerViewport = (): ScrollerViewport => { ); }; - const scheduleMeasure = () => { - if (frameRef.current !== null) return; - frameRef.current = requestAnimationFrame(measure); - }; + measureRef.current = measure; + const scheduleMeasure = remeasure; scroller.addEventListener('scroll', scheduleMeasure, { passive: true }); @@ -80,12 +93,14 @@ export const useScrollerViewport = (): ScrollerViewport => { resizeObserver?.disconnect(); if (frameRef.current !== null) cancelAnimationFrame(frameRef.current); frameRef.current = null; + measureRef.current = () => undefined; }; - }, [scroller]); + }, [remeasure, scroller]); return { handleScrollerRef, isAtTrueBottom, + remeasure, threshold: clamp(viewportHeight * THRESHOLD_RATIO, MIN_THRESHOLD, MAX_THRESHOLD), }; }; From 973e52a9ec0c054528b675ba9cc327880ce8bc48 Mon Sep 17 00:00:00 2001 From: martincupela Date: Mon, 12 Oct 2026 10:56:33 +0200 Subject: [PATCH 3/6] fix(VirtualizedMessageList): measure the document when the list scrolls the window With `useWindowScroll` Virtuoso hands back the window rather than an element, and the viewport hook discarded it. The list was then never measured: the thresholds stayed at their full-height defaults and the true-bottom check answered "yes" forever, so a channel could still be marked read with the newest message cut off. Scroll events still come from the window, but the measurements now come from the document's scrolling element. Also covers the two callback-composition paths that had no tests: an integrator's `scrollerRef` and `totalListHeightChanged` still run, and the list still measures itself rather than silently falling back to the unmeasured defaults. Co-Authored-By: Claude Opus 5 --- .../__tests__/VirtualizedMessageList.test.tsx | 44 +++++++++++++++- .../__tests__/useScrollerViewport.test.ts | 50 +++++++++++++++++++ .../useScrollerViewport.ts | 31 ++++++++---- 3 files changed, 112 insertions(+), 13 deletions(-) create mode 100644 src/components/MessageList/hooks/VirtualizedMessageList/__tests__/useScrollerViewport.test.ts diff --git a/src/components/MessageList/__tests__/VirtualizedMessageList.test.tsx b/src/components/MessageList/__tests__/VirtualizedMessageList.test.tsx index 690fa5536a..3c7c6f0751 100644 --- a/src/components/MessageList/__tests__/VirtualizedMessageList.test.tsx +++ b/src/components/MessageList/__tests__/VirtualizedMessageList.test.tsx @@ -144,7 +144,7 @@ describe('VirtualizedMessageList', () => { const MESSAGE_COUNT = 20; - const renderList = async () => { + const renderList = async ({ listProps = {} } = {}) => { const user1 = generateUser(); const user2 = generateUser(); const mockedChannel = generateChannel({ @@ -163,7 +163,7 @@ describe('VirtualizedMessageList', () => { - + , @@ -185,6 +185,46 @@ describe('VirtualizedMessageList', () => { }); }); + // The thresholds and the true-bottom check are only honest while these callbacks reach the + // viewport hook, so an integrator override has to compose with them rather than replace them. + it('still measures the viewport when a scrollerRef is supplied', async () => { + const scrollerRefFromProps = vi.fn(); + const props = await renderList({ + listProps: { additionalVirtuosoProps: { scrollerRef: scrollerRefFromProps } }, + }); + + expect(props.scrollerRef).not.toBe(scrollerRefFromProps); + + const scroller = document.createElement('div'); + act(() => { + props.scrollerRef(scroller); + }); + + expect(scrollerRefFromProps).toHaveBeenCalledWith(scroller); + const measured = virtuosoProps[virtuosoProps.length - 1]; + expect(measured.atBottomThreshold).not.toBe(100); + expect(measured.atBottomThreshold).toBe(measured.atTopThreshold); + }); + + it('still remeasures when a totalListHeightChanged is supplied', async () => { + const totalListHeightChangedFromProps = vi.fn(); + const props = await renderList({ + listProps: { + additionalVirtuosoProps: { + totalListHeightChanged: totalListHeightChangedFromProps, + }, + }, + }); + + expect(props.totalListHeightChanged).not.toBe(totalListHeightChangedFromProps); + + act(() => { + props.totalListHeightChanged(1234); + }); + + expect(totalListHeightChangedFromProps).toHaveBeenCalledWith(1234); + }); + it('anchors scrollToBottom to the end of the last item', async () => { const props = await renderList(); diff --git a/src/components/MessageList/hooks/VirtualizedMessageList/__tests__/useScrollerViewport.test.ts b/src/components/MessageList/hooks/VirtualizedMessageList/__tests__/useScrollerViewport.test.ts new file mode 100644 index 0000000000..a0f15999d6 --- /dev/null +++ b/src/components/MessageList/hooks/VirtualizedMessageList/__tests__/useScrollerViewport.test.ts @@ -0,0 +1,50 @@ +import { act, renderHook } from '@testing-library/react'; + +import { useScrollerViewport } from '../useScrollerViewport'; + +// The thresholds and the true-bottom check are only meaningful once something has been measured. +// Before that the hook deliberately reports the slack a full-height list always had. +const UNMEASURED_THRESHOLD = 100; + +describe('useScrollerViewport', () => { + it('reports the full-height defaults until a scroller arrives', () => { + const { result } = renderHook(() => useScrollerViewport()); + + expect(result.current.threshold).toBe(UNMEASURED_THRESHOLD); + expect(result.current.isAtTrueBottom).toBe(true); + }); + + it('measures the element it is given', () => { + const scroller = document.createElement('div'); + Object.defineProperty(scroller, 'clientHeight', { value: 300 }); + document.body.appendChild(scroller); + + const { result } = renderHook(() => useScrollerViewport()); + act(() => { + result.current.handleScrollerRef(scroller); + }); + + expect(result.current.threshold).toBe(30); + + document.body.removeChild(scroller); + }); + + // With `useWindowScroll` Virtuoso hands back the window rather than an element. Discarding it + // would leave the list unmeasured, the thresholds at their full-height defaults, and the + // true-bottom check answering "yes" forever. + it('measures the document when the list scrolls the window', () => { + const measured = (document.scrollingElement ?? + document.documentElement) as HTMLElement; + Object.defineProperty(measured, 'clientHeight', { + configurable: true, + value: 500, + }); + + const { result } = renderHook(() => useScrollerViewport()); + act(() => { + result.current.handleScrollerRef(window); + }); + + expect(result.current.threshold).toBe(50); + }); +}); diff --git a/src/components/MessageList/hooks/VirtualizedMessageList/useScrollerViewport.ts b/src/components/MessageList/hooks/VirtualizedMessageList/useScrollerViewport.ts index 4e99c02611..0e753ce546 100644 --- a/src/components/MessageList/hooks/VirtualizedMessageList/useScrollerViewport.ts +++ b/src/components/MessageList/hooks/VirtualizedMessageList/useScrollerViewport.ts @@ -40,9 +40,9 @@ export type ScrollerViewport = { * scale with the rendered list rather than assuming a full-height one. */ export const useScrollerViewport = (): ScrollerViewport => { - // State rather than a ref: the element arrives from Virtuoso's scrollerRef callback after + // State rather than a ref: the scroller arrives from Virtuoso's scrollerRef callback after // mount, and the observers below have to attach at that point. - const [scroller, setScroller] = useState(null); + const [scrollTarget, setScrollTarget] = useState(null); // Until the scroller is measured, keep the behaviour a full-height list already had. const [viewportHeight, setViewportHeight] = useState(MAX_THRESHOLD / THRESHOLD_RATIO); const [isAtTrueBottom, setIsAtTrueBottom] = useState(true); @@ -59,17 +59,26 @@ export const useScrollerViewport = (): ScrollerViewport => { }, []); const handleScrollerRef = useCallback((element: HTMLElement | Window | null) => { - // Virtuoso types this as HTMLElement | Window; the message list always scrolls an element. - setScroller(element instanceof HTMLElement ? element : null); + setScrollTarget(element); }, []); useEffect(() => { - if (!scroller) return; + if (!scrollTarget) return; + + // With `useWindowScroll` Virtuoso hands back the window rather than an element. The scroll + // events still arrive on the window, but the measurements have to come from the document's + // scrolling element -- without this the list is never measured, the thresholds stay at their + // full-height defaults, and the true-bottom check silently answers "yes" forever. + const isWindow = !(scrollTarget instanceof HTMLElement); + const measured = isWindow + ? ((document.scrollingElement ?? document.documentElement) as HTMLElement | null) + : scrollTarget; + if (!measured) return; const measure = () => { - setViewportHeight(scroller.clientHeight); + setViewportHeight(measured.clientHeight); setIsAtTrueBottom( - scroller.scrollHeight - scroller.scrollTop - scroller.clientHeight <= + measured.scrollHeight - measured.scrollTop - measured.clientHeight <= TRUE_BOTTOM_EPSILON, ); }; @@ -77,25 +86,25 @@ export const useScrollerViewport = (): ScrollerViewport => { measureRef.current = measure; const scheduleMeasure = remeasure; - scroller.addEventListener('scroll', scheduleMeasure, { passive: true }); + scrollTarget.addEventListener('scroll', scheduleMeasure, { passive: true }); // Matches useScrollLocationLogic: the panel can be resized without the list scrolling, but // environments without ResizeObserver (jsdom) still get the scroll-driven measurements. let resizeObserver: ResizeObserver | undefined; if (typeof ResizeObserver !== 'undefined') { resizeObserver = new ResizeObserver(scheduleMeasure); - resizeObserver.observe(scroller); + resizeObserver.observe(measured); } measure(); return () => { - scroller.removeEventListener('scroll', scheduleMeasure); + scrollTarget.removeEventListener('scroll', scheduleMeasure); resizeObserver?.disconnect(); if (frameRef.current !== null) cancelAnimationFrame(frameRef.current); frameRef.current = null; measureRef.current = () => undefined; }; - }, [remeasure, scroller]); + }, [remeasure, scrollTarget]); return { handleScrollerRef, From 0a9854736daad9db1bcd58e1be9fcfb40f92973e Mon Sep 17 00:00:00 2001 From: martincupela Date: Mon, 12 Oct 2026 11:05:46 +0200 Subject: [PATCH 4/6] fix(VirtualizedMessageList): feed the viewport hook in window-scroll modes The previous commit handled a Window arriving through `scrollerRef`, but that never happens: Virtuoso's window scroller passes its own no-op in the scrollerRef slot (List.tsx, buildWindowScroller), so an integrator's callback is never invoked under `useWindowScroll` or `customScrollParent`. Nothing was measured, the thresholds stayed at their full-height defaults and the true-bottom check answered "yes" for the life of the list -- so a channel could still be marked read with the newest message clipped. The list now tells the hook what to measure for those two modes rather than waiting for a callback that will not come. Covered by a test for each mode, which needs a ResizeObserver stub because Virtuoso's own window-viewport hook requires one. Co-Authored-By: Claude Opus 5 --- .../MessageList/VirtualizedMessageList.tsx | 10 ++++++ .../__tests__/VirtualizedMessageList.test.tsx | 32 +++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/src/components/MessageList/VirtualizedMessageList.tsx b/src/components/MessageList/VirtualizedMessageList.tsx index b2c0dc2f68..d6ebf72d8d 100644 --- a/src/components/MessageList/VirtualizedMessageList.tsx +++ b/src/components/MessageList/VirtualizedMessageList.tsx @@ -392,10 +392,20 @@ const VirtualizedMessageListWithContext = ( // spread: these two callbacks are what keep the thresholds and the true-bottom check honest, so // an override must not be able to silently switch the measurement off. const { + customScrollParent, scrollerRef: scrollerRefFromProps, totalListHeightChanged: totalListHeightChangedFromProps, + useWindowScroll, } = overridingVirtuosoProps; + // Virtuoso's window-scroll path never calls the supplied `scrollerRef` -- it passes its own + // no-op in that slot -- so for these two modes the hook has to be told what to measure, or the + // thresholds stay at their full-height defaults and the true-bottom check always answers "yes". + useEffect(() => { + if (!useWindowScroll && !customScrollParent) return; + handleScrollerRef(customScrollParent ?? window); + }, [customScrollParent, handleScrollerRef, useWindowScroll]); + const scrollerRef = useCallback( (element: HTMLElement | Window | null) => { handleScrollerRef(element); diff --git a/src/components/MessageList/__tests__/VirtualizedMessageList.test.tsx b/src/components/MessageList/__tests__/VirtualizedMessageList.test.tsx index 3c7c6f0751..f3330a8323 100644 --- a/src/components/MessageList/__tests__/VirtualizedMessageList.test.tsx +++ b/src/components/MessageList/__tests__/VirtualizedMessageList.test.tsx @@ -225,6 +225,38 @@ describe('VirtualizedMessageList', () => { expect(totalListHeightChangedFromProps).toHaveBeenCalledWith(1234); }); + // Virtuoso's window-scroll path never calls the supplied scrollerRef, so without feeding the + // hook directly the list is never measured and the true-bottom check answers "yes" forever. + describe('window scrolling', () => { + // Virtuoso's own window-viewport hook needs a ResizeObserver, which jsdom does not + // provide. mock-builders' ResizeObserverMock has no `unobserve`, which Virtuoso calls. + const OriginalResizeObserver = globalThis.ResizeObserver; + + beforeEach(() => { + globalThis.ResizeObserver = class implements ResizeObserver { + disconnect = vi.fn(); + observe = vi.fn(); + unobserve = vi.fn(); + }; + }); + + afterEach(() => { + globalThis.ResizeObserver = OriginalResizeObserver; + }); + + it.each([ + ['useWindowScroll', { useWindowScroll: true }], + ['customScrollParent', { customScrollParent: document.createElement('div') }], + ])('measures the viewport with %s', async (_label, virtuosoProp) => { + const props = await renderList({ + listProps: { additionalVirtuosoProps: virtuosoProp }, + }); + + expect(props.atBottomThreshold).not.toBe(100); + expect(props.atBottomThreshold).toBe(props.atTopThreshold); + }); + }); + it('anchors scrollToBottom to the end of the last item', async () => { const props = await renderList(); From 5601f27f14bb6cb52f66854a57bc04c1273b3662 Mon Sep 17 00:00:00 2001 From: martincupela Date: Mon, 12 Oct 2026 11:12:39 +0200 Subject: [PATCH 5/6] test(VirtualizedMessageList): assert the composed scrollerRef forwards teardown Virtuoso passes null when the scroller goes away, and a caller relying on that signal has to receive it through the composed callback too. Co-Authored-By: Claude Opus 5 --- .../MessageList/__tests__/VirtualizedMessageList.test.tsx | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/components/MessageList/__tests__/VirtualizedMessageList.test.tsx b/src/components/MessageList/__tests__/VirtualizedMessageList.test.tsx index f3330a8323..85ed9461ba 100644 --- a/src/components/MessageList/__tests__/VirtualizedMessageList.test.tsx +++ b/src/components/MessageList/__tests__/VirtualizedMessageList.test.tsx @@ -204,6 +204,12 @@ describe('VirtualizedMessageList', () => { const measured = virtuosoProps[virtuosoProps.length - 1]; expect(measured.atBottomThreshold).not.toBe(100); expect(measured.atBottomThreshold).toBe(measured.atTopThreshold); + + // Virtuoso passes null on teardown; the caller has to see that too. + act(() => { + props.scrollerRef(null); + }); + expect(scrollerRefFromProps).toHaveBeenCalledWith(null); }); it('still remeasures when a totalListHeightChanged is supplied', async () => { From 2d8fada668b47ce90e4b6cd5553970a3f6296947 Mon Sep 17 00:00:00 2001 From: martincupela Date: Mon, 12 Oct 2026 12:42:16 +0200 Subject: [PATCH 6/6] fix(VirtualizedMessageList): gate mark-read on the same threshold the UI shows The bottom check used a flat 4px while the jump-to-latest control used the viewport-scaled threshold, so between the two there was a band -- 4 to ~20px on a short panel, 4 to ~67px on a full-height one -- where the control was hidden because the list called itself at the bottom, and mark-read was blocked because the stricter check disagreed. The user saw no way to scroll further and the channel never went read. Both now derive from the same measured height, so they cannot disagree: if the UI says the list is at the bottom, the channel marks read. Found by cross-browser testing against a real channel; reproduced in Chrome, Firefox and Safari, and gone in all three after this change. Co-Authored-By: Claude Opus 5 --- .../__tests__/useScrollerViewport.test.ts | 35 +++++++++++++++++++ .../useScrollerViewport.ts | 22 +++++++----- 2 files changed, 48 insertions(+), 9 deletions(-) diff --git a/src/components/MessageList/hooks/VirtualizedMessageList/__tests__/useScrollerViewport.test.ts b/src/components/MessageList/hooks/VirtualizedMessageList/__tests__/useScrollerViewport.test.ts index a0f15999d6..d5fb743c3e 100644 --- a/src/components/MessageList/hooks/VirtualizedMessageList/__tests__/useScrollerViewport.test.ts +++ b/src/components/MessageList/hooks/VirtualizedMessageList/__tests__/useScrollerViewport.test.ts @@ -29,6 +29,41 @@ describe('useScrollerViewport', () => { document.body.removeChild(scroller); }); + // The gate that gurads mark-read has to agree with the threshold that decides whether a + // jump-to-latest control is shown. If it were stricter there would be a band where the UI + // claims the list is at the bottom while the channel silently refuses to go read. + describe.each([ + [300, 30], + [200, 20], + [1200, 100], + ])('viewport %ipx (threshold %ipx)', (viewportHeight, threshold) => { + const mount = (distanceFromBottom: number) => { + const scroller = document.createElement('div'); + Object.defineProperty(scroller, 'clientHeight', { value: viewportHeight }); + Object.defineProperty(scroller, 'scrollHeight', { value: viewportHeight + 5000 }); + scroller.scrollTop = 5000 - distanceFromBottom; + document.body.appendChild(scroller); + + const { result } = renderHook(() => useScrollerViewport()); + act(() => { + result.current.handleScrollerRef(scroller); + }); + document.body.removeChild(scroller); + return result; + }; + + it('is at the bottom just inside the threshold', () => { + const result = mount(threshold - 1); + expect(result.current.threshold).toBe(threshold); + expect(result.current.isAtTrueBottom).toBe(true); + }); + + it('is not at the bottom just outside the threshold', () => { + const result = mount(threshold + 1); + expect(result.current.isAtTrueBottom).toBe(false); + }); + }); + // With `useWindowScroll` Virtuoso hands back the window rather than an element. Discarding it // would leave the list unmeasured, the thresholds at their full-height defaults, and the // true-bottom check answering "yes" forever. diff --git a/src/components/MessageList/hooks/VirtualizedMessageList/useScrollerViewport.ts b/src/components/MessageList/hooks/VirtualizedMessageList/useScrollerViewport.ts index 0e753ce546..a2a3c44ca4 100644 --- a/src/components/MessageList/hooks/VirtualizedMessageList/useScrollerViewport.ts +++ b/src/components/MessageList/hooks/VirtualizedMessageList/useScrollerViewport.ts @@ -10,9 +10,6 @@ const THRESHOLD_RATIO = 0.1; const MIN_THRESHOLD = 4; const MAX_THRESHOLD = 100; -/** Distance from the true bottom, in pixels, still counted as "the last message is fully shown". */ -const TRUE_BOTTOM_EPSILON = 4; - const clamp = (value: number, min: number, max: number) => Math.min(Math.max(value, min), max); @@ -28,9 +25,13 @@ export type ScrollerViewport = { */ remeasure: () => void; /** - * Whether the list is at its true bottom, within a few pixels. Unlike Virtuoso's thresholded - * at-bottom flag this does not go true while part of the last message is still below the fold, - * which is what makes it safe to gate marking a channel read. + * Whether the list is at its bottom, measured against the same `threshold` Virtuoso uses. It + * deliberately shares that definition: the threshold is what decides whether the user is shown + * a jump-to-latest control, so gating mark-read on anything stricter would leave a band where + * the UI claims the list is at the bottom while the channel silently refuses to go read. + * + * It is still worth having separately from Virtuoso's own flag, which only updates when the + * at-bottom state flips -- this is re-measured whenever the content or the viewport changes. */ isAtTrueBottom: boolean; }; @@ -76,10 +77,13 @@ export const useScrollerViewport = (): ScrollerViewport => { if (!measured) return; const measure = () => { - setViewportHeight(measured.clientHeight); + const height = measured.clientHeight; + setViewportHeight(height); + // Derived from the height just measured rather than from state, so the bottom check can + // never disagree with the threshold reported alongside it. + const threshold = clamp(height * THRESHOLD_RATIO, MIN_THRESHOLD, MAX_THRESHOLD); setIsAtTrueBottom( - measured.scrollHeight - measured.scrollTop - measured.clientHeight <= - TRUE_BOTTOM_EPSILON, + measured.scrollHeight - measured.scrollTop - measured.clientHeight <= threshold, ); };