fix: GIF/rich attachment collapse state resetting on message list scroll

.changeset/yellow-rooms-tease.md

@@ -0,0 +1,5 @@
+---
+'@rocket.chat/meteor': patch
+---
+
+Fixes an issue where message attachments lose their collapsed state when scrolled out of view.

apps/meteor/client/components/message/content/Attachments.tsx

@@ -7,11 +7,17 @@ export type AttachmentsProps = {
 	attachments: MessageAttachmentBase[];
 	id?: string | undefined;
 	source?: AudioAttachmentSource;
+	keyPrefix?: string;
 };
 
-const Attachments = ({ attachments, id, source }: AttachmentsProps) => {
+const Attachments = ({ attachments, id, source, keyPrefix }: AttachmentsProps) => {
 	return (
-		<>{attachments?.map((attachment, index) => <AttachmentsItem key={index} id={id} attachment={{ ...attachment }} source={source} />)}</>
+		<>
+			{attachments?.map((attachment, index) => {
+				const path = keyPrefix ? `${keyPrefix}.${index}` : String(index);
+				return <AttachmentsItem key={index} id={id} path={path} attachment={{ ...attachment }} source={source} />;
+			})}
+		</>
 	);
 };
 

apps/meteor/client/components/message/content/attachments/AttachmentsItem.tsx

@@ -10,19 +10,22 @@ import type { AudioAttachmentSource } from './file/AudioAttachment';
 export type AttachmentsItemProps = {
 	attachment: MessageAttachmentBase;
 	id: string | undefined;
+	path: string;
 	source?: AudioAttachmentSource;
 };
 
-const AttachmentsItem = ({ attachment, id, source }: AttachmentsItemProps) => {
+const AttachmentsItem = ({ attachment, id, path, source }: AttachmentsItemProps) => {
 	if (isFileAttachment(attachment)) {
 		return <FileAttachment id={id} source={source} {...attachment} />;
 	}
 
 	if (isQuoteAttachment(attachment)) {
-		return <QuoteAttachment attachment={attachment} source={source} />;
+		return <QuoteAttachment attachment={attachment} source={source} path={path} />;
 	}
 
-	return <DefaultAttachment {...(attachment as any)} />;
+	const collapseKey = source?.mid ? `${source.mid}-${path}` : undefined;
+
+	return <DefaultAttachment {...attachment} collapseKey={collapseKey} />;
 };
 
 export default memo(AttachmentsItem);

apps/meteor/client/components/message/content/attachments/DefaultAttachment.spec.tsx

@@ -0,0 +1,86 @@
+import { mockAppRoot } from '@rocket.chat/mock-providers';
+import { render, screen } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
+
+import DefaultAttachment from './DefaultAttachment';
+import { collapseToggleStore } from '../../../../views/room/MessageList/contexts/CollapsedAttachmentsContext';
+
+beforeEach(() => {
+	collapseToggleStore.toggledAttachments.clear();
+});
+
+const giphyAttachment = (collapseKey: string | undefined) => ({
+	collapseKey,
+	title: 'GIPHY',
+	image_url: 'https://media.giphy.com/foo.gif',
+	image_alt: 'a fun gif',
+});
+
+it('should render expanded by default and collapse on toggle click', async () => {
+	render(<DefaultAttachment {...giphyAttachment('message-1-0')} />, { wrapper: mockAppRoot().build() });
+	expect(screen.getByRole('img')).toBeInTheDocument();
+
+	await userEvent.click(screen.getByTitle('Collapse'));
+	expect(screen.queryByRole('img')).not.toBeInTheDocument();
+});
+
+it('should keep a manually collapsed attachment collapsed after the row unmounts and remounts (simulating virtua recycling it on scroll)', async () => {
+	const attachment = giphyAttachment('message-2-0');
+	const { unmount } = render(<DefaultAttachment {...attachment} />, { wrapper: mockAppRoot().build() });
+
+	await userEvent.click(screen.getByTitle('Collapse'));
+	expect(screen.queryByRole('img')).not.toBeInTheDocument();
+
+	unmount();
+
+	render(<DefaultAttachment {...attachment} />, { wrapper: mockAppRoot().build() });
+	expect(screen.queryByRole('img')).not.toBeInTheDocument();
+});
+
+it('should keep a manually expanded attachment expanded after unmount/remount when collapsed by default', async () => {
+	const attachment = { ...giphyAttachment('message-3-0'), collapsed: true };
+
+	const { unmount } = render(<DefaultAttachment {...attachment} />, { wrapper: mockAppRoot().build() });
+	expect(screen.queryByRole('img')).not.toBeInTheDocument();
+
+	await userEvent.click(screen.getByTitle('Uncollapse'));
+	expect(screen.getByRole('img')).toBeInTheDocument();
+
+	unmount();
+
+	render(<DefaultAttachment {...attachment} />, { wrapper: mockAppRoot().build() });
+	expect(screen.getByRole('img')).toBeInTheDocument();
+});
+
+it('should not affect other messages when one is toggled', async () => {
+	const { unmount } = render(<DefaultAttachment {...giphyAttachment('message-4a-0')} />, { wrapper: mockAppRoot().build() });
+	await userEvent.click(screen.getByTitle('Collapse'));
+
+	unmount();
+
+	render(<DefaultAttachment {...giphyAttachment('message-4b-0')} />, { wrapper: mockAppRoot().build() });
+	expect(screen.getByRole('img')).toBeInTheDocument();
+});
+
+it('should toggle attachments within the same message independently', async () => {
+	const { unmount } = render(<DefaultAttachment {...giphyAttachment('message-5-0')} />, {
+		wrapper: mockAppRoot().build(),
+	});
+	await userEvent.click(screen.getByTitle('Collapse'));
+
+	unmount();
+
+	render(<DefaultAttachment {...giphyAttachment('message-5-1')} />, { wrapper: mockAppRoot().build() });
+	expect(screen.getByRole('img')).toBeInTheDocument();
+});
+
+it('should still toggle via local state when there is no collapseKey (e.g. the composer quote preview)', async () => {
+	render(<DefaultAttachment {...giphyAttachment(undefined)} />, { wrapper: mockAppRoot().build() });
+	expect(screen.getByRole('img')).toBeInTheDocument();
+
+	await userEvent.click(screen.getByTitle('Collapse'));
+	expect(screen.queryByRole('img')).not.toBeInTheDocument();
+
+	await userEvent.click(screen.getByTitle('Uncollapse'));
+	expect(screen.getByRole('img')).toBeInTheDocument();
+});

apps/meteor/client/components/message/content/attachments/DefaultAttachment.tsx

@@ -1,5 +1,6 @@
 import type { MarkdownFields, MessageAttachmentDefault } from '@rocket.chat/core-typings';
 import { isActionAttachment } from '@rocket.chat/core-typings';
+import { useAttachmentIsCollapsedByDefault } from '@rocket.chat/ui-contexts';
 import type { ReactNode, ComponentProps } from 'react';
 
 import { ActionAttachment } from './default/ActionAttachtment';
@@ -14,6 +15,10 @@ import AttachmentRow from './structure/AttachmentRow';
 import AttachmentText from './structure/AttachmentText';
 import AttachmentThumb from './structure/AttachmentThumb';
 import AttachmentTitle from './structure/AttachmentTitle';
+import {
+	useIsAttachmentCollapseToggled,
+	useToggleAttachmentCollapse,
+} from '../../../../views/room/MessageList/contexts/CollapsedAttachmentsContext';
 import MarkdownText from '../../../MarkdownText';
 import { useCollapse } from '../../hooks/useCollapse';
 import CollapsibleContent from '../collapsible/CollapsibleContent';
@@ -25,10 +30,20 @@ const applyMarkdownIfRequires = (
 	variant: ComponentProps<typeof MarkdownText>['variant'] = 'inline',
 ): ReactNode => (list?.includes(key) ? <MarkdownText parseEmoji variant={variant} content={text} /> : text);
 
-export type DefaultAttachmentProps = MessageAttachmentDefault;
+export type DefaultAttachmentProps = MessageAttachmentDefault & {
+	collapseKey?: string;
+};
+
+const DefaultAttachment = ({ collapseKey, ...attachment }: DefaultAttachmentProps) => {
+	const defaultCollapsed = useAttachmentIsCollapsedByDefault() || !!attachment.collapsed;
+	const toggled = useIsAttachmentCollapseToggled(collapseKey);
+	const togglePersistedCollapse = useToggleAttachmentCollapse(collapseKey);
+	// Without a collapseKey (e.g. the composer's quoted-message preview, which has no real
+	// message id to key off of) fall back to plain local state
+	const [localCollapsed, toggleLocalCollapse] = useCollapse(attachment.collapsed);
 
-const DefaultAttachment = (attachment: DefaultAttachmentProps) => {
-	const [collapsed, toggleCollapse] = useCollapse(!!attachment.collapsed);
+	const collapsed = collapseKey ? toggled !== defaultCollapsed : localCollapsed;
+	const toggleCollapse = collapseKey ? togglePersistedCollapse : toggleLocalCollapse;
 
 	return (
 		<AttachmentBlock

apps/meteor/client/components/message/content/attachments/QuoteAttachment.tsx

@@ -36,9 +36,11 @@ const quoteStyles = css`
 export type QuoteAttachmentProps = {
 	attachment: MessageQuoteAttachment;
 	source?: AudioAttachmentSource;
+	/** This quote's own path, used to prefix its nested attachments' collapse-state keys. */
+	path?: string;
 };
 
-export const QuoteAttachment = ({ attachment, source }: QuoteAttachmentProps) => {
+export const QuoteAttachment = ({ attachment, source, path }: QuoteAttachmentProps) => {
 	const formatTime = useTimeAgo();
 	const displayAvatarPreference = useUserPreference<boolean>('displayAvatars');
 
@@ -71,6 +73,7 @@ export const QuoteAttachment = ({ attachment, source }: QuoteAttachmentProps) =>
 								attachments={attachment.attachments}
 								id={attachment.attachments[0]?.title_link}
 								source={source && { rid: source.rid, mid: source.mid, name: attachment.author_name }}
+								keyPrefix={path}
 							/>
 						</AttachmentInner>
 					)}

apps/meteor/client/lib/constants.ts

@@ -3,3 +3,4 @@ export const BIO_TEXT_MAX_LENGTH = 260;
 export const VIDEOCONF_STACK_MAX_USERS = 6;
 export const NAVIGATION_REGION_ID = 'navigation-region';
 export const MAX_FILE_SIZE_PREVIEW = 10485760; // 10MB
+export const MAX_TOGGLED_ATTACHMENTS = 1_000;

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

@@ -24,6 +24,7 @@ import RoomForeword from '../body/RoomForeword/RoomForeword';
 import { useStoreScrollPosition } from '../body/hooks/useStoreScrollPosition';
 import { useChat } from '../contexts/ChatContext';
 import type { RetentionPolicy } from '../hooks/useRetentionPolicy';
+import { CollapsedAttachmentsProvider } from '../providers/CollapsedAttachmentsProvider';
 import { useKeepMountedMessages } from './hooks/useKeepMountedMessages';
 
 export type MessageListProps = {
@@ -255,69 +256,71 @@ export const MessageList = function MessageList({
 	return (
 		<MessageListProvider>
 			<SelectedMessagesProvider>
-				<VList
-					ref={virtualizerRef}
-					shift={isPrepend.current === true}
-					style={{ height: '100%' }}
-					aria-label={t('Message_list')}
-					aria-busy={isLoadingMoreMessages}
-					role='list'
-					className='messages-list'
-					keepMounted={keepMountedMessages}
-					onScroll={(offset: number) => {
-						handlePrepend(offset);
-						storeScrollPosition();
-						debouncedClearNewMessagesOnScroll();
-
-						const handle = virtualizerRef.current;
-						const viewportTopPadding = 21; // TODO: we should derive this value from somewhere else.
-						const topMessage = handle
-							? messages[handle.findItemIndex(handle.scrollOffset - viewportTopPadding) - (canPreview ? 1 : 0)]
-							: undefined;
-						handleTopVisibleMessage(topMessage);
-						handleDateScroll(topMessage, offset);
-						debouncedMessageRead();
-					}}
-				>
-					{canPreview ? (
-						<>
-							{hasMorePreviousMessages ? (
-								<li className='load-more'>{isLoadingMoreMessages ? <LoadingMessagesIndicator /> : null}</li>
-							) : (
-								<li>
-									<RoomForeword user={user} room={room} />
-									{retentionPolicy?.isActive ? <RetentionPolicyWarning room={room} /> : null}
-								</li>
-							)}
-						</>
-					) : null}
-					{messages.map((message, index, { [index - 1]: previous }) => {
-						const sequential = isMessageSequential(message, previous, messageGroupingPeriod);
-						const showUnreadDivider = firstUnreadMessageId === message._id;
-						const system = MessageTypes.isSystemMessage(message);
-						const visible = !isThreadMessage(message) && !system;
-
-						if (showUnreadDivider) {
-							unreadMarkIndex.current = index;
-						}
-
-						return (
-							<Fragment key={message._id}>
-								<MessageListItem
-									message={message}
-									previous={previous}
-									showUnreadDivider={showUnreadDivider}
-									showUserAvatar={showUserAvatar}
-									sequential={sequential}
-									visible={visible}
-									subscription={subscription}
-									system={system}
-								/>
-							</Fragment>
-						);
-					})}
-					{hasMoreNextMessages ? <li className='load-more'>{isLoadingMoreMessages ? <LoadingMessagesIndicator /> : null}</li> : null}
-				</VList>
+				<CollapsedAttachmentsProvider>
+					<VList
+						ref={virtualizerRef}
+						shift={isPrepend.current === true}
+						style={{ height: '100%' }}
+						aria-label={t('Message_list')}
+						aria-busy={isLoadingMoreMessages}
+						role='list'
+						className='messages-list'
+						keepMounted={keepMountedMessages}
+						onScroll={(offset: number) => {
+							handlePrepend(offset);
+							storeScrollPosition();
+							debouncedClearNewMessagesOnScroll();
+
+							const handle = virtualizerRef.current;
+							const viewportTopPadding = 21; // TODO: we should derive this value from somewhere else.
+							const topMessage = handle
+								? messages[handle.findItemIndex(handle.scrollOffset - viewportTopPadding) - (canPreview ? 1 : 0)]
+								: undefined;
+							handleTopVisibleMessage(topMessage);
+							handleDateScroll(topMessage, offset);
+							debouncedMessageRead();
+						}}
+					>
+						{canPreview ? (
+							<>
+								{hasMorePreviousMessages ? (
+									<li className='load-more'>{isLoadingMoreMessages ? <LoadingMessagesIndicator /> : null}</li>
+								) : (
+									<li>
+										<RoomForeword user={user} room={room} />
+										{retentionPolicy?.isActive ? <RetentionPolicyWarning room={room} /> : null}
+									</li>
+								)}
+							</>
+						) : null}
+						{messages.map((message, index, { [index - 1]: previous }) => {
+							const sequential = isMessageSequential(message, previous, messageGroupingPeriod);
+							const showUnreadDivider = firstUnreadMessageId === message._id;
+							const system = MessageTypes.isSystemMessage(message);
+							const visible = !isThreadMessage(message) && !system;
+
+							if (showUnreadDivider) {
+								unreadMarkIndex.current = index;
+							}
+
+							return (
+								<Fragment key={message._id}>
+									<MessageListItem
+										message={message}
+										previous={previous}
+										showUnreadDivider={showUnreadDivider}
+										showUserAvatar={showUserAvatar}
+										sequential={sequential}
+										visible={visible}
+										subscription={subscription}
+										system={system}
+									/>
+								</Fragment>
+							);
+						})}
+						{hasMoreNextMessages ? <li className='load-more'>{isLoadingMoreMessages ? <LoadingMessagesIndicator /> : null}</li> : null}
+					</VList>
+				</CollapsedAttachmentsProvider>
 			</SelectedMessagesProvider>
 		</MessageListProvider>
 	);