diff --git a/apps/meteor/app/ui-utils/client/lib/RoomHistoryManager.ts b/apps/meteor/app/ui-utils/client/lib/RoomHistoryManager.ts index 67777b346718b..db11d3ece34fc 100644 --- a/apps/meteor/app/ui-utils/client/lib/RoomHistoryManager.ts +++ b/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); } diff --git a/apps/meteor/client/components/message/list/MessageListContext.tsx b/apps/meteor/client/components/message/list/MessageListContext.tsx index 64899357b2c5b..2f3b1e54c5377 100644 --- a/apps/meteor/client/components/message/list/MessageListContext.tsx +++ b/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; + messageListRef?: MutableRefObject; }; export const MessageListContext = createContext({ @@ -43,7 +43,7 @@ export const MessageListContext = createContext({ showUsername: false, showColors: false, username: undefined, - messageListRef: { current: null }, + messageListRef: { current: undefined }, }); export const useShowTranslated: MessageListContextValue['useShowTranslated'] = (...args) => diff --git a/apps/meteor/client/views/room/MessageList/hooks/useJumpToMessage.ts b/apps/meteor/client/views/room/MessageList/hooks/useJumpToMessage.ts index 2ec888f51d922..096a6cd4f7777 100644 --- a/apps/meteor/client/views/room/MessageList/hooks/useJumpToMessage.ts +++ b/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) { diff --git a/apps/meteor/client/views/room/MessageList/providers/MessageListProvider.tsx b/apps/meteor/client/views/room/MessageList/providers/MessageListProvider.tsx index 682b8b5154ae2..2cf9cc5014359 100644 --- a/apps/meteor/client/views/room/MessageList/providers/MessageListProvider.tsx +++ b/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; + messageListRef?: MutableRefObject; attachmentDimension?: { width?: number; height?: number; diff --git a/apps/meteor/client/views/room/body/RoomBody.tsx b/apps/meteor/client/views/room/body/RoomBody.tsx index 2c5e9d3c09a16..e4b64f35c5864 100644 --- a/apps/meteor/client/views/room/body/RoomBody.tsx +++ b/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(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} - + {hasMoreNextMessages ? (
  • {isLoadingMoreMessages ? : null}
    • ) : null} diff --git a/apps/meteor/client/views/room/body/RoomBodyV2.tsx b/apps/meteor/client/views/room/body/RoomBodyV2.tsx index 04ddc5da463f4..2038163f82637 100644 --- a/apps/meteor/client/views/room/body/RoomBodyV2.tsx +++ b/apps/meteor/client/views/room/body/RoomBodyV2.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 { isTruthy } from '../../../../lib/isTruthy'; import { CustomScrollbars } from '../../../components/CustomScrollbars'; @@ -81,8 +81,6 @@ const RoomBody = (): ReactElement => { return subscribed; }, [allowAnonymousRead, canPreviewChannelRoom, room, subscribed]); - const innerBoxRef = useRef(null); - const { wrapperRef, 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, @@ -248,7 +245,7 @@ const RoomBody = (): ReactElement => { )} ) : null} - + {hasMoreNextMessages ? (
  • {isLoadingMoreMessages ? : null}
    • ) : null} diff --git a/apps/meteor/client/views/room/body/hooks/useListIsAtBottom.ts b/apps/meteor/client/views/room/body/hooks/useListIsAtBottom.ts index fc67c50d9fd83..7425a43e53c42 100644 --- a/apps/meteor/client/views/room/body/hooks/useListIsAtBottom.ts +++ b/apps/meteor/client/views/room/body/hooks/useListIsAtBottom.ts @@ -9,6 +9,8 @@ import { useSafeRefCallback } from '../../../../hooks/useSafeRefCallback'; export const useListIsAtBottom = () => { const atBottomRef = useRef(true); + const jumpToRef = useRef(undefined); + const innerBoxRef = useRef(null); const sendToBottom = useCallback(() => { @@ -16,6 +18,9 @@ export const useListIsAtBottom = () => { }, []); 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, }; }; diff --git a/apps/meteor/client/views/room/body/hooks/useRestoreScrollPosition.spec.ts b/apps/meteor/client/views/room/body/hooks/useRestoreScrollPosition.spec.ts index 5444ac71e9818..30bfd0322008e 100644 --- a/apps/meteor/client/views/room/body/hooks/useRestoreScrollPosition.spec.ts +++ b/apps/meteor/client/views/room/body/hooks/useRestoreScrollPosition.spec.ts @@ -33,13 +33,14 @@ 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 }); @@ -47,7 +48,7 @@ describe('useRestoreScrollPosition', () => { const { unmount } = renderHook(() => useRestoreScrollPosition()); expect(useRefSpy).toHaveBeenCalledWith(null); - expect(mockElement).toHaveProperty('scrollTop', 800); + expect(mockElement).toHaveProperty('scrollTop', 123); expect(mockElement).not.toHaveProperty('scrollLeft'); unmount(); diff --git a/apps/meteor/client/views/room/body/hooks/useRestoreScrollPosition.ts b/apps/meteor/client/views/room/body/hooks/useRestoreScrollPosition.ts index 77c03bbed39c9..f4a54699d3625 100644 --- a/apps/meteor/client/views/room/body/hooks/useRestoreScrollPosition.ts +++ b/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]); diff --git a/apps/meteor/client/views/room/contextualBar/Threads/components/ThreadMessageList.tsx b/apps/meteor/client/views/room/contextualBar/Threads/components/ThreadMessageList.tsx index ed0ed10a488cc..b52e8c7b0cc57 100644 --- a/apps/meteor/client/views/room/contextualBar/Threads/components/ThreadMessageList.tsx +++ b/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('hideUsernames'); const showUserAvatar = !!useUserPreference('displayAvatars'); @@ -68,18 +63,14 @@ const ThreadMessageList = ({ mainMessage }: ThreadMessageListProps): ReactElemen const { messageListRef } = useMessageListNavigation(); - const ref = useRef(null); - const listRef = useMergedRefs(listScrollRef, messageListRef); - const scrollRef = useMergedRefs(innerRef, listWrapperScrollRef, ref); - return (
    - + @@ -88,7 +79,7 @@ const ThreadMessageList = ({ mainMessage }: ThreadMessageListProps): ReactElemen ) : ( - + {[mainMessage, ...messages].map((message, index, { [index - 1]: previous }) => { const sequential = isMessageSequential(message, previous, messageGroupingPeriod); const newDay = isMessageNewDay(message, previous); diff --git a/apps/meteor/client/views/room/contextualBar/Threads/hooks/useLegacyThreadMessageListScrolling.ts b/apps/meteor/client/views/room/contextualBar/Threads/hooks/useLegacyThreadMessageListScrolling.ts index e7b6dabfe7d69..51ccda787b542 100644 --- a/apps/meteor/client/views/room/contextualBar/Threads/hooks/useLegacyThreadMessageListScrolling.ts +++ b/apps/meteor/client/views/room/contextualBar/Threads/hooks/useLegacyThreadMessageListScrolling.ts @@ -1,36 +1,16 @@ import type { IMessage } from '@rocket.chat/core-typings'; import { isEditedMessage } from '@rocket.chat/core-typings'; -import { useUser } from '@rocket.chat/ui-contexts'; -import { useCallback, useEffect, useRef } from 'react'; +import { useUserId } from '@rocket.chat/ui-contexts'; +import { useEffect } from 'react'; import { callbacks } from '../../../../../../lib/callbacks'; -import type { OverlayScrollbars } from '../../../../../components/CustomScrollbars'; +import { useListIsAtBottom } from '../../../body/hooks/useListIsAtBottom'; import { useRoom } from '../../../contexts/RoomContext'; export const useLegacyThreadMessageListScrolling = (mainMessage: IMessage) => { - const listWrapperRef = useRef(null); - const listRef = useRef(null); - - const atBottomRef = useRef(true); - - const onScroll = useCallback(({ elements }: OverlayScrollbars) => { - const { - viewport: { scrollTop, scrollHeight, clientHeight }, - } = elements(); - atBottomRef.current = scrollTop >= scrollHeight - clientHeight; - }, []); - - const sendToBottomIfNecessary = useCallback(() => { - if (atBottomRef.current === true) { - const listWrapper = listWrapperRef.current; - - listWrapper?.scrollTo(30, listWrapper.scrollHeight); - } - }, []); - + const { atBottomRef, innerRef, sendToBottom, sendToBottomIfNecessary, isAtBottom, jumpToRef } = useListIsAtBottom(); const room = useRoom(); - const user = useUser(); - + const uid = useUserId(); useEffect(() => { callbacks.add( 'streamNewMessage', @@ -39,7 +19,7 @@ export const useLegacyThreadMessageListScrolling = (mainMessage: IMessage) => { return; } - if (msg.u._id === user?._id) { + if (msg.u._id === uid) { atBottomRef.current = true; sendToBottomIfNecessary(); } @@ -51,20 +31,7 @@ export const useLegacyThreadMessageListScrolling = (mainMessage: IMessage) => { return () => { callbacks.remove('streamNewMessage', `thread-scroll-${room._id}`); }; - }, [room._id, sendToBottomIfNecessary, user?._id, mainMessage._id]); - - useEffect(() => { - const observer = new ResizeObserver(() => { - sendToBottomIfNecessary(); - }); - - if (listWrapperRef.current) observer.observe(listWrapperRef.current); - if (listRef.current) observer.observe(listRef.current); - - return () => { - observer.disconnect(); - }; - }, [sendToBottomIfNecessary]); + }, [room._id, atBottomRef, sendToBottomIfNecessary, uid, mainMessage._id]); - return { listWrapperRef, listRef, requestScrollToBottom: sendToBottomIfNecessary, onScroll }; + return { atBottomRef, innerRef, sendToBottom, sendToBottomIfNecessary, isAtBottom, jumpToRef }; }; diff --git a/apps/meteor/tests/e2e/jump-to-thread-message.spec.ts b/apps/meteor/tests/e2e/jump-to-thread-message.spec.ts index 5bcb23d76d342..fd92d933ae5a0 100644 --- a/apps/meteor/tests/e2e/jump-to-thread-message.spec.ts +++ b/apps/meteor/tests/e2e/jump-to-thread-message.spec.ts @@ -47,6 +47,11 @@ test.describe.serial('Threads', () => { const messageLink = `/channel/${targetChannel.name}?msg=${mainMessage._id}`; await page.goto(messageLink); + await expect(async () => { + await page.waitForSelector('.main-content'); + await page.waitForSelector(`[data-qa-rc-room="${targetChannel._id}"]`); + }).toPass(); + const message = page.locator(`[aria-label=\"Message list\"] [data-id=\"${mainMessage._id}\"]`); await expect(message).toBeVisible(); @@ -54,9 +59,29 @@ test.describe.serial('Threads', () => { }); test('expect to jump scroll to thread message on opening its message link', async ({ page }) => { + const threadMessageLink = `/channel/${targetChannel.name}?msg=${threadMessage._id}`; + await page.goto(threadMessageLink); + + await expect(async () => { + await page.waitForSelector('.main-content'); + await page.waitForSelector(`[data-qa-rc-room="${targetChannel._id}"]`); + }).toPass(); + + const message = await page.locator(`[aria-label=\"Thread message list\"] [data-id=\"${threadMessage._id}\"]`); + + await expect(message).toBeVisible(); + await expect(message).toBeInViewport(); + }); + + test('expect to jump scroll to thread message on opening its message link from a different channel', async ({ page }) => { const threadMessageLink = `/channel/general?msg=${threadMessage._id}`; await page.goto(threadMessageLink); + await expect(async () => { + await page.waitForSelector('.main-content'); + await page.waitForSelector(`[data-qa-rc-room="${targetChannel._id}"]`); + }).toPass(); + const message = await page.locator(`[aria-label=\"Thread message list\"] [data-id=\"${threadMessage._id}\"]`); await expect(message).toBeVisible();