refactor: dont use useEffect anymore to control thread scroll

apps/meteor/app/ui-utils/client/lib/RoomHistoryManager.ts

@@ -56,8 +56,6 @@ export function upsertMessageBulk(
 
 const defaultLimit = parseInt(getConfig('roomListLimit') ?? '50') || 50;
 
-const waitAfterFlush = (fn: () => void) => setTimeout(() => Tracker.afterFlush(fn), 10);
-
 class RoomHistoryManagerClass extends Emitter {
 	private lastRequest?: Date;
 
@@ -156,8 +154,6 @@ class RoomHistoryManagerClass extends Emitter {
 
 		this.unqueue();
 
-		let previousHeight: number | undefined;
-		let scroll: number | undefined;
 		const { messages = [] } = result;
 		room.unreadNotLoaded.set(result.unreadNotLoaded);
 		room.firstUnread.set(result.firstUnread);
@@ -166,12 +162,7 @@ class RoomHistoryManagerClass extends Emitter {
 			room.oldestTs = messages[messages.length - 1].ts;
 		}
 
-		const wrapper = await waitForElement('.messages-box .wrapper [data-overlayscrollbars-viewport]');
-
-		if (wrapper) {
-			previousHeight = wrapper.scrollHeight;
-			scroll = wrapper.scrollTop;
-		}
+		await waitForElement('.messages-box .wrapper [data-overlayscrollbars-viewport]');
 
 		upsertMessageBulk({
 			msgs: messages.filter((msg) => msg.t !== 'command'),
@@ -194,12 +185,6 @@ class RoomHistoryManagerClass extends Emitter {
 			return this.getMore(rid);
 		}
 
-		waitAfterFlush(() => {
-			this.emit('loaded-messages');
-			const heightDiff = wrapper.scrollHeight - (previousHeight ?? NaN);
-			wrapper.scrollTop = (scroll ?? NaN) + heightDiff;
-		});
-
 		room.isLoading.set(false);
 	}
 

apps/meteor/client/components/message/list/MessageListContext.tsx

@@ -1,5 +1,5 @@
 import type { IMessage } from '@rocket.chat/core-typings';
-import type { KeyboardEvent, MouseEvent, RefObject } from 'react';
+import type { KeyboardEvent, MouseEvent, MutableRefObject } from 'react';
 import { createContext, useContext } from 'react';
 
 export type MessageListContextValue = {
@@ -25,7 +25,7 @@ export type MessageListContextValue = {
 	showColors: boolean;
 	jumpToMessageParam?: string;
 	username: string | undefined;
-	messageListRef?: RefObject<HTMLElement>;
+	messageListRef?: MutableRefObject<HTMLElement | undefined>;
 };
 
 export const MessageListContext = createContext<MessageListContextValue>({
@@ -43,7 +43,7 @@ export const MessageListContext = createContext<MessageListContextValue>({
 	showUsername: false,
 	showColors: false,
 	username: undefined,
-	messageListRef: { current: null },
+	messageListRef: { current: undefined },
 });
 
 export const useShowTranslated: MessageListContextValue['useShowTranslated'] = (...args) =>

apps/meteor/client/views/room/MessageList/hooks/useJumpToMessage.ts

@@ -3,48 +3,69 @@ import { useRouter } from '@rocket.chat/ui-contexts';
 import { useCallback } from 'react';
 
 import { useMessageListJumpToMessageParam, useMessageListRef } from '../../../../components/message/list/MessageListContext';
+import { useSafeRefCallback } from '../../../../hooks/useSafeRefCallback';
 import { setHighlightMessage, clearHighlightMessage } from '../providers/messageHighlightSubscription';
 
 // this is an arbitrary value so that there's a gap between the header and the message;
-const SCROLL_EXTRA_OFFSET = 60;
 
 export const useJumpToMessage = (messageId: IMessage['_id']) => {
 	const jumpToMessageParam = useMessageListJumpToMessageParam();
 	const listRef = useMessageListRef();
 	const router = useRouter();
 
-	const ref = useCallback(
-		(node: HTMLElement | null) => {
-			if (!node || !scroll) {
-				return;
-			}
-			setTimeout(() => {
-				if (listRef?.current) {
-					const wrapper = listRef.current;
-					const containerRect = wrapper.getBoundingClientRect();
-					const messageRect = node.getBoundingClientRect();
-
-					const offset = messageRect.top - containerRect.top;
-					const scrollPosition = wrapper.scrollTop;
-					const newScrollPosition = scrollPosition + offset - SCROLL_EXTRA_OFFSET;
-
-					wrapper.scrollTo({ top: newScrollPosition, behavior: 'smooth' });
+	const ref = useSafeRefCallback(
+		useCallback(
+			(node: HTMLElement | null) => {
+				if (!node || !scroll) {
+					return;
 				}
 
-				const { msg: _, ...search } = router.getSearchParameters();
-				router.navigate(
+				if (listRef) {
+					listRef.current = node;
+				}
+
+				node.scrollIntoView({
+					behavior: 'smooth',
+					block: 'center',
+				});
+				const handleScroll = () => {
+					const { msg: _, ...search } = router.getSearchParameters();
+					router.navigate(
+						{
+							pathname: router.getLocationPathname(),
+							search,
+						},
+						{ replace: true },
+					);
+					setTimeout(clearHighlightMessage, 2000);
+				};
+
+				const observer = new IntersectionObserver(
+					(entries) => {
+						entries.forEach((entry) => {
+							if (entry.isIntersecting) {
+								handleScroll();
+							}
+						});
+					},
 					{
-						pathname: router.getLocationPathname(),
-						search,
+						threshold: 0.1,
 					},
-					{ replace: true },
 				);
 
+				observer.observe(node);
+
 				setHighlightMessage(messageId);
-				setTimeout(clearHighlightMessage, 2000);
-			}, 500);
-		},
-		[listRef, messageId, router],
+
+				return () => {
+					observer.disconnect();
+					if (listRef) {
+						listRef.current = undefined;
+					}
+				};
+			},
+			[listRef, messageId, router],
+		),
 	);
 
 	if (jumpToMessageParam !== messageId) {

apps/meteor/client/views/room/MessageList/providers/MessageListProvider.tsx

@@ -1,6 +1,6 @@
 import { isThreadMainMessage } from '@rocket.chat/core-typings';
 import { useLayout, useUser, useUserPreference, useSetting, useEndpoint, useSearchParameter } from '@rocket.chat/ui-contexts';
-import type { ReactNode, RefObject } from 'react';
+import type { MutableRefObject, ReactNode } from 'react';
 import { useMemo, memo } from 'react';
 
 import { getRegexHighlight, getRegexHighlightUrl } from '../../../../../app/highlight-words/client/helper';
@@ -15,7 +15,7 @@ import { useLoadSurroundingMessages } from '../hooks/useLoadSurroundingMessages'
 
 type MessageListProviderProps = {
 	children: ReactNode;
-	messageListRef?: RefObject<HTMLElement>;
+	messageListRef?: MutableRefObject<HTMLElement | undefined>;
 	attachmentDimension?: {
 		width?: number;
 		height?: number;

apps/meteor/client/views/room/body/RoomBody.tsx

@@ -2,7 +2,7 @@ import { Box } from '@rocket.chat/fuselage';
 import { useMergedRefs } from '@rocket.chat/fuselage-hooks';
 import { usePermission, useRole, useSetting, useTranslation, useUser, useUserPreference } from '@rocket.chat/ui-contexts';
 import type { MouseEvent, ReactElement } from 'react';
-import { memo, useCallback, useMemo, useRef } from 'react';
+import { memo, useCallback, useMemo } from 'react';
 
 import DropTargetOverlay from './DropTargetOverlay';
 import JumpToRecentMessageButton from './JumpToRecentMessageButton';
@@ -81,8 +81,6 @@ const RoomBody = (): ReactElement => {
 		return subscribed;
 	}, [allowAnonymousRead, canPreviewChannelRoom, room, subscribed]);
 
-	const innerBoxRef = useRef<HTMLDivElement | null>(null);
-
 	const {
 		wrapperRef: unreadBarWrapperRef,
 		innerRef: unreadBarInnerRef,
@@ -93,7 +91,7 @@ const RoomBody = (): ReactElement => {
 
 	const { innerRef: dateScrollInnerRef, bubbleRef, listStyle, ...bubbleDate } = useDateScroll();
 
-	const { innerRef: isAtBottomInnerRef, atBottomRef, sendToBottom, sendToBottomIfNecessary, isAtBottom } = useListIsAtBottom();
+	const { innerRef: isAtBottomInnerRef, atBottomRef, sendToBottom, sendToBottomIfNecessary, isAtBottom, jumpToRef } = useListIsAtBottom();
 
 	const { innerRef: getMoreInnerRef } = useGetMore(room._id, atBottomRef);
 
@@ -118,7 +116,6 @@ const RoomBody = (): ReactElement => {
 
 	const innerRef = useMergedRefs(
 		dateScrollInnerRef,
-		innerBoxRef,
 		restoreScrollPositionInnerRef,
 		isAtBottomInnerRef,
 		newMessagesScrollRef,
@@ -245,7 +242,7 @@ const RoomBody = (): ReactElement => {
 														)}
 													</>
 												) : null}
-												<MessageList rid={room._id} messageListRef={innerBoxRef} />
+												<MessageList rid={room._id} messageListRef={jumpToRef} />
 												{hasMoreNextMessages ? (
 													<li className='load-more'>{isLoadingMoreMessages ? <LoadingMessagesIndicator /> : null}</li>
 												) : null}

apps/meteor/client/views/room/body/RoomBodyV2.tsx

@@ -81,7 +81,7 @@ const RoomBody = (): ReactElement => {
 		return subscribed;
 	}, [allowAnonymousRead, canPreviewChannelRoom, room, subscribed]);
 
-	const innerBoxRef = useRef<HTMLDivElement | null>(null);
+	const innerBoxRef = useRef<HTMLElement>(undefined);
 
 	const {
 		wrapperRef,
@@ -93,7 +93,7 @@ const RoomBody = (): ReactElement => {
 
 	const { innerRef: dateScrollInnerRef, bubbleRef, listStyle, ...bubbleDate } = useDateScroll();
 
-	const { innerRef: isAtBottomInnerRef, atBottomRef, sendToBottom, sendToBottomIfNecessary, isAtBottom } = useListIsAtBottom();
+	const { innerRef: isAtBottomInnerRef, atBottomRef, sendToBottom, sendToBottomIfNecessary, isAtBottom, jumpToRef } = useListIsAtBottom();
 
 	const { innerRef: getMoreInnerRef } = useGetMore(room._id, atBottomRef);
 
@@ -248,7 +248,7 @@ const RoomBody = (): ReactElement => {
 														)}
 													</>
 												) : null}
-												<MessageList rid={room._id} messageListRef={innerBoxRef} />
+												<MessageList rid={room._id} messageListRef={jumpToRef} />
 												{hasMoreNextMessages ? (
 													<li className='load-more'>{isLoadingMoreMessages ? <LoadingMessagesIndicator /> : null}</li>
 												) : null}

apps/meteor/client/views/room/body/hooks/useListIsAtBottom.ts

@@ -9,13 +9,18 @@ import { useSafeRefCallback } from '../../../../hooks/useSafeRefCallback';
 export const useListIsAtBottom = () => {
 	const atBottomRef = useRef(true);
 
+	const jumpToRef = useRef<HTMLElement>(undefined);
+
 	const innerBoxRef = useRef<HTMLDivElement | null>(null);
 
 	const sendToBottom = useCallback(() => {
 		innerBoxRef.current?.scrollTo({ left: 30, top: innerBoxRef.current?.scrollHeight });
 	}, []);
 
 	const sendToBottomIfNecessary = useCallback(() => {
+		if (jumpToRef.current) {
+			atBottomRef.current = false;
+		}
 		if (atBottomRef.current === true) {
 			sendToBottom();
 		}
@@ -42,6 +47,9 @@ export const useListIsAtBottom = () => {
 				}
 
 				const observer = new ResizeObserver(() => {
+					if (jumpToRef.current) {
+						atBottomRef.current = false;
+					}
 					if (atBottomRef.current === true) {
 						node.scrollTo({ left: 30, top: node.scrollHeight });
 					}
@@ -72,5 +80,6 @@ export const useListIsAtBottom = () => {
 		sendToBottom,
 		sendToBottomIfNecessary,
 		isAtBottom,
+		jumpToRef,
 	};
 };

apps/meteor/client/views/room/body/hooks/useRestoreScrollPosition.spec.ts

@@ -33,21 +33,22 @@ describe('useRestoreScrollPosition', () => {
 		expect(mockElement.removeEventListener).toHaveBeenCalledWith('scroll', expect.any(Function));
 	});
 
-	it('should not restore scroll position if already at bottom', () => {
-		(RoomManager.getStore as jest.Mock).mockReturnValue({ scroll: 100, atBottom: true });
+	it('should do nothing if no previous scroll position is stored', () => {
+		(RoomManager.getStore as jest.Mock).mockReturnValue({ atBottom: true });
 
 		const mockElement = {
 			addEventListener: jest.fn(),
 			removeEventListener: jest.fn(),
 			scrollHeight: 800,
+			scrollTop: 123,
 		};
 
 		const useRefSpy = jest.spyOn(React, 'useRef').mockReturnValueOnce({ current: mockElement });
 
 		const { unmount } = renderHook(() => useRestoreScrollPosition());
 
 		expect(useRefSpy).toHaveBeenCalledWith(null);
-		expect(mockElement).toHaveProperty('scrollTop', 800);
+		expect(mockElement).toHaveProperty('scrollTop', 123);
 		expect(mockElement).not.toHaveProperty('scrollLeft');
 
 		unmount();

apps/meteor/client/views/room/body/hooks/useRestoreScrollPosition.ts

@@ -16,11 +16,9 @@ export function useRestoreScrollPosition() {
 
 		const store = RoomManager.getStore(roomId);
 
-		if (store?.scroll && !store.atBottom) {
+		if (store?.scroll !== undefined && !store.atBottom) {
 			ref.current.scrollTop = store.scroll;
 			ref.current.scrollLeft = 30;
-		} else {
-			ref.current.scrollTop = ref.current.scrollHeight;
 		}
 	}, [roomId]);
 

apps/meteor/client/views/room/contextualBar/Threads/components/ThreadMessageList.tsx

@@ -1,10 +1,9 @@
 import type { IMessage, IThreadMainMessage } from '@rocket.chat/core-typings';
 import { Box } from '@rocket.chat/fuselage';
-import { useMergedRefs } from '@rocket.chat/fuselage-hooks';
 import { useSetting, useUserPreference } from '@rocket.chat/ui-contexts';
 import { differenceInSeconds } from 'date-fns';
 import type { ReactElement } from 'react';
-import { Fragment, useRef } from 'react';
+import { Fragment } from 'react';
 import { useTranslation } from 'react-i18next';
 
 import { ThreadMessageItem } from './ThreadMessageItem';
@@ -55,11 +54,7 @@ const ThreadMessageList = ({ mainMessage }: ThreadMessageListProps): ReactElemen
 
 	const { messages, loading } = useLegacyThreadMessages(mainMessage._id);
 
-	const {
-		listWrapperRef: listWrapperScrollRef,
-		listRef: listScrollRef,
-		onScroll: handleScroll,
-	} = useLegacyThreadMessageListScrolling(mainMessage);
+	const { innerRef: listScrollRef, jumpToRef } = useLegacyThreadMessageListScrolling(mainMessage);
 
 	const hideUsernames = useUserPreference<boolean>('hideUsernames');
 	const showUserAvatar = !!useUserPreference<boolean>('displayAvatars');
@@ -68,18 +63,14 @@ const ThreadMessageList = ({ mainMessage }: ThreadMessageListProps): ReactElemen
 
 	const { messageListRef } = useMessageListNavigation();
 
-	const ref = useRef<HTMLElement | null>(null);
-	const listRef = useMergedRefs<HTMLElement | null>(listScrollRef, messageListRef);
-	const scrollRef = useMergedRefs<HTMLElement | null>(innerRef, listWrapperScrollRef, ref);
-
 	return (
 		<div className={['thread-list js-scroll-thread', hideUsernames && 'hide-usernames'].filter(isTruthy).join(' ')}>
 			<BubbleDate ref={bubbleRef} {...bubbleDate} />
-			<CustomScrollbars ref={scrollRef} onScroll={handleScroll} style={{ scrollBehavior: 'smooth', overflowX: 'hidden' }}>
+			<CustomScrollbars ref={listScrollRef} style={{ scrollBehavior: 'smooth', overflowX: 'hidden' }}>
 				<Box
 					is='ul'
 					className={[listStyle, 'thread']}
-					ref={listRef}
+					ref={messageListRef}
 					aria-label={t('Thread_message_list')}
 					style={{ scrollBehavior: 'smooth', overflowX: 'hidden' }}
 				>
@@ -88,7 +79,7 @@ const ThreadMessageList = ({ mainMessage }: ThreadMessageListProps): ReactElemen
 							<LoadingMessagesIndicator />
 						</li>
 					) : (
-						<MessageListProvider messageListRef={ref}>
+						<MessageListProvider messageListRef={jumpToRef}>
 							{[mainMessage, ...messages].map((message, index, { [index - 1]: previous }) => {
 								const sequential = isMessageSequential(message, previous, messageGroupingPeriod);
 								const newDay = isMessageNewDay(message, previous);