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.
This commit is contained in:
David Langley 2025-08-26 12:12:34 +01:00 committed by GitHub
parent 67e0ecc454
commit 14d16364db
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 39 additions and 3 deletions

View File

@ -5,7 +5,7 @@
* Please see LICENSE files in the repository root for full details. * 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 Room } from "matrix-js-sdk/src/matrix";
import { type ScrollIntoViewLocation } from "react-virtuoso"; import { type ScrollIntoViewLocation } from "react-virtuoso";
import { isEqual } from "lodash"; import { isEqual } from "lodash";
@ -33,6 +33,7 @@ export function RoomList({ vm: { roomsResult, activeIndex } }: RoomListProps): J
const lastSpaceId = useRef<string | undefined>(undefined); const lastSpaceId = useRef<string | undefined>(undefined);
const lastFilterKeys = useRef<FilterKey[] | undefined>(undefined); const lastFilterKeys = useRef<FilterKey[] | undefined>(undefined);
const roomCount = roomsResult.rooms.length; const roomCount = roomsResult.rooms.length;
const [isScrolling, setIsScrolling] = useState(false);
const getItemComponent = useCallback( const getItemComponent = useCallback(
( (
index: number, index: number,
@ -57,10 +58,11 @@ export function RoomList({ vm: { roomsResult, activeIndex } }: RoomListProps): J
roomIndex={index} roomIndex={index}
roomCount={roomCount} roomCount={roomCount}
onFocus={onFocus} onFocus={onFocus}
listIsScrolling={isScrolling}
/> />
); );
}, },
[activeIndex, roomCount], [activeIndex, roomCount, isScrolling],
); );
const getItemKey = useCallback((item: Room): string => { const getItemKey = useCallback((item: Room): string => {
@ -116,6 +118,7 @@ export function RoomList({ vm: { roomsResult, activeIndex } }: RoomListProps): J
getItemKey={getItemKey} getItemKey={getItemKey}
isItemFocusable={() => true} isItemFocusable={() => true}
onKeyDown={keyDownCallback} onKeyDown={keyDownCallback}
isScrolling={setIsScrolling}
/> />
); );
} }

View File

@ -41,6 +41,10 @@ interface RoomListItemViewProps extends React.HTMLAttributes<HTMLButtonElement>
* The total number of rooms in the list * The total number of rooms in the list
*/ */
roomCount: number; roomCount: number;
/**
* Whether the list is currently scrolling
*/
listIsScrolling: boolean;
} }
/** /**
@ -53,6 +57,7 @@ export const RoomListItemView = memo(function RoomListItemView({
onFocus, onFocus,
roomIndex: index, roomIndex: index,
roomCount: count, roomCount: count,
listIsScrolling,
...props ...props
}: RoomListItemViewProps): JSX.Element { }: RoomListItemViewProps): JSX.Element {
const ref = useRef<HTMLButtonElement>(null); const ref = useRef<HTMLButtonElement>(null);
@ -141,7 +146,11 @@ export const RoomListItemView = memo(function RoomListItemView({
</Flex> </Flex>
); );
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 ( return (
<RoomListItemContextMenuView <RoomListItemContextMenuView

View File

@ -37,6 +37,7 @@ describe("<RoomListItemView />", () => {
onFocus: jest.fn(), onFocus: jest.fn(),
roomIndex: 0, roomIndex: 0,
roomCount: 1, roomCount: 1,
listIsScrolling: false,
}; };
return render(<RoomListItemView {...defaultProps} {...props} />, withClientContextRenderOptions(matrixClient)); return render(<RoomListItemView {...defaultProps} {...props} />, withClientContextRenderOptions(matrixClient));
@ -128,6 +129,7 @@ describe("<RoomListItemView />", () => {
onFocus={jest.fn()} onFocus={jest.fn()}
roomIndex={0} roomIndex={0}
roomCount={1} roomCount={1}
listIsScrolling={false}
/>, />,
); );
@ -191,4 +193,26 @@ describe("<RoomListItemView />", () => {
await user.keyboard("{Escape}"); await user.keyboard("{Escape}");
expect(screen.queryByRole("menu")).toBeNull(); 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();
});
}); });