Skip to content

Commit 42368b1

Browse files
reviews: Fix bot reviews and issue with jump to message
1 parent 2bb25fc commit 42368b1

3 files changed

Lines changed: 72 additions & 14 deletions

File tree

apps/meteor/app/api/server/v1/chat.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1163,7 +1163,7 @@ const chatEndpoints = API.v1
11631163
} else {
11641164
const target = await Messages.findOneById(aroundId, { projection: { ts: 1, tmid: 1 } });
11651165
if (target?.tmid === tmid && target.ts) {
1166-
const before = await Messages.countDocuments({ tmid, ts: { $lt: target.ts } });
1166+
const before = await Messages.countDocuments({ ...query, tmid, ts: { $lt: target.ts } });
11671167
resolvedOffset = Math.max(0, before - Math.floor(count / 2));
11681168
}
11691169
}

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

Lines changed: 60 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
import type { IMessage, IThreadMainMessage } from '@rocket.chat/core-typings';
22
import { isEditedMessage } from '@rocket.chat/core-typings';
3-
import { useDebouncedCallback } from '@rocket.chat/fuselage-hooks';
3+
import { useDebouncedCallback, useSafeRefCallback } from '@rocket.chat/fuselage-hooks';
44
import { MessageTypes } from '@rocket.chat/message-types';
55
import { isTruthy } from '@rocket.chat/tools';
66
import { clientCallbacks, CustomVirtuaScrollbars } from '@rocket.chat/ui-client';
77
import { useSearchParameter, useSetting, useUserId, useUserPreference } from '@rocket.chat/ui-contexts';
88
import { differenceInSeconds } from 'date-fns';
9-
import { Fragment, useEffect, useLayoutEffect, useMemo, useRef } from 'react';
9+
import { Fragment, useCallback, useEffect, useLayoutEffect, useMemo, useRef } from 'react';
1010
import { useTranslation } from 'react-i18next';
1111
import type { VirtualizerHandle } from 'virtua';
1212
import { VList } from 'virtua';
@@ -76,9 +76,11 @@ const ThreadMessageList = ({ mainMessage, shouldJumpToBottom, setShouldJumpToBot
7676
} = useThreadMessagesQuery(mainMessage._id);
7777
const messages = useMemo(() => data?.messages ?? [], [data?.messages]);
7878

79+
const userInteractedRef = useRef(false);
80+
7981
const loadMoreMessages = useDebouncedCallback(
8082
() => {
81-
if (hasNextPage && !isFetchingNextPage) {
83+
if (userInteractedRef.current && hasNextPage && !isFetchingNextPage) {
8284
void fetchNextPage();
8385
}
8486
},
@@ -90,7 +92,13 @@ const ThreadMessageList = ({ mainMessage, shouldJumpToBottom, setShouldJumpToBot
9092

9193
const loadPreviousMessages = useDebouncedCallback(
9294
() => {
93-
if (initialScrollDoneRef.current && isAtBottom.current !== true && hasPreviousPage && !isFetchingPreviousPage) {
95+
if (
96+
userInteractedRef.current &&
97+
initialScrollDoneRef.current &&
98+
isAtBottom.current !== true &&
99+
hasPreviousPage &&
100+
!isFetchingPreviousPage
101+
) {
94102
void fetchPreviousPage();
95103
}
96104
},
@@ -129,7 +137,7 @@ const ThreadMessageList = ({ mainMessage, shouldJumpToBottom, setShouldJumpToBot
129137
if (loading || !msgJumpParam || isFetchingNextPage || isFetchingPreviousPage) {
130138
return;
131139
}
132-
if (msgJumpParam === mainMessage._id) {
140+
if (msgJumpParam === mainMessage._id && !hasPreviousPage) {
133141
return;
134142
}
135143
if (messages.some((message) => message._id === msgJumpParam)) {
@@ -141,9 +149,30 @@ const ThreadMessageList = ({ mainMessage, shouldJumpToBottom, setShouldJumpToBot
141149
}
142150
loadingWindowKeyRef.current = windowKey;
143151
void loadMessageAround(msgJumpParam);
144-
}, [loading, isFetchingNextPage, isFetchingPreviousPage, msgJumpParam, messages, mainMessage._id, loadMessageAround]);
152+
}, [loading, isFetchingNextPage, isFetchingPreviousPage, msgJumpParam, messages, mainMessage._id, loadMessageAround, hasPreviousPage]);
153+
154+
const interactionRef = useSafeRefCallback(
155+
useCallback((element: HTMLDivElement) => {
156+
const markInteracted = () => {
157+
userInteractedRef.current = true;
158+
};
159+
const handleKeydown = (e: KeyboardEvent) => {
160+
if (['PageUp', 'PageDown', 'ArrowUp', 'ArrowDown', 'Home', 'End'].includes(e.key)) {
161+
userInteractedRef.current = true;
162+
}
163+
};
164+
element.addEventListener('wheel', markInteracted, { passive: true });
165+
element.addEventListener('touchmove', markInteracted, { passive: true });
166+
element.addEventListener('keydown', handleKeydown);
167+
return () => {
168+
element.removeEventListener('wheel', markInteracted);
169+
element.removeEventListener('touchmove', markInteracted);
170+
element.removeEventListener('keydown', handleKeydown);
171+
};
172+
}, []),
173+
);
145174

146-
const mergedRefs = useMergedRefsV2(messageListRef, keepAtBottomRef);
175+
const mergedRefs = useMergedRefsV2(messageListRef, keepAtBottomRef, interactionRef);
147176

148177
const lastScrollSizeRef = useRef(0);
149178

@@ -173,14 +202,16 @@ const ThreadMessageList = ({ mainMessage, shouldJumpToBottom, setShouldJumpToBot
173202
return -1;
174203
}
175204
if (msgJumpParam === mainMessage._id) {
176-
return 0;
205+
return showMainMessage ? 0 : -1;
177206
}
178207
const replyIndex = messages.findIndex((m) => m._id === msgJumpParam);
179208
if (replyIndex < 0) {
180209
return -1;
181210
}
182-
return showMainMessage ? 1 + replyIndex : replyIndex;
183-
}, [msgJumpParam, loading, mainMessage._id, messages, showMainMessage]);
211+
const mainMessageOffset = showMainMessage ? 1 : 0;
212+
const previousPageLoaderOffset = hasPreviousPage ? 1 : 0;
213+
return mainMessageOffset + previousPageLoaderOffset + replyIndex;
214+
}, [msgJumpParam, loading, mainMessage._id, messages, showMainMessage, hasPreviousPage]);
184215

185216
const lastThreadJumpKeyRef = useRef<string | undefined>(undefined);
186217

@@ -189,6 +220,7 @@ const ThreadMessageList = ({ mainMessage, shouldJumpToBottom, setShouldJumpToBot
189220
loadingWindowKeyRef.current = undefined;
190221
prevItemsLengthRef.current = 0;
191222
initialScrollDoneRef.current = false;
223+
userInteractedRef.current = false;
192224
}, [mainMessage._id]);
193225

194226
useEffect(() => {
@@ -241,12 +273,28 @@ const ThreadMessageList = ({ mainMessage, shouldJumpToBottom, setShouldJumpToBot
241273
}
242274
lastThreadJumpKeyRef.current = jumpKey;
243275
setShouldJumpToBottom(false);
244-
handle.scrollToIndex(threadMsgTargetIndex, { align: 'center' });
245276
initialScrollDoneRef.current = true;
277+
278+
let frame = 0;
279+
let rafId = 0;
280+
const recenter = () => {
281+
handle.scrollToIndex(threadMsgTargetIndex, { align: 'center' });
282+
frame += 1;
283+
if (frame < 8) {
284+
rafId = requestAnimationFrame(recenter);
285+
}
286+
};
287+
recenter();
288+
246289
setHighlightMessage(msgJumpParam);
247-
setTimeout(() => {
290+
const highlightTimeout = setTimeout(() => {
248291
clearHighlightMessage();
249292
}, 2000);
293+
294+
return () => {
295+
cancelAnimationFrame(rafId);
296+
clearTimeout(highlightTimeout);
297+
};
250298
}, [threadMsgTargetIndex, msgJumpParam, mainMessage._id, setShouldJumpToBottom]);
251299

252300
useEffect(() => {

apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadMessagesQuery.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,12 @@ export const useThreadMessagesQuery = (tmid: IThreadMainMessage['_id'], rid?: IR
117117
async (messageId: string) => {
118118
const currentQueryKey = roomsQueryKeys.threadMessages(roomId, tmid);
119119

120+
await queryClient.cancelQueries({ queryKey: currentQueryKey });
121+
queryClient.setQueryData<ThreadMessagesInfiniteData>(currentQueryKey, {
122+
pages: [],
123+
pageParams: [],
124+
});
125+
120126
const { messages, total, offset } = await getThreadMessages({
121127
tmid,
122128
aroundId: messageId,
@@ -177,7 +183,11 @@ export const useThreadMessagesQuery = (tmid: IThreadMainMessage['_id'], rid?: IR
177183
return lastPageParam > 0 ? Math.max(0, lastPageParam - count) : undefined;
178184
},
179185
getPreviousPageParam: (firstPage, _allPages, firstPageParam) => {
180-
const next = firstPageParam + count;
186+
const pageSize = Math.min(firstPage.items.length, count);
187+
if (pageSize <= 0) {
188+
return undefined;
189+
}
190+
const next = firstPageParam + pageSize;
181191
return next < firstPage.itemCount ? next : undefined;
182192
},
183193
select: ({ pages }) => {

0 commit comments

Comments
 (0)