fix: Restore room scroll position (#34908)

Author: yash-rajpal

.changeset/gentle-moons-unite.md

@@ -0,0 +1,5 @@
+---
+'@rocket.chat/meteor': patch
+---
+
+Fixes an issue where room scroll position wasn't being restored when moving between rooms.

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

@@ -0,0 +1,82 @@
+import { renderHook } from '@testing-library/react';
+import React from 'react';
+
+import { useRestoreScrollPosition } from './useRestoreScrollPosition';
+import { RoomManager } from '../../../../lib/RoomManager';
+
+jest.mock('../../../../lib/RoomManager', () => ({
+	RoomManager: {
+		getStore: jest.fn(),
+	},
+}));
+
+describe('useRestoreScrollPosition', () => {
+	it('should restore room scroll position based on store', () => {
+		(RoomManager.getStore as jest.Mock).mockReturnValue({ scroll: 100, atBottom: false });
+
+		const mockElement = {
+			addEventListener: jest.fn(),
+			removeEventListener: jest.fn(),
+		};
+
+		const useRefSpy = jest.spyOn(React, 'useRef').mockReturnValueOnce({ current: mockElement });
+
+		const { unmount } = renderHook(() => useRestoreScrollPosition('room-id'), { legacyRoot: true });
+
+		expect(useRefSpy).toHaveBeenCalledWith(null);
+		expect(mockElement).toHaveProperty('scrollTop', 100);
+		expect(mockElement).toHaveProperty('scrollLeft', 30);
+
+		unmount();
+		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 });
+
+		const mockElement = {
+			addEventListener: jest.fn(),
+			removeEventListener: jest.fn(),
+			scrollHeight: 800,
+		};
+
+		const useRefSpy = jest.spyOn(React, 'useRef').mockReturnValueOnce({ current: mockElement });
+
+		const { unmount } = renderHook(() => useRestoreScrollPosition('room-id'), { legacyRoot: true });
+
+		expect(useRefSpy).toHaveBeenCalledWith(null);
+		expect(mockElement).toHaveProperty('scrollTop', 800);
+		expect(mockElement).not.toHaveProperty('scrollLeft');
+
+		unmount();
+		expect(mockElement.removeEventListener).toHaveBeenCalledWith('scroll', expect.any(Function));
+	});
+
+	it('should update store based on scroll position', () => {
+		const update = jest.fn();
+		(RoomManager.getStore as jest.Mock).mockReturnValue({ update });
+
+		const mockElement = {
+			addEventListener: jest.fn((event, handler) => {
+				if (event === 'scroll') {
+					handler({
+						target: {
+							scrollTop: 500,
+						},
+					});
+				}
+			}),
+			removeEventListener: jest.fn(),
+		};
+
+		const useRefSpy = jest.spyOn(React, 'useRef').mockReturnValueOnce({ current: mockElement });
+
+		const { unmount } = renderHook(() => useRestoreScrollPosition('room-id'), { legacyRoot: true });
+
+		expect(useRefSpy).toHaveBeenCalledWith(null);
+		expect(update).toHaveBeenCalledWith({ scroll: 500, atBottom: false });
+
+		unmount();
+		expect(mockElement.removeEventListener).toHaveBeenCalledWith('scroll', expect.any(Function));
+	});
+});

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

@@ -1,54 +1,50 @@
 import type { IRoom } from '@rocket.chat/core-typings';
-import { useMergedRefs } from '@rocket.chat/fuselage-hooks';
-import type { RefObject } from 'react';
-import { useCallback } from 'react';
+import { useCallback, useEffect, useRef } from 'react';
 
 import { isAtBottom } from '../../../../../app/ui/client/views/app/lib/scrolling';
 import { withThrottling } from '../../../../../lib/utils/highOrderFunctions';
 import { RoomManager } from '../../../../lib/RoomManager';
 
 export function useRestoreScrollPosition(roomId: IRoom['_id']) {
-	const ref = useCallback(
-		(node: HTMLElement | null) => {
-			if (!node) {
-				return;
-			}
-			const store = RoomManager.getStore(roomId);
-
-			if (store?.scroll && !store.atBottom) {
-				node.scrollTo({
-					left: 30,
-					top: store.scroll,
-				});
-			} else {
-				node.scrollTo({
-					top: node.scrollHeight,
-				});
-			}
-		},
-		[roomId],
-	);
-
-	const refCallback = useCallback(
-		(node: HTMLElement | null) => {
-			if (!node) {
-				return;
-			}
-
-			const store = RoomManager.getStore(roomId);
-
-			const handleWrapperScroll = withThrottling({ wait: 100 })(() => {
-				store?.update({ scroll: node.scrollTop, atBottom: isAtBottom(node, 50) });
-			});
-
-			node.addEventListener('scroll', handleWrapperScroll, {
-				passive: true,
-			});
-		},
-		[roomId],
-	);
+	const ref = useRef<HTMLElement>(null);
+
+	const handleRestoreScroll = useCallback(() => {
+		if (!ref.current) {
+			return;
+		}
+
+		const store = RoomManager.getStore(roomId);
+
+		if (store?.scroll && !store.atBottom) {
+			ref.current.scrollTop = store.scroll;
+			ref.current.scrollLeft = 30;
+		} else {
+			ref.current.scrollTop = ref.current.scrollHeight;
+		}
+	}, [roomId]);
+
+	useEffect(() => {
+		if (!ref.current) {
+			return;
+		}
+
+		handleRestoreScroll();
+
+		const refValue = ref.current;
+		const store = RoomManager.getStore(roomId);
+
+		const handleWrapperScroll = withThrottling({ wait: 100 })((event) => {
+			store?.update({ scroll: event.target.scrollTop, atBottom: isAtBottom(event.target, 50) });
+		});
+
+		refValue.addEventListener('scroll', handleWrapperScroll, { passive: true });
+
+		return () => {
+			refValue.removeEventListener('scroll', handleWrapperScroll);
+		};
+	}, [roomId, handleRestoreScroll]);
 
 	return {
-		innerRef: useMergedRefs(refCallback, ref) as unknown as RefObject<HTMLElement>,
+		innerRef: ref,
 	};
 }