-
Notifications
You must be signed in to change notification settings - Fork 13.8k
fix: GIF/rich attachment collapse state resetting on message list scroll #41783
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.
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
nazabucciarelli
wants to merge
9
commits into
develop
Choose a base branch
from
fix/giphy-collapse-state-scroll
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 2 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
b0b671a
fix: GIF/rich attachment collapse state resetting on message list scroll
nazabucciarelli b7d00c7
add recursive logic to keep mounted nested attachments
nazabucciarelli 52e28d2
change of approach, use a context to store collapsed state of message…
nazabucciarelli 24e42da
delete previous changeset
nazabucciarelli 72dfa1b
make each message attachment able to be toggled independently
nazabucciarelli ff31382
add cache bound
nazabucciarelli c0c0781
renaming from "messages" to "attachments"
nazabucciarelli 6a0082e
remove unneeded comments
nazabucciarelli 9d8f02d
reset store between tests
nazabucciarelli File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@rocket.chat/meteor': patch | ||
| --- | ||
|
|
||
| Fixes collapsed/expanded GIFs (e.g. sent via `/giphy`) and other rich attachments from apps and integrations toggling back to the default state when scrolling the message list. |
114 changes: 114 additions & 0 deletions
114
apps/meteor/client/views/room/MessageList/hooks/useKeepMountedMessages.spec.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,114 @@ | ||
| import type { IMessage, MessageAttachment } from '@rocket.chat/core-typings'; | ||
|
|
||
| import { useKeepMountedMessages } from './useKeepMountedMessages'; | ||
|
|
||
| const date = new Date('2021-10-27T00:00:00.000Z'); | ||
| const baseMessage: IMessage = { | ||
| ts: date, | ||
| u: { | ||
| _id: 'userId', | ||
| name: 'userName', | ||
| username: 'userName', | ||
| }, | ||
| msg: 'message', | ||
| rid: 'roomId', | ||
| _id: 'messageId', | ||
| _updatedAt: date, | ||
| urls: [], | ||
| }; | ||
|
|
||
| it('should not keep mounted a plain message', () => { | ||
| const messages: IMessage[] = [{ ...baseMessage }]; | ||
| expect(useKeepMountedMessages(messages)).toEqual([]); | ||
| }); | ||
|
|
||
| it('should keep mounted a message with files', () => { | ||
| const messages: IMessage[] = [{ ...baseMessage, files: [{ _id: 'fileId', name: 'file.png', type: 'image/png', format: 'png', size: 123 }] }]; | ||
|
Check failure on line 26 in apps/meteor/client/views/room/MessageList/hooks/useKeepMountedMessages.spec.ts
|
||
| expect(useKeepMountedMessages(messages)).toEqual([0]); | ||
| }); | ||
|
|
||
| it('should keep mounted a message with a URL preview', () => { | ||
| const messages: IMessage[] = [{ ...baseMessage, urls: [{ url: 'https://example.com', meta: { title: 'Example' } }] }]; | ||
| expect(useKeepMountedMessages(messages)).toEqual([0]); | ||
| }); | ||
|
|
||
| it('should not keep mounted a message with a URL that has no preview metadata', () => { | ||
| const messages: IMessage[] = [{ ...baseMessage, urls: [{ url: 'https://example.com', meta: {} }] }]; | ||
| expect(useKeepMountedMessages(messages)).toEqual([]); | ||
| }); | ||
|
|
||
| it('should keep mounted a message with a rich attachment from an app/integration (e.g. /giphy)', () => { | ||
| const attachments: MessageAttachment[] = [{ title: 'GIPHY', image_url: 'https://media.giphy.com/foo.gif' }]; | ||
| const messages: IMessage[] = [{ ...baseMessage, attachments }]; | ||
| expect(useKeepMountedMessages(messages)).toEqual([0]); | ||
| }); | ||
|
|
||
| it('should not keep mounted a message whose only attachment is a file attachment', () => { | ||
| const attachments: MessageAttachment[] = [{ type: 'file', title: 'file.png' } as MessageAttachment]; | ||
| const messages: IMessage[] = [{ ...baseMessage, attachments }]; | ||
| expect(useKeepMountedMessages(messages)).toEqual([]); | ||
| }); | ||
|
|
||
| it('should not keep mounted a message whose only attachment is a quote attachment', () => { | ||
| const attachments: MessageAttachment[] = [ | ||
| { author_name: 'someone', author_icon: 'icon', message_link: 'https://example.com/msg', text: 'quoted text' }, | ||
| ]; | ||
| const messages: IMessage[] = [{ ...baseMessage, attachments }]; | ||
| expect(useKeepMountedMessages(messages)).toEqual([]); | ||
| }); | ||
|
|
||
| it('should keep mounted a message quoting another message that had a rich attachment (e.g. quoting a /giphy gif)', () => { | ||
| const attachments: MessageAttachment[] = [ | ||
| { | ||
| author_name: 'someone', | ||
| author_icon: 'icon', | ||
| message_link: 'https://example.com/msg', | ||
| text: 'quoted text', | ||
| attachments: [{ title: 'GIPHY', image_url: 'https://media.giphy.com/foo.gif' }], | ||
| }, | ||
| ]; | ||
| const messages: IMessage[] = [{ ...baseMessage, attachments }]; | ||
| expect(useKeepMountedMessages(messages)).toEqual([0]); | ||
| }); | ||
|
|
||
| it('should not keep mounted a message quoting another message whose only nested attachment is a file attachment', () => { | ||
| const attachments: MessageAttachment[] = [ | ||
| { | ||
| author_name: 'someone', | ||
| author_icon: 'icon', | ||
| message_link: 'https://example.com/msg', | ||
| text: 'quoted text', | ||
| attachments: [{ type: 'file', title: 'file.png' } as MessageAttachment], | ||
| }, | ||
| ]; | ||
| const messages: IMessage[] = [{ ...baseMessage, attachments }]; | ||
| expect(useKeepMountedMessages(messages)).toEqual([]); | ||
| }); | ||
|
|
||
| it('should keep mounted a message with a chain of nested quotes ending in a rich attachment', () => { | ||
| const attachments: MessageAttachment[] = [ | ||
| { | ||
| author_name: 'someone', | ||
| author_icon: 'icon', | ||
| message_link: 'https://example.com/msg1', | ||
| text: 'quoted text 1', | ||
| attachments: [ | ||
| { | ||
| author_name: 'someone-else', | ||
| author_icon: 'icon', | ||
| message_link: 'https://example.com/msg2', | ||
| text: 'quoted text 2', | ||
| attachments: [{ title: 'GIPHY', image_url: 'https://media.giphy.com/foo.gif' }], | ||
| }, | ||
| ], | ||
| }, | ||
| ]; | ||
| const messages: IMessage[] = [{ ...baseMessage, attachments }]; | ||
| expect(useKeepMountedMessages(messages)).toEqual([0]); | ||
| }); | ||
|
|
||
| it('should offset indexes by 1 when canPreview is true', () => { | ||
| const attachments: MessageAttachment[] = [{ title: 'GIPHY', image_url: 'https://media.giphy.com/foo.gif' }]; | ||
| const messages: IMessage[] = [{ ...baseMessage }, { ...baseMessage, _id: 'messageId2', attachments }]; | ||
| expect(useKeepMountedMessages(messages, true)).toEqual([2]); | ||
| }); | ||
28 changes: 22 additions & 6 deletions
28
apps/meteor/client/views/room/MessageList/hooks/useKeepMountedMessages.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.