Skip to content

fix: persist attachment collapsed state across channel switches #40365

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

Open
SYED-MHAMIL wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

fix: persist attachment collapsed state across channel switches #40365

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
09e1265
add-persistant-attachment
SYED-MHAMIL May 2, 2026
f664fa5
Merge branch 'develop' into fix/persistant-collapse-attachment
SYED-MHAMIL May 3, 2026
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
5 changes: 3 additions & 2 deletions apps/meteor/client/components/message/MessageCollapsible.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,11 @@ type MessageCollapsibleProps = {
link?: string;
size?: number;
isCollapsed?: boolean;
attachmentId? : string ; //
};

const MessageCollapsible = ({ children, title, hasDownload, link, size, isCollapsed }: MessageCollapsibleProps): ReactElement => {
const [collapsed, collapse] = useCollapse(isCollapsed);
const MessageCollapsible = ({ children, title, hasDownload, link, size, isCollapsed, attachmentId }: MessageCollapsibleProps): ReactElement => {
const [collapsed, collapse] = useCollapse(attachmentId,isCollapsed); //

return (
<>
Expand Down
3 changes: 2 additions & 1 deletion apps/meteor/client/components/message/content/attachments/file/AudioAttachment.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { useReloadOnError } from './hooks/useReloadOnError';
import MessageCollapsible from '../../../MessageCollapsible';

const AudioAttachment = ({
id,
title,
audio_url: url,
audio_type: type,
Expand All @@ -21,7 +22,7 @@ const AudioAttachment = ({

return (
<>
<MessageCollapsible title={title} hasDownload={hasDownload} link={getURL(link || url)} size={size} isCollapsed={collapsed}>
<MessageCollapsible title={title} hasDownload={hasDownload} link={getURL(link || url)} size={size} isCollapsed={collapsed} attachmentId={id}>
<AudioPlayer src={src} type={type} ref={mediaRef} />
</MessageCollapsible>
</>
Expand Down
3 changes: 2 additions & 1 deletion apps/meteor/client/components/message/content/attachments/file/GenericFileAttachment.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ const openDocumentViewer = window.RocketChatDesktop?.openDocumentViewer;
type GenericFileAttachmentProps = MessageAttachmentBase;

const GenericFileAttachment = ({
id,
title,
title_link: link,
title_link_download: hasDownload,
Expand Down Expand Up @@ -68,7 +69,7 @@ const GenericFileAttachment = ({

return (
<>
<MessageCollapsible title={title} hasDownload={hasDownload} link={link} isCollapsed={collapsed}>
<MessageCollapsible title={title} hasDownload={hasDownload} link={link} isCollapsed={collapsed} attachmentId={id}>
<MessageGenericPreview style={{ maxWidth: 368, width: '100%' }}>
<MessageGenericPreviewContent
thumb={<MessageGenericPreviewIcon name='attachment-file' type={format || getFileExtension(title)} />}
Expand Down
2 changes: 1 addition & 1 deletion apps/meteor/client/components/message/content/attachments/file/ImageAttachment.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const ImageAttachment = ({

return (
<>
<MessageCollapsible title={title} hasDownload={hasDownload} link={getURL(link || url)} size={size} isCollapsed={collapsed}>
<MessageCollapsible title={title} hasDownload={hasDownload} link={getURL(link || url)} size={size} isCollapsed={collapsed} attachmentId={id}>
<AttachmentImage
{...imageDimensions}
loadImage={loadImage}
Expand Down
3 changes: 2 additions & 1 deletion apps/meteor/client/components/message/content/attachments/file/VideoAttachment.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { userAgentMIMETypeFallback } from '../../../../../lib/utils/userAgentMIM
import MessageCollapsible from '../../../MessageCollapsible';

const VideoAttachment = ({
id,
title,
video_url: url,
video_type: type,
Expand All @@ -22,7 +23,7 @@ const VideoAttachment = ({

return (
<>
<MessageCollapsible title={title} hasDownload={hasDownload} link={getURL(link || url)} size={size} isCollapsed={collapsed}>
<MessageCollapsible title={title} hasDownload={hasDownload} link={getURL(link || url)} size={size} isCollapsed={collapsed} attachmentId={id}>
<MessageGenericPreview style={{ maxWidth: 368, width: '100%' }}>
<Box is='video' controls preload='metadata' ref={mediaRef}>
<source src={getURL(url)} type={userAgentMIMETypeFallback(type)} />
Expand Down
2 changes: 1 addition & 1 deletion apps/meteor/client/components/message/content/urlPreviews/OEmbedCollapsible.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const OEmbedCollapsible = ({ children, ...props }: OEmbedCollapsibleProps): Reac
const { t } = useTranslation();

return (
<MessageCollapsible title={t('Link_Preview')}>
<MessageCollapsible title={t('Link_Preview')} >
<MessageGenericPreview>
{children}
<OEmbedPreviewContent {...props} />
Expand Down
52 changes: 49 additions & 3 deletions apps/meteor/client/components/message/hooks/useCollapse.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,57 @@
import { useToggle } from '@rocket.chat/fuselage-hooks';
import { useAttachmentIsCollapsedByDefault } from '@rocket.chat/ui-contexts';
import type { ReactNode } from 'react';
import {useState,useEffect} from 'react'

import CollapsibleContent from '../content/collapsible/CollapsibleContent';

export const useCollapse = (attachmentCollapsed?: boolean): [collapsed: boolean, node: ReactNode] => {
const collpaseByDefault = useAttachmentIsCollapsedByDefault();
const [collapsed, toogleCollapsed] = useToggle(collpaseByDefault || attachmentCollapsed);
const usePersistedCollapse= (attachmentId : string | undefined , initialCollapsed: boolean)=>{
const [collapsed,setCollapsed] = useState(()=>{
if(attachmentId){
const stored= sessionStorage.getItem(`persistant-attachment-${attachmentId}`)
if(stored != null){
return stored == 'true'

}
}

return initialCollapsed

})

const toogleCollapsed= ()=>{
setCollapsed((prev)=>{
const value = !prev
if(attachmentId){
sessionStorage.setItem(`persistant-attachment-${attachmentId}`,value ? 'true' : 'false')
}
return value
})

}

// Sync with state

useEffect(() => {
if (attachmentId) {
const stored = sessionStorage.getItem(`persistant-attachment-${attachmentId}`);
if (stored !== null) {
setCollapsed(stored === 'true');
}
}
}, [attachmentId]);
Comment on lines +35 to +42
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Reset to the new attachment's default when storage is empty.

When attachmentId changes, this effect only updates state if sessionStorage already has a value. If the new attachment has no saved entry yet, the hook keeps the previous attachment’s collapsed state instead of reinitializing from initialCollapsed.

🔧 Proposed fix 
 useEffect(() => {
 	if (attachmentId) {
 		const stored = sessionStorage.getItem(`persistant-attachment-${attachmentId}`);
-		if (stored !== null) {
-			setCollapsed(stored === 'true');
-		}
+		setCollapsed(stored === null ? initialCollapsed : stored === 'true');
 	}
-}, [attachmentId]);
+}, [attachmentId, initialCollapsed]);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
useEffect(() => {
if (attachmentId) {
const stored = sessionStorage.getItem(`persistant-attachment-${attachmentId}`);
if (stored !== null) {
setCollapsed(stored === 'true');
}
}
}, [attachmentId]);
useEffect(() => {
if (attachmentId) {
const stored = sessionStorage.getItem(`persistant-attachment-${attachmentId}`);
setCollapsed(stored === null ? initialCollapsed : stored === 'true');
}
}, [attachmentId, initialCollapsed]);
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@apps/meteor/client/components/message/hooks/useCollapse.tsx` around lines 35
- 42, When attachmentId changes the useEffect currently only calls setCollapsed
if sessionStorage has a value, so the previous attachment's state can leak;
update the effect in useEffect that reads
`persistant-attachment-${attachmentId}` to call setCollapsed(initialCollapsed)
when stored is null (i.e., no saved entry) so the new attachment resets to the
default; ensure you reference `attachmentId`, `setCollapsed`, and
`initialCollapsed` inside that effect (and add `initialCollapsed` to the
dependency array if it isn’t already).


return [collapsed,toogleCollapsed] as const

}



export const useCollapse = (attachmentId : string | undefined,attachmentCollapsed?: boolean): [collapsed: boolean, node: ReactNode] => {

console.log('reset state')
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Remove the render-time debug log.

console.log('reset state') runs on every render and will leak noise into production consoles.

🧹 Proposed fix 
-	console.log('reset state')
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
console.log('reset state')
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@apps/meteor/client/components/message/hooks/useCollapse.tsx` at line 52, The
debug console.log in the useCollapse hook is printing "reset state" on every
render; remove that console.log('reset state') from the hook (or, if you want to
keep it for local debugging only, guard it behind a development-only check such
as NODE_ENV or __DEV__) so the hook (useCollapse) no longer emits render-time
logs in production.

const collapseByDefault = useAttachmentIsCollapsedByDefault()
const initialCollapsed = collapseByDefault || attachmentCollapsed
const [collapsed, toogleCollapsed] = usePersistedCollapse(attachmentId,initialCollapsed) ||useToggle(initialCollapsed);
return [collapsed, <CollapsibleContent collapsed={collapsed} onClick={toogleCollapsed as any} key='collapsible-content-action' />];
};