From 174fc4f9adb17297e9e44b2caeb98148502c8208 Mon Sep 17 00:00:00 2001 From: onozaty Date: Fri, 25 Jul 2025 21:20:40 +0900 Subject: [PATCH 1/7] fix: prevent auto-scroll to thread parent message on click Remove unintended navigation to parent message when clicking thread message preview. Always navigate to the clicked message instead of parent message. --- .../components/message/variants/ThreadMessagePreview.tsx | 4 ---- 1 file changed, 4 deletions(-) diff --git a/apps/meteor/client/components/message/variants/ThreadMessagePreview.tsx b/apps/meteor/client/components/message/variants/ThreadMessagePreview.tsx index af0aadefbf308..06e4e00800bc3 100644 --- a/apps/meteor/client/components/message/variants/ThreadMessagePreview.tsx +++ b/apps/meteor/client/components/message/variants/ThreadMessagePreview.tsx @@ -60,10 +60,6 @@ const ThreadMessagePreview = ({ message, showUserAvatar, sequential, ...props }: 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 }); } From d988be9e90883f15a80c31cf3d93c224d8b9204a Mon Sep 17 00:00:00 2001 From: onozaty Date: Sat, 26 Jul 2025 17:59:39 +0900 Subject: [PATCH 2/7] chore: add changeset for thread message preview fix --- .changeset/slimy-actors-report.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/slimy-actors-report.md diff --git a/.changeset/slimy-actors-report.md b/.changeset/slimy-actors-report.md new file mode 100644 index 0000000000000..17e33af84f393 --- /dev/null +++ b/.changeset/slimy-actors-report.md @@ -0,0 +1,5 @@ +--- +'@rocket.chat/meteor': patch +--- + +Fixed thread message preview click behavior to navigate to the clicked reply message instead of unintentionally jumping to the parent message From c94600df6939482a391126aa92e6e03502a04d0d Mon Sep 17 00:00:00 2001 From: MartinSchoeler Date: Mon, 28 Jul 2025 18:14:13 -0300 Subject: [PATCH 3/7] fix: separate thread preview and thread parent link components --- .../variants/ThreadMessageParentLink.tsx | 128 ++++++++++++++++++ .../message/variants/ThreadMessagePreview.tsx | 47 +------ .../room/MessageList/MessageListItem.tsx | 3 +- 3 files changed, 131 insertions(+), 47 deletions(-) create mode 100644 apps/meteor/client/components/message/variants/ThreadMessageParentLink.tsx diff --git a/apps/meteor/client/components/message/variants/ThreadMessageParentLink.tsx b/apps/meteor/client/components/message/variants/ThreadMessageParentLink.tsx new file mode 100644 index 0000000000000..a7c43164f7897 --- /dev/null +++ b/apps/meteor/client/components/message/variants/ThreadMessageParentLink.tsx @@ -0,0 +1,128 @@ +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 { MouseEvent, KeyboardEvent, 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'; + +type ThreadMessageParentLinkProps = { + message: IThreadMessage; +} & ComponentProps; + +const useLinkPattern = ({ onPress }: { onPress: (e: MouseEvent | KeyboardEvent) => void }) => { + const handleKeyDown = (event: KeyboardEvent) => { + if (event.key === 'Enter') { + event.preventDefault(); + onPress(event); + } + }; + + return { onClick: onPress, onKeyDown: handleKeyDown, role: 'link', tabIndex: 0 }; +}; + +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 linkProps = useLinkPattern({ + onPress: handleClick, + }); + + const threadMessageProps = { + 'aria-roledescription': isOTRMsg ? t('OTR_thread_message_preview') : t('thread_message_preview'), + isSelected, + 'data-qa-selected': isSelected, + ...linkProps, + ...props, + }; + + return ( + + + + + + + + {parentMessage.isSuccess && !messageType && ( + <> + {(parentMessage.data as { ignored?: boolean })?.ignored ? ( + t('Message_Ignored') + ) : ( + + )} + {translated && ( + <> + {' '} + + + )} + + )} + {messageType && t(messageType.message, messageType.data ? messageType.data(message) : {})} + {parentMessage.isLoading && } + + + + + + ); +}; + +export default memo(ThreadMessageParentLink); diff --git a/apps/meteor/client/components/message/variants/ThreadMessagePreview.tsx b/apps/meteor/client/components/message/variants/ThreadMessagePreview.tsx index 06e4e00800bc3..7ee2e272cf915 100644 --- a/apps/meteor/client/components/message/variants/ThreadMessagePreview.tsx +++ b/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,16 +13,12 @@ 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'; @@ -35,12 +27,9 @@ import ThreadMessagePreviewBody from './threadPreview/ThreadMessagePreviewBody'; type ThreadMessagePreviewProps = { message: IThreadMessage; showUserAvatar: boolean; - sequential: boolean; } & ComponentProps; -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,11 +40,6 @@ 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 = () => { @@ -81,35 +65,6 @@ const ThreadMessagePreview = ({ message, showUserAvatar, sequential, ...props }: data-qa-selected={isSelected} {...props} > - {!sequential && ( - - - - - - - {parentMessage.isSuccess && !messageType && ( - <> - {(parentMessage.data as { ignored?: boolean })?.ignored ? ( - t('Message_Ignored') - ) : ( - - )} - {translated && ( - <> - {' '} - - - )} - - )} - {messageType && t(messageType.message, messageType.data ? messageType.data(message) : {})} - {parentMessage.isLoading && } - - - - - )} {!isSelecting && showUserAvatar && ( diff --git a/apps/meteor/client/views/room/MessageList/MessageListItem.tsx b/apps/meteor/client/views/room/MessageList/MessageListItem.tsx index ea405f3af848c..2c1936b3518b0 100644 --- a/apps/meteor/client/views/room/MessageList/MessageListItem.tsx +++ b/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) && (
  • + {!shouldShowAsSequential && } From 14dbb6030c3e7df00dc2aaa48e72c40ab83f2482 Mon Sep 17 00:00:00 2001 From: Martin Schoeler Date: Mon, 28 Jul 2025 18:17:36 -0300 Subject: [PATCH 4/7] Update .changeset/slimy-actors-report.md --- .changeset/slimy-actors-report.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/slimy-actors-report.md b/.changeset/slimy-actors-report.md index 17e33af84f393..9a470b6649356 100644 --- a/.changeset/slimy-actors-report.md +++ b/.changeset/slimy-actors-report.md @@ -2,4 +2,4 @@ '@rocket.chat/meteor': patch --- -Fixed thread message preview click behavior to navigate to the clicked reply message instead of unintentionally jumping to the parent message +Fixes thread message preview click behavior, navigate to the clicked reply message instead of unintentionally jumping to the parent message From 0ba2ccf7242b3a8b76643e380a9337d33951b883 Mon Sep 17 00:00:00 2001 From: dougfabris Date: Mon, 28 Jul 2025 19:13:59 -0300 Subject: [PATCH 5/7] fix: review --- .../variants/ThreadMessageParentLink.tsx | 28 +++---------------- .../message/variants/ThreadMessagePreview.tsx | 14 +++------- .../threadPreview/useThreadMessageProps.ts | 28 +++++++++++++++++++ 3 files changed, 36 insertions(+), 34 deletions(-) create mode 100644 apps/meteor/client/components/message/variants/threadPreview/useThreadMessageProps.ts diff --git a/apps/meteor/client/components/message/variants/ThreadMessageParentLink.tsx b/apps/meteor/client/components/message/variants/ThreadMessageParentLink.tsx index a7c43164f7897..9b8537f832492 100644 --- a/apps/meteor/client/components/message/variants/ThreadMessageParentLink.tsx +++ b/apps/meteor/client/components/message/variants/ThreadMessageParentLink.tsx @@ -11,7 +11,7 @@ import { MessageStatusIndicatorItem, } from '@rocket.chat/fuselage'; import { useEffectEvent } from '@rocket.chat/fuselage-hooks'; -import type { MouseEvent, KeyboardEvent, ComponentProps, ReactElement } from 'react'; +import type { ComponentProps, ReactElement } from 'react'; import { memo } from 'react'; import { useTranslation } from 'react-i18next'; @@ -28,22 +28,12 @@ import { isParsedMessage } from '../../../views/room/MessageList/lib/isParsedMes 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; -const useLinkPattern = ({ onPress }: { onPress: (e: MouseEvent | KeyboardEvent) => void }) => { - const handleKeyDown = (event: KeyboardEvent) => { - if (event.key === 'Enter') { - event.preventDefault(); - onPress(event); - } - }; - - return { onClick: onPress, onKeyDown: handleKeyDown, role: 'link', tabIndex: 0 }; -}; - const ThreadMessageParentLink = ({ message, ...props }: ThreadMessageParentLinkProps): ReactElement => { const parentMessage = useParentMessage(message.tmid); @@ -80,20 +70,10 @@ const ThreadMessageParentLink = ({ message, ...props }: ThreadMessageParentLinkP return toggleSelected(); }); - const linkProps = useLinkPattern({ - onPress: handleClick, - }); - - const threadMessageProps = { - 'aria-roledescription': isOTRMsg ? t('OTR_thread_message_preview') : t('thread_message_preview'), - isSelected, - 'data-qa-selected': isSelected, - ...linkProps, - ...props, - }; + const threadMessageProps = useThreadMessageProps(handleClick, isOTRMsg, isSelected); return ( - + diff --git a/apps/meteor/client/components/message/variants/ThreadMessagePreview.tsx b/apps/meteor/client/components/message/variants/ThreadMessagePreview.tsx index 7ee2e272cf915..42f1aef14c1bb 100644 --- a/apps/meteor/client/components/message/variants/ThreadMessagePreview.tsx +++ b/apps/meteor/client/components/message/variants/ThreadMessagePreview.tsx @@ -23,6 +23,7 @@ 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; @@ -54,17 +55,10 @@ const ThreadMessagePreview = ({ message, showUserAvatar, ...props }: ThreadMessa return toggleSelected(); }; + const threadMessageProps = useThreadMessageProps(handleThreadClick, isOTRMsg, isSelected); + return ( - e.code === 'Enter' && handleThreadClick()} - isSelected={isSelected} - data-qa-selected={isSelected} - {...props} - > + {!isSelecting && showUserAvatar && ( diff --git a/apps/meteor/client/components/message/variants/threadPreview/useThreadMessageProps.ts b/apps/meteor/client/components/message/variants/threadPreview/useThreadMessageProps.ts new file mode 100644 index 0000000000000..1a3cff283b2c3 --- /dev/null +++ b/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 | KeyboardEvent) => 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, + }; +}; From 801f9dd2e9bfa4ec4019e1e720e07e7f3d916002 Mon Sep 17 00:00:00 2001 From: MartinSchoeler Date: Thu, 31 Jul 2025 14:58:02 -0300 Subject: [PATCH 6/7] test: add e2e tests --- apps/meteor/tests/e2e/threads.spec.ts | 33 ++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/apps/meteor/tests/e2e/threads.spec.ts b/apps/meteor/tests/e2e/threads.spec.ts index fbbfcb07e87eb..404c05f2afd02 100644 --- a/apps/meteor/tests/e2e/threads.spec.ts +++ b/apps/meteor/tests/e2e/threads.spec.ts @@ -1,10 +1,12 @@ +import { faker } from '@faker-js/faker'; + import { Users } from './fixtures/userStates'; import { HomeChannel } from './page-objects'; import { createTargetChannel, deleteChannel } from './utils'; import { expect, test } from './utils/test'; test.use({ storageState: Users.admin.state }); -test.describe.serial('Threads', () => { +test.describe.only('Threads', () => { let poHomeChannel: HomeChannel; let targetChannel: string; test.beforeAll(async ({ api }) => { @@ -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( From 7577eea4bc4da8a27fefa7f71431355da3ff79b9 Mon Sep 17 00:00:00 2001 From: MartinSchoeler Date: Thu, 31 Jul 2025 15:02:38 -0300 Subject: [PATCH 7/7] chore: :tasso: --- apps/meteor/tests/e2e/threads.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/meteor/tests/e2e/threads.spec.ts b/apps/meteor/tests/e2e/threads.spec.ts index 404c05f2afd02..bcb6a6d82224e 100644 --- a/apps/meteor/tests/e2e/threads.spec.ts +++ b/apps/meteor/tests/e2e/threads.spec.ts @@ -6,7 +6,7 @@ import { createTargetChannel, deleteChannel } from './utils'; import { expect, test } from './utils/test'; test.use({ storageState: Users.admin.state }); -test.describe.only('Threads', () => { +test.describe.serial('Threads', () => { let poHomeChannel: HomeChannel; let targetChannel: string; test.beforeAll(async ({ api }) => {