diff --git a/.changeset/gentle-moons-unite.md b/.changeset/gentle-moons-unite.md new file mode 100644 index 0000000000000..7916cfb0bec1d --- /dev/null +++ b/.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. diff --git a/apps/meteor/client/views/room/body/hooks/useRestoreScrollPosition.spec.ts b/apps/meteor/client/views/room/body/hooks/useRestoreScrollPosition.spec.ts new file mode 100644 index 0000000000000..8eb34801dc2be --- /dev/null +++ b/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)); + }); +}); diff --git a/apps/meteor/client/views/room/body/hooks/useRestoreScrollPosition.ts b/apps/meteor/client/views/room/body/hooks/useRestoreScrollPosition.ts index 231fb81ab852d..2876e781a5707 100644 --- a/apps/meteor/client/views/room/body/hooks/useRestoreScrollPosition.ts +++ b/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(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, + innerRef: ref, }; }