Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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: 5 additions & 0 deletions .changeset/lucky-boxes-shave.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@rocket.chat/meteor": patch
---

Fixes the message list often not scrolling to the bottom after you send a message. The scroll was triggered by the server's echo of the message, which is skipped whenever the response to the send is processed first — the message is then no longer recognised as new and nothing tells the list to move. It now scrolls as soon as the message is appended locally, so it no longer waits for the round-trip.
40 changes: 40 additions & 0 deletions apps/meteor/client/views/room/MessageList/MessageList.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ describe('MessageList scroll position', () => {

beforeEach(() => {
jest.clearAllMocks();
defaultProps.isAtBottom.current = false;
mockVirtualizerHandle.scrollToIndex.mockClear();
mockVirtualizerHandle.scrollTo.mockClear();
mockVirtualizerHandle.findItemIndex.mockImplementation((offset: number) => offset);
Expand Down Expand Up @@ -178,6 +179,45 @@ describe('MessageList scroll position', () => {
expect(defaultProps.setShouldJumpToBottom).toHaveBeenCalledWith(true);
});

it("should jump to bottom when the current user's optimistic message is appended", () => {
const store = {
scroll: 123,
atBottom: false,
update: jest.fn(),
};
(RoomManager.getStore as jest.Mock).mockReturnValue(store);

const { rerender } = render(<MessageList {...defaultProps} />, { wrapper: root.build() });
defaultProps.setShouldJumpToBottom.mockClear();

const ownOptimisticMessage = { ...createMessage('message-3'), temp: true } as IMessage;
(useMessages as jest.Mock).mockReturnValue([createMessage('message-1'), createMessage('message-2'), ownOptimisticMessage]);
rerender(<MessageList {...defaultProps} />);

expect(defaultProps.setShouldJumpToBottom).toHaveBeenCalledWith(true);
Comment thread
ndo84bw marked this conversation as resolved.
});

it('should not jump to bottom when someone else sends a message', () => {
const store = {
scroll: 123,
atBottom: false,
update: jest.fn(),
};
(RoomManager.getStore as jest.Mock).mockReturnValue(store);

const { rerender } = render(<MessageList {...defaultProps} />, { wrapper: root.build() });
defaultProps.setShouldJumpToBottom.mockClear();

const otherUserMessage = {
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
...createMessage('message-3'),
u: { _id: 'other-user-id', username: 'other' },
} as IMessage;
(useMessages as jest.Mock).mockReturnValue([createMessage('message-1'), createMessage('message-2'), otherUserMessage]);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
rerender(<MessageList {...defaultProps} />);

expect(defaultProps.setShouldJumpToBottom).not.toHaveBeenCalledWith(true);
});

it('should do nothing if no previous scroll position is stored', () => {
const store = {
scroll: undefined,
Expand Down
22 changes: 20 additions & 2 deletions apps/meteor/client/views/room/MessageList/MessageList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ export const MessageList = function MessageList({

const virtualizerRef = useRef<VirtualizerHandle | null>(null);
const lastScrollSizeRef = useRef(0);
const prevMessagesLengthRef = useRef(0);
const ownUserId = user?._id;

const messages = useMessages({ rid });

Expand Down Expand Up @@ -178,10 +180,25 @@ export const MessageList = function MessageList({

const handle = virtualizerRef.current;
const lastItemIndex = messages.length - 1;
if (shouldJumpToBottom === true) {

// Scroll to bottom when the user's own temp message is appended, so the list does not
// depend on streamNewMessage, which may not run for a message the client already added.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
const prevMessagesLength = prevMessagesLengthRef.current;
prevMessagesLengthRef.current = messages.length;
if (messages.length > prevMessagesLength && ownUserId) {
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
Outdated
const lastMessage = messages.at(-1);
if (lastMessage?.temp && lastMessage.u._id === ownUserId) {
setShouldJumpToBottom(true);
}
}

if (shouldJumpToBottom === true && handle) {
// Mark as at-bottom before the scroll runs, so useKeepAtBottom re-scrolls if the
// content grows in between. Only with a handle, otherwise nothing would scroll.
isAtBottom.current = true;
// When new messages arrive, this effect is triggered, but the latest message is not on the index, so it scrolls to the previous index
// TODO: Find if there is a better way to scroll to the latest message
handle?.scrollToIndex(lastItemIndex + 1, {
handle.scrollToIndex(lastItemIndex + 1, {
align: 'center',
});
}
Expand All @@ -202,6 +219,7 @@ export const MessageList = function MessageList({
rid,
firstUnreadMessageId,
setShouldJumpToBottom,
ownUserId,
]);

const storeScrollPosition = useStoreScrollPosition({ rid, isAtBottom, virtualizerRef });
Expand Down