fix: Prevent auto-scroll to thread parent message on click

.changeset/slimy-actors-report.md

@@ -0,0 +1,5 @@
+---
+'@rocket.chat/meteor': patch
+---
+
+Fixes thread message preview click behavior, navigate to the clicked reply message instead of unintentionally jumping to the parent message

apps/meteor/client/components/message/variants/ThreadMessageParentLink.tsx

@@ -0,0 +1,108 @@
+import { isOTRAckMessage, isOTRMessage, type IThreadMessage } from '@rocket.chat/core-typings';
+import {
+	Skeleton,
+	ThreadMessage,
+	ThreadMessageRow,
+	ThreadMessageLeftContainer,
+	ThreadMessageIconThread,
+	ThreadMessageContainer,
+	ThreadMessageOrigin,
+	ThreadMessageUnfollow,
+	MessageStatusIndicatorItem,
+} from '@rocket.chat/fuselage';
+import { useEffectEvent } from '@rocket.chat/fuselage-hooks';
+import type { ComponentProps, ReactElement } from 'react';
+import { memo } from 'react';
+import { useTranslation } from 'react-i18next';
+
+import { MessageTypes } from '../../../../app/ui-utils/client';
+import {
+	useIsSelecting,
+	useToggleSelect,
+	useIsSelectedMessage,
+	useCountSelected,
+} from '../../../views/room/MessageList/contexts/SelectedMessagesContext';
+import { useMessageBody } from '../../../views/room/MessageList/hooks/useMessageBody';
+import { useParentMessage } from '../../../views/room/MessageList/hooks/useParentMessage';
+import { isParsedMessage } from '../../../views/room/MessageList/lib/isParsedMessage';
+import { useGoToThread } from '../../../views/room/hooks/useGoToThread';
+import { useShowTranslated } from '../list/MessageListContext';
+import ThreadMessagePreviewBody from './threadPreview/ThreadMessagePreviewBody';
+import { useThreadMessageProps } from './threadPreview/useThreadMessageProps';
+
+type ThreadMessageParentLinkProps = {
+	message: IThreadMessage;
+} & ComponentProps<typeof ThreadMessage>;
+
+const ThreadMessageParentLink = ({ message, ...props }: ThreadMessageParentLinkProps): ReactElement => {
+	const parentMessage = useParentMessage(message.tmid);
+
+	const translated = useShowTranslated(message);
+	const { t } = useTranslation();
+
+	const isSelecting = useIsSelecting();
+	const isOTRMsg = isOTRMessage(message) || isOTRAckMessage(message);
+
+	const toggleSelected = useToggleSelect(message._id);
+	const isSelected = useIsSelectedMessage(message._id, isOTRMsg);
+	useCountSelected();
+
+	const messageType = parentMessage.isSuccess ? MessageTypes.getType(parentMessage.data) : null;
+	const messageBody = useMessageBody(parentMessage.data);
+
+	const previewMessage = isParsedMessage(messageBody) ? { md: messageBody } : { msg: messageBody };
+
+	const goToThread = useGoToThread();
+
+	const handleClick = useEffectEvent(() => {
+		if (!isSelecting && parentMessage.isSuccess) {
+			return goToThread({
+				rid: message.rid,
+				tmid: message.tmid,
+				msg: parentMessage.data?._id,
+			});
+		}
+
+		if (isOTRMsg) {
+			return toggleSelected();
+		}
+
+		return toggleSelected();
+	});
+
+	const threadMessageProps = useThreadMessageProps(handleClick, isOTRMsg, isSelected);
+
+	return (
+		<ThreadMessage {...threadMessageProps} {...props}>
+			<ThreadMessageRow>
+				<ThreadMessageLeftContainer>
+					<ThreadMessageIconThread />
+				</ThreadMessageLeftContainer>
+				<ThreadMessageContainer>
+					<ThreadMessageOrigin>
+						{parentMessage.isSuccess && !messageType && (
+							<>
+								{(parentMessage.data as { ignored?: boolean })?.ignored ? (
+									t('Message_Ignored')
+								) : (
+									<ThreadMessagePreviewBody message={{ ...parentMessage.data, ...previewMessage }} />
+								)}
+								{translated && (
+									<>
+										{' '}
+										<MessageStatusIndicatorItem name='language' color='info' title={t('Translated')} />
+									</>
+								)}
+							</>
+						)}
+						{messageType && t(messageType.message, messageType.data ? messageType.data(message) : {})}
+						{parentMessage.isLoading && <Skeleton />}
+					</ThreadMessageOrigin>
+					<ThreadMessageUnfollow />
+				</ThreadMessageContainer>
+			</ThreadMessageRow>
+		</ThreadMessage>
+	);
+};
+
+export default memo(ThreadMessageParentLink);

apps/meteor/client/components/message/variants/ThreadMessagePreview.tsx

