Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
7 changes: 5 additions & 2 deletions src/components/views/rooms/RoomListPanel/RoomList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -33,6 +33,7 @@ export function RoomList({ vm: { roomsResult, activeIndex } }: RoomListProps): J
const lastSpaceId = useRef<string | undefined>(undefined);
const lastFilterKeys = useRef<FilterKey[] | undefined>(undefined);
const roomCount = roomsResult.rooms.length;
const [isScrolling, setIsScrolling] = useState(false);
const getItemComponent = useCallback(
(
index: number,
Expand All @@ -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 => {
Expand Down Expand Up @@ -116,6 +118,7 @@ export function RoomList({ vm: { roomsResult, activeIndex } }: RoomListProps): J
getItemKey={getItemKey}
isItemFocusable={() => true}
onKeyDown={keyDownCallback}
isScrolling={setIsScrolling}
/>
);
}
11 changes: 10 additions & 1 deletion src/components/views/rooms/RoomListPanel/RoomListItemView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ interface RoomListItemViewProps extends React.HTMLAttributes<HTMLButtonElement>
* The total number of rooms in the list
*/
roomCount: number;
/**
* Whether the list is currently scrolling
*/
listIsScrolling: boolean;
}

/**
Expand All @@ -53,6 +57,7 @@ export const RoomListItemView = memo(function RoomListItemView({
onFocus,
roomIndex: index,
roomCount: count,
listIsScrolling,
...props
}: RoomListItemViewProps): JSX.Element {
const ref = useRef<HTMLButtonElement>(null);
Expand Down Expand Up @@ -141,7 +146,11 @@ export const RoomListItemView = memo(function RoomListItemView({
</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;
Copy link
Member

Choose a reason for hiding this comment

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

A comment will be great to explain why we don't want to display the context menu when scrolling


return (
<RoomListItemContextMenuView
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ describe("<RoomListItemView />", () => {
onFocus: jest.fn(),
roomIndex: 0,
roomCount: 1,
listIsScrolling: false,
};

return render(<RoomListItemView {...defaultProps} {...props} />, withClientContextRenderOptions(matrixClient));
Expand Down Expand Up @@ -128,6 +129,7 @@ describe("<RoomListItemView />", () => {
onFocus={jest.fn()}
roomIndex={0}
roomCount={1}
listIsScrolling={false}
/>,
);

Expand Down Expand Up @@ -191,4 +193,26 @@ describe("<RoomListItemView />", () => {
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();
});
});
Loading