Skip to content

regression: jump to message #35918

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ggazzo merged 4 commits into from
May 5, 2025
Merged

regression: jump to message #35918

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c181236
wip
ggazzo May 2, 2025
575f16b
fix: delete room state after logout
ggazzo May 5, 2025
e9ce201
just make sure we dont do anything if the element is detached
ggazzo May 5, 2025
a00d29b
review
ggazzo May 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions apps/meteor/app/ui-utils/client/lib/LegacyRoomManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ function close(typeName: string) {

if (rid) {
RoomManager.close(rid);
return RoomHistoryManager.clear(rid);
return RoomHistoryManager.close(rid);
}
}
}
Expand Down Expand Up @@ -93,8 +93,6 @@ const computation = Tracker.autorun(() => {

const room = roomCoordinator.getRoomDirectives(type).findRoom(name);

void RoomHistoryManager.getMoreIfIsEmpty(record.rid);

if (room) {
if (record.streamActive !== true) {
void sdk
Expand Down
21 changes: 19 additions & 2 deletions apps/meteor/app/ui-utils/client/lib/RoomHistoryManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ class RoomHistoryManagerClass extends Emitter {

public getRoom(rid: IRoom['_id']) {
if (!this.histories[rid]) {
console.log('create room history', rid, new Error().stack);
Comment thread
ggazzo marked this conversation as resolved.
Outdated
this.histories[rid] = {
hasMore: new ReactiveVar(true),
hasMoreNext: new ReactiveVar(false),
Expand Down Expand Up @@ -122,6 +123,11 @@ class RoomHistoryManagerClass extends Emitter {
return setTimeout(fn, 500 - difference);
}

public isLoaded(rid: IRoom['_id']) {
const room = this.getRoom(rid);
return room.loaded !== undefined;
}

private unqueue() {
const requestId = this.requestsList.pop();
if (!requestId) {
Expand Down Expand Up @@ -286,10 +292,15 @@ class RoomHistoryManagerClass extends Emitter {
return room.isLoading.get();
}

public close(rid: IRoom['_id']) {
Messages.remove({ rid });
delete this.histories[rid];
}

public clear(rid: IRoom['_id']) {
const room = this.getRoom(rid);
Messages.remove({ rid });
room.isLoading.set(true);
room.isLoading.set(false);
room.hasMore.set(true);
room.hasMoreNext.set(false);
room.oldestTs = undefined;
Expand All @@ -308,15 +319,21 @@ class RoomHistoryManagerClass extends Emitter {
}

const room = this.getRoom(message.rid);
this.clear(message.rid);

const subscription = Subscriptions.findOne({ rid: message.rid });

const result = await callWithErrorHandling('loadSurroundingMessages', message, defaultLimit);

this.clear(message.rid);

if (!result) {
return;
}
const { messages = [] } = result;

if (messages.length > 0) {
room.oldestTs = messages[messages.length - 1].ts;
}

await upsertMessageBulk({ msgs: Array.from(result.messages).filter((msg) => msg.t !== 'command'), subscription });

Expand Down
17 changes: 17 additions & 0 deletions apps/meteor/app/ui/client/views/app/lib/scrolling.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,20 @@
export function isAtBottom(element: HTMLElement, scrollThreshold = 0): boolean {
return element.scrollTop + scrollThreshold >= element.scrollHeight - element.clientHeight;
}

// Mainly used for allow mock during testing

export const getBoundingClientRect = (ref: HTMLElement) => {
const { top, bottom, left, right } = ref.getBoundingClientRect();
const { scrollTop, scrollHeight, clientHeight } = ref;

return {
top,
bottom,
left,
right,
scrollTop,
scrollHeight,
clientHeight,
};
};
3 changes: 2 additions & 1 deletion apps/meteor/client/lib/utils/legacyJumpToMessage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ export const legacyJumpToMessage = async (message: IMessage) => {
if (tab === 'thread' && (context === message.tmid || context === message._id)) {
return;
}

router.navigate(
{
name: router.getRouteName()!,
Expand All @@ -32,6 +31,8 @@ export const legacyJumpToMessage = async (message: IMessage) => {
},
{ replace: false },
);
await RoomHistoryManager.getSurroundingMessages(message);
Comment thread
kody-ai[bot] marked this conversation as resolved.

return;
}

Expand Down
39 changes: 33 additions & 6 deletions apps/meteor/client/views/room/MessageList/hooks/useJumpToMessage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,44 @@
import type { IMessage } from '@rocket.chat/core-typings';
import { useMergedRefs } from '@rocket.chat/fuselage-hooks';
import { useSafeRefCallback } from '@rocket.chat/ui-client';
import { useRouter } from '@rocket.chat/ui-contexts';
import { useCallback } from 'react';
import { useCallback, useRef } from 'react';

import { useMessageListJumpToMessageParam, useMessageListRef } from '../../../../components/message/list/MessageListContext';
import { setRef } from '../../composer/hooks/useMessageComposerMergedRefs';
import { setHighlightMessage, clearHighlightMessage } from '../providers/messageHighlightSubscription';

// this is an arbitrary value so that there's a gap between the header and the message;
/**
* That is completely messy, CustomScrollbars force us to initialize the scrollbars inside an effect
* all refCallbacks happen before the effect, more than that, the scrollbars also reset the scroll position
* so we need to check if the scrollbars are initialized and if there is any message to be highlighted
*/
Comment thread
MartinSchoeler marked this conversation as resolved.

export const useJumpToMessageImperative = () => {
const jumpToRef = useRef<HTMLDivElement>(null);
const containerRef = useRef<HTMLDivElement>(null);

const jumpToRefAction = useCallback(() => {
if (!jumpToRef.current || !containerRef.current) {
return;
}
jumpToRef.current.scrollIntoView({
block: 'center',
});
}, []);
Comment thread
ggazzo marked this conversation as resolved.

return {
jumpToRef: useMergedRefs(jumpToRef, jumpToRefAction),
innerRef: useMergedRefs(containerRef, jumpToRefAction),
};
};

/**
* `listRef` is a reference to the message node in the message list.
* its shared between other hooks like `useLoadSurroundingMessages`, `useJumpToMessage`, `useGetMore`, `useListIsAtBottom` and `useRestoreScrollPosition`
* since each hook has a different concern, this ref helps each other aware if a message is being highlighted which changes the scroll position

*/

export const useJumpToMessage = (messageId: IMessage['_id']) => {
const jumpToMessageParam = useMessageListJumpToMessageParam();
Expand All @@ -27,10 +58,6 @@ export const useJumpToMessage = (messageId: IMessage['_id']) => {

setRef(listRef, node);

node.scrollIntoView({
behavior: 'smooth',
block: 'center',
});
const handleScroll = () => {
const { msg: _, ...search } = router.getSearchParameters();
router.navigate(
Expand Down
16 changes: 13 additions & 3 deletions apps/meteor/client/views/room/MessageList/hooks/useLoadSurroundingMessages.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,26 @@
import type { IMessage } from '@rocket.chat/core-typings';
import { useEndpoint } from '@rocket.chat/ui-contexts';
import { useEndpoint, useSearchParameter } from '@rocket.chat/ui-contexts';
import { useQueryClient } from '@tanstack/react-query';
import { useEffect } from 'react';
import { useEffect, useRef } from 'react';

import { legacyJumpToMessage } from '../../../../lib/utils/legacyJumpToMessage';

export const useLoadSurroundingMessages = (msgId?: IMessage['_id']) => {
export const useLoadSurroundingMessages = () => {
Comment thread
ggazzo marked this conversation as resolved.
const msgId = useSearchParameter('msg');
const jumpToRef = useRef<HTMLElement>(undefined);

const queryClient = useQueryClient();
const getMessage = useEndpoint('GET', '/v1/chat.getMessage');

useEffect(() => {
if (!msgId) {
return;
}

if (jumpToRef.current) {
return;
}

const abort = new AbortController();

queryClient
Expand All @@ -36,4 +44,6 @@ export const useLoadSurroundingMessages = (msgId?: IMessage['_id']) => {
abort.abort();
};
}, [msgId, queryClient, getMessage]);

return { jumpToRef };
};
3 changes: 0 additions & 3 deletions apps/meteor/client/views/room/MessageList/providers/MessageListProvider.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import { useChat } from '../../contexts/ChatContext';
import { useRoom, useRoomSubscription } from '../../contexts/RoomContext';
import { useAutoTranslate } from '../hooks/useAutoTranslate';
import { useKatex } from '../hooks/useKatex';
import { useLoadSurroundingMessages } from '../hooks/useLoadSurroundingMessages';

type MessageListProviderProps = {
children: ReactNode;
Expand Down Expand Up @@ -62,8 +61,6 @@ const MessageListProvider = ({ children, messageListRef, attachmentDimension }:
const hasSubscription = Boolean(subscription);
const msgParameter = useSearchParameter('msg');

useLoadSurroundingMessages(msgParameter);

const chat = useChat();

const context: MessageListContextValue = useMemo(
Expand Down
21 changes: 17 additions & 4 deletions apps/meteor/client/views/room/body/RoomBody.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ import { useListIsAtBottom } from './hooks/useListIsAtBottom';
import { useQuoteMessageByUrl } from './hooks/useQuoteMessageByUrl';
import { useRestoreScrollPosition } from './hooks/useRestoreScrollPosition';
import { useHandleUnread } from './hooks/useUnreadMessages';
import { useJumpToMessageImperative } from '../MessageList/hooks/useJumpToMessage';
import { useLoadSurroundingMessages } from '../MessageList/hooks/useLoadSurroundingMessages';

const RoomBody = (): ReactElement => {
const chat = useChat();
Expand Down Expand Up @@ -81,6 +83,10 @@ const RoomBody = (): ReactElement => {
return subscribed;
}, [allowAnonymousRead, canPreviewChannelRoom, room, subscribed]);

const { jumpToRef: jumpToRefGetMoreImperative, innerRef: jumpToRefGetMoreImperativeInnerRef } = useJumpToMessageImperative();

const { jumpToRef: surroundingMessagesJumpTpRef } = useLoadSurroundingMessages();

const {
wrapperRef: unreadBarWrapperRef,
innerRef: unreadBarInnerRef,
Expand All @@ -102,7 +108,15 @@ const RoomBody = (): ReactElement => {

const { innerRef: getMoreInnerRef, jumpToRef: jumpToRefGetMore } = useGetMore(room._id, atBottomRef);

const jumpToRef = useMergedRefs(jumpToRefGetMore, jumpToRefIsAtBottom);
const { innerRef: restoreScrollPositionInnerRef, jumpToRef: jumpToRefRestoreScrollPosition } = useRestoreScrollPosition(room._id);

const jumpToRef = useMergedRefs(
jumpToRefGetMore,
jumpToRefIsAtBottom,
jumpToRefRestoreScrollPosition,
surroundingMessagesJumpTpRef,
jumpToRefGetMoreImperative,
);

const {
uploads,
Expand All @@ -111,8 +125,6 @@ const RoomBody = (): ReactElement => {
targeDrop: [fileUploadTriggerProps, fileUploadOverlayProps],
} = useFileUpload();

const { innerRef: restoreScrollPositionInnerRef } = useRestoreScrollPosition();

const { messageListRef } = useMessageListNavigation();
const { innerRef: selectAndScrollRef, selectAllAndScrollToTop } = useSelectAllAndScrollToTop();

Expand All @@ -132,6 +144,7 @@ const RoomBody = (): ReactElement => {
getMoreInnerRef,
selectAndScrollRef,
messageListRef,
jumpToRefGetMoreImperativeInnerRef,
);

const wrapperBoxRefs = useMergedRefs(unreadBarWrapperRef);
Expand Down Expand Up @@ -237,7 +250,7 @@ const RoomBody = (): ReactElement => {
.join(' ')}
>
<MessageListErrorBoundary>
<CustomScrollbars ref={innerRef}>
<CustomScrollbars ref={innerRef} key={room._id}>
<ul className='messages-list' aria-label={t('Message_list')} aria-busy={isLoadingMoreMessages}>
{canPreview ? (
<>
Expand Down
21 changes: 17 additions & 4 deletions apps/meteor/client/views/room/body/RoomBodyV2.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ import { useListIsAtBottom } from './hooks/useListIsAtBottom';
import { useRestoreScrollPosition } from './hooks/useRestoreScrollPosition';
import { useSelectAllAndScrollToTop } from './hooks/useSelectAllAndScrollToTop';
import { useHandleUnread } from './hooks/useUnreadMessages';
import { useJumpToMessageImperative } from '../MessageList/hooks/useJumpToMessage';
import { useLoadSurroundingMessages } from '../MessageList/hooks/useLoadSurroundingMessages';

const RoomBody = (): ReactElement => {
const chat = useChat();
Expand Down Expand Up @@ -81,6 +83,10 @@ const RoomBody = (): ReactElement => {
return subscribed;
}, [allowAnonymousRead, canPreviewChannelRoom, room, subscribed]);

const { jumpToRef: jumpToRefGetMoreImperative, innerRef: jumpToRefGetMoreImperativeInnerRef } = useJumpToMessageImperative();

const { jumpToRef: surroundingMessagesJumpTpRef } = useLoadSurroundingMessages();

const {
wrapperRef,
innerRef: unreadBarInnerRef,
Expand All @@ -102,7 +108,15 @@ const RoomBody = (): ReactElement => {

const { innerRef: getMoreInnerRef, jumpToRef: jumpToRefGetMore } = useGetMore(room._id, atBottomRef);

const jumpToRef = useMergedRefs(jumpToRefIsAtBottom, jumpToRefGetMore);
const { innerRef: restoreScrollPositionInnerRef, jumpToRef: jumpToRefRestoreScrollPosition } = useRestoreScrollPosition(room._id);

const jumpToRef = useMergedRefs(
jumpToRefIsAtBottom,
jumpToRefGetMore,
jumpToRefRestoreScrollPosition,
jumpToRefGetMoreImperative,
surroundingMessagesJumpTpRef,
);

const {
uploads,
Expand All @@ -111,8 +125,6 @@ const RoomBody = (): ReactElement => {
targeDrop: [fileUploadTriggerProps, fileUploadOverlayProps],
} = useFileUpload();

const { innerRef: restoreScrollPositionInnerRef } = useRestoreScrollPosition();

const { messageListRef } = useMessageListNavigation();
const { innerRef: selectAndScrollRef, selectAllAndScrollToTop } = useSelectAllAndScrollToTop();

Expand All @@ -132,6 +144,7 @@ const RoomBody = (): ReactElement => {
getMoreInnerRef,
selectAndScrollRef,
messageListRef,
jumpToRefGetMoreImperativeInnerRef,
);

const handleNavigateToPreviousMessage = useCallback((): void => {
Expand Down Expand Up @@ -240,7 +253,7 @@ const RoomBody = (): ReactElement => {
.join(' ')}
>
<MessageListErrorBoundary>
<CustomScrollbars ref={innerRef}>
<CustomScrollbars ref={innerRef} key={room._id}>
<ul className='messages-list' aria-label={t('Message_list')} aria-busy={isLoadingMoreMessages}>
{canPreview ? (
<>
Expand Down
Loading