@@ -1,14 +1,10 @@
 import { isOTRAckMessage, isOTRMessage, type IThreadMessage } from '@rocket.chat/core-typings';
 import {
-	Skeleton,
 	ThreadMessage,
 	ThreadMessageRow,
 	ThreadMessageLeftContainer,
-	ThreadMessageIconThread,
 	ThreadMessageContainer,
-	ThreadMessageOrigin,
 	ThreadMessageBody,
-	ThreadMessageUnfollow,
 	CheckBox,
 	MessageStatusIndicatorItem,
 } from '@rocket.chat/fuselage';
@@ -17,30 +13,24 @@ import type { ComponentProps, ReactElement } from 'react';
 import { memo } from 'react';
 import { useTranslation } from 'react-i18next';
 
-import { MessageTypes } from '../../../../app/ui-utils/client';
 import {
 	useIsSelecting,
 	useToggleSelect,
 	useIsSelectedMessage,
 	useCountSelected,
 } from '../../../views/room/MessageList/contexts/SelectedMessagesContext';
-import { useMessageBody } from '../../../views/room/MessageList/hooks/useMessageBody';
-import { useParentMessage } from '../../../views/room/MessageList/hooks/useParentMessage';
-import { isParsedMessage } from '../../../views/room/MessageList/lib/isParsedMessage';
 import { useGoToThread } from '../../../views/room/hooks/useGoToThread';
 import Emoji from '../../Emoji';
 import { useShowTranslated } from '../list/MessageListContext';
 import ThreadMessagePreviewBody from './threadPreview/ThreadMessagePreviewBody';
+import { useThreadMessageProps } from './threadPreview/useThreadMessageProps';
 
 type ThreadMessagePreviewProps = {
 	message: IThreadMessage;
 	showUserAvatar: boolean;
-	sequential: boolean;
 } & ComponentProps<typeof ThreadMessage>;
 
