From 14d16364dbb5e981efc0876b01190bb420a397a9 Mon Sep 17 00:00:00 2001 From: David Langley Date: Tue, 26 Aug 2025 12:12:34 +0100 Subject: [PATCH] Don't render context menu when scrolling (#30613) * Don't render context menu when scrolling * Add test to check context menu is not rendered when scrolling * Add comment. --- .../views/rooms/RoomListPanel/RoomList.tsx | 7 ++++-- .../rooms/RoomListPanel/RoomListItemView.tsx | 11 ++++++++- .../RoomListPanel/RoomListItemView-test.tsx | 24 +++++++++++++++++++ 3 files changed, 39 insertions(+), 3 deletions(-) diff --git a/src/components/views/rooms/RoomListPanel/RoomList.tsx b/src/components/views/rooms/RoomListPanel/RoomList.tsx index e35118c146..cf72ef4392 100644 --- a/src/components/views/rooms/RoomListPanel/RoomList.tsx +++ b/src/components/views/rooms/RoomListPanel/RoomList.tsx @@ -5,7 +5,7 @@ * Please see LICENSE files in the repository root for full details. */ -import React, { useCallback, useRef, type JSX } from "react"; +import React, { useCallback, useRef, useState, type JSX } from "react"; import { type Room } from "matrix-js-sdk/src/matrix"; import { type ScrollIntoViewLocation } from "react-virtuoso"; import { isEqual } from "lodash"; @@ -33,6 +33,7 @@ export function RoomList({ vm: { roomsResult, activeIndex } }: RoomListProps): J const lastSpaceId = useRef(undefined); const lastFilterKeys = useRef(undefined); const roomCount = roomsResult.rooms.length; + const [isScrolling, setIsScrolling] = useState(false); const getItemComponent = useCallback( ( index: number, @@ -57,10 +58,11 @@ export function RoomList({ vm: { roomsResult, activeIndex } }: RoomListProps): J roomIndex={index} roomCount={roomCount} onFocus={onFocus} + listIsScrolling={isScrolling} /> ); }, - [activeIndex, roomCount], + [activeIndex, roomCount, isScrolling], ); const getItemKey = useCallback((item: Room): string => { @@ -116,6 +118,7 @@ export function RoomList({ vm: { roomsResult, activeIndex } }: RoomListProps): J getItemKey={getItemKey} isItemFocusable={() => true} onKeyDown={keyDownCallback} + isScrolling={setIsScrolling} /> ); } diff --git a/src/components/views/rooms/RoomListPanel/RoomListItemView.tsx b/src/components/views/rooms/RoomListPanel/RoomListItemView.tsx index 81e63e6f6f..500fbe953f 100644 --- a/src/components/views/rooms/RoomListPanel/RoomListItemView.tsx +++ b/src/components/views/rooms/RoomListPanel/RoomListItemView.tsx @@ -41,6 +41,10 @@ interface RoomListItemViewProps extends React.HTMLAttributes * The total number of rooms in the list */ roomCount: number; + /** + * Whether the list is currently scrolling + */ + listIsScrolling: boolean; } /** @@ -53,6 +57,7 @@ export const RoomListItemView = memo(function RoomListItemView({ onFocus, roomIndex: index, roomCount: count, + listIsScrolling, ...props }: RoomListItemViewProps): JSX.Element { const ref = useRef(null); @@ -141,7 +146,11 @@ export const RoomListItemView = memo(function RoomListItemView({ ); - if (!vm.showContextMenu) return content; + // Rendering multiple context menus can causes crashes in radix upstream, + // See https://github.com/radix-ui/primitives/issues/2717. + // We also don't need the context menu while scrolling so can improve scroll performance + // by not rendering it. + if (!vm.showContextMenu || listIsScrolling) return content; return ( ", () => { onFocus: jest.fn(), roomIndex: 0, roomCount: 1, + listIsScrolling: false, }; return render(, withClientContextRenderOptions(matrixClient)); @@ -128,6 +129,7 @@ describe("", () => { onFocus={jest.fn()} roomIndex={0} roomCount={1} + listIsScrolling={false} />, ); @@ -191,4 +193,26 @@ describe("", () => { await user.keyboard("{Escape}"); expect(screen.queryByRole("menu")).toBeNull(); }); + + test("should not render context menu when list is scrolling", async () => { + const user = userEvent.setup(); + + mocked(useRoomListItemViewModel).mockReturnValue({ + ...defaultValue, + showContextMenu: true, + }); + + renderRoomListItem({ + listIsScrolling: true, + }); + + const button = screen.getByRole("option", { name: `Open room ${room.name}` }); + await user.pointer([{ target: button }, { keys: "[MouseRight]", target: button }]); + + // Context menu should not appear when scrolling + expect(screen.queryByRole("menu")).toBeNull(); + + // But the room item itself should still be rendered + expect(button).toBeInTheDocument(); + }); });