-const ThreadMessagePreview = ({ message, showUserAvatar, sequential, ...props }: ThreadMessagePreviewProps): ReactElement => {
-	const parentMessage = useParentMessage(message.tmid);
-
+const ThreadMessagePreview = ({ message, showUserAvatar, ...props }: ThreadMessagePreviewProps): ReactElement => {
 	const translated = useShowTranslated(message);
 	const { t } = useTranslation();
 
@@ -51,19 +41,10 @@ const ThreadMessagePreview = ({ message, showUserAvatar, sequential, ...props }:
 	const isSelected = useIsSelectedMessage(message._id, isOTRMsg);
 	useCountSelected();
 
-	const messageType = parentMessage.isSuccess ? MessageTypes.getType(parentMessage.data) : null;
-	const messageBody = useMessageBody(parentMessage.data);
-
-	const previewMessage = isParsedMessage(messageBody) ? { md: messageBody } : { msg: messageBody };
-
 	const goToThread = useGoToThread();
 
 	const handleThreadClick = () => {
 		if (!isSelecting) {
-			if (!sequential) {
-				return parentMessage.isSuccess && goToThread({ rid: message.rid, tmid: message.tmid, msg: parentMessage.data?._id });
-			}
-
 			return goToThread({ rid: message.rid, tmid: message.tmid, msg: message._id });
 		}
 
@@ -74,46 +55,10 @@ const ThreadMessagePreview = ({ message, showUserAvatar, sequential, ...props }:
 		return toggleSelected();
 	};
 
+	const threadMessageProps = useThreadMessageProps(handleThreadClick, isOTRMsg, isSelected);
+
 	return (
-		<ThreadMessage
-			role='link'
-			aria-roledescription={isOTRMsg ? t('OTR_thread_message_preview') : t('thread_message_preview')}
-			tabIndex={0}
-			onClick={handleThreadClick}
-			onKeyDown={(e) => e.code === 'Enter' && handleThreadClick()}
-			isSelected={isSelected}
-			data-qa-selected={isSelected}
-			{...props}
-		>
-			{!sequential && (
-				<ThreadMessageRow>
-					<ThreadMessageLeftContainer>
-						<ThreadMessageIconThread />
-					</ThreadMessageLeftContainer>
-					<ThreadMessageContainer>
-						<ThreadMessageOrigin system={!!messageType}>
-							{parentMessage.isSuccess && !messageType && (
-								<>
-									{(parentMessage.data as { ignored?: boolean })?.ignored ? (
-										t('Message_Ignored')
-									) : (
-										<ThreadMessagePreviewBody message={{ ...parentMessage.data, ...previewMessage }} />
-									)}
-									{translated && (
-										<>
-											{' '}
-											<MessageStatusIndicatorItem name='language' color='info' title={t('Translated')} />
-										</>
-									)}
-								</>
-							)}
-							{messageType && t(messageType.message, messageType.data ? messageType.data(message) : {})}
-							{parentMessage.isLoading && <Skeleton />}
-						</ThreadMessageOrigin>
-						<ThreadMessageUnfollow />
-					</ThreadMessageContainer>
-				</ThreadMessageRow>
-			)}
+		<ThreadMessage {...threadMessageProps} {...props}>
 			<ThreadMessageRow>
 				<ThreadMessageLeftContainer>
 					{!isSelecting && showUserAvatar && (

apps/meteor/client/components/message/variants/threadPreview/useThreadMessageProps.ts

@@ -0,0 +1,28 @@
+import type { MouseEvent, KeyboardEvent } from 'react';
+import { useTranslation } from 'react-i18next';
+
+// TODO: Move this hook to fuselage-hooks
+const useLinkPattern = ({ onPress }: { onPress: (e: MouseEvent<Element> | KeyboardEvent<Element>) => void }) => {
+	const handleKeyDown = (event: KeyboardEvent) => {
+		if (event.key === 'Enter') {
+			event.preventDefault();
+			onPress(event);
+		}
+	};
+
+	return { onClick: onPress, onKeyDown: handleKeyDown, role: 'link', tabIndex: 0 };
+};
+
+export const useThreadMessageProps = (onClick: () => void, isOTRMsg: boolean, isSelected: boolean) => {
+	const { t } = useTranslation();
+	const linkProps = useLinkPattern({
+		onPress: onClick,
+	});
+
+	return {
+		'aria-roledescription': isOTRMsg ? t('OTR_thread_message_preview') : t('thread_message_preview'),
+		isSelected,
+		'data-qa-selected': isSelected,
+		...linkProps,
+	};
+};

apps/meteor/client/views/room/MessageList/MessageListItem.tsx

@@ -8,6 +8,7 @@ import ThreadMessagePreview from '../../../components/message/variants/ThreadMes
 import { useDateRef } from '../providers/DateListProvider';
 import { isMessageNewDay } from './lib/isMessageNewDay';
 import { useMessageListFormatDate } from '../../../components/message/list/MessageListContext';
+import ThreadMessageParentLink from '../../../components/message/variants/ThreadMessageParentLink';
 
 type MessageListItemProps = {
 	message: IMessage;
@@ -79,12 +80,12 @@ export const MessageListItem = ({
 			)}
 			{isThreadMessage(message) && (
 				<li>
+					{!shouldShowAsSequential && <ThreadMessageParentLink message={message} />}
 					<ThreadMessagePreview
 						data-mid={message._id}
 						data-tmid={message.tmid}
 						data-unread={showUnreadDivider}
 						data-sequential={sequential}
-						sequential={shouldShowAsSequential}
 						message={message}
 						showUserAvatar={showUserAvatar}
 					/>

apps/meteor/tests/e2e/threads.spec.ts

@@ -1,3 +1,5 @@
+import { faker } from '@faker-js/faker';
+
 import { Users } from './fixtures/userStates';
 import { HomeChannel } from './page-objects';
 import { createTargetChannel, deleteChannel } from './utils';
@@ -45,6 +47,35 @@ test.describe.serial('Threads', () => {
 		await expect(page).toHaveURL(/.*thread/);
 		await expect(poHomeChannel.content.lastThreadMessageText).toContainText('This is a thread message also sent in channel');
 	});
+
+	test('expect to highlight the correct message in the thread contextual bar', async ({ page }) => {
+		await poHomeChannel.content.lastThreadMessagePreviewText.click();
+		await expect(page).toHaveURL(/.*thread/);
+
+		await expect(poHomeChannel.content.lastThreadMessageText).toHaveAttribute('data-qa-editing', 'true');
+	});
+
+	test('expect highlight the correct message in the thread contextual bar after a non sequential message', async ({ page }) => {
+		const threadMessage = `thread_${faker.string.uuid()}`;
+		await poHomeChannel.content.sendMessage(threadMessage);
+		await poHomeChannel.content.sendMessage('this message should break the thread sequence');
+		await poHomeChannel.content.getMessageByText(threadMessage).hover();
+		await page.locator('role=button[name="Reply in thread"]').click();
+
+		await expect(page).toHaveURL(/.*thread/);
+
+		await poHomeChannel.content.toggleAlsoSendThreadToChannel(true);
+		await page.getByRole('dialog').locator('[name="msg"]').last().fill('This is a thread message also sent in channel');
+		await page.keyboard.press('Enter');
+		await expect(poHomeChannel.content.lastThreadMessageText).toContainText('This is a thread message also sent in channel');
+		await expect(poHomeChannel.content.lastUserMessage).toContainText('This is a thread message also sent in channel');
+
+		await poHomeChannel.content.lastThreadMessagePreviewText.click();
+		await expect(page).toHaveURL(/.*thread/);
+
+		await expect(poHomeChannel.content.lastThreadMessageText).toHaveAttribute('data-qa-editing', 'true');
+	});
+
 	test.describe('hideFlexTab Preference enabled for threads', () => {
 		test.beforeAll(async ({ api }) => {
 			await expect(