Skip to content

Commit 14d1636

Browse files
authored
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.
1 parent 67e0ecc commit 14d1636

File tree

3 files changed

+39
-3
lines changed

3 files changed

+39
-3
lines changed

src/components/views/rooms/RoomListPanel/RoomList.tsx

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
* Please see LICENSE files in the repository root for full details.
66
*/
77

8-
import React, { useCallback, useRef, type JSX } from "react";
8+
import React, { useCallback, useRef, useState, type JSX } from "react";
99
import { type Room } from "matrix-js-sdk/src/matrix";
1010
import { type ScrollIntoViewLocation } from "react-virtuoso";
1111
import { isEqual } from "lodash";
@@ -33,6 +33,7 @@ export function RoomList({ vm: { roomsResult, activeIndex } }: RoomListProps): J
3333
const lastSpaceId = useRef<string | undefined>(undefined);
3434
const lastFilterKeys = useRef<FilterKey[] | undefined>(undefined);
3535
const roomCount = roomsResult.rooms.length;
36+
const [isScrolling, setIsScrolling] = useState(false);
3637
const getItemComponent = useCallback(
3738
(
3839
index: number,
@@ -57,10 +58,11 @@ export function RoomList({ vm: { roomsResult, activeIndex } }: RoomListProps): J
5758
roomIndex={index}
5859
roomCount={roomCount}
5960
onFocus={onFocus}
61+
listIsScrolling={isScrolling}
6062
/>
6163
);
6264
},
63-
[activeIndex, roomCount],
65+
[activeIndex, roomCount, isScrolling],
6466
);
6567

6668
const getItemKey = useCallback((item: Room): string => {
@@ -116,6 +118,7 @@ export function RoomList({ vm: { roomsResult, activeIndex } }: RoomListProps): J
116118
getItemKey={getItemKey}
117119
isItemFocusable={() => true}
118120
onKeyDown={keyDownCallback}
121+
isScrolling={setIsScrolling}
119122
/>
120123
);
121124
}

src/components/views/rooms/RoomListPanel/RoomListItemView.tsx

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,10 @@ interface RoomListItemViewProps extends React.HTMLAttributes<HTMLButtonElement>
4141
* The total number of rooms in the list
4242
*/
4343
roomCount: number;
44+
/**
45+
* Whether the list is currently scrolling
46+
*/
47+
listIsScrolling: boolean;
4448
}
4549

4650
/**
@@ -53,6 +57,7 @@ export const RoomListItemView = memo(function RoomListItemView({
5357
onFocus,
5458
roomIndex: index,
5559
roomCount: count,
60+
listIsScrolling,
5661
...props
5762
}: RoomListItemViewProps): JSX.Element {
5863
const ref = useRef<HTMLButtonElement>(null);
@@ -141,7 +146,11 @@ export const RoomListItemView = memo(function RoomListItemView({
141146
</Flex>
142147
);
143148

144-
if (!vm.showContextMenu) return content;
149+
// Rendering multiple context menus can causes crashes in radix upstream,
150+
// See https://github.com/radix-ui/primitives/issues/2717.
151+
// We also don't need the context menu while scrolling so can improve scroll performance
152+
// by not rendering it.
153+
if (!vm.showContextMenu || listIsScrolling) return content;
145154

146155
return (
147156
<RoomListItemContextMenuView

test/unit-tests/components/views/rooms/RoomListPanel/RoomListItemView-test.tsx

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ describe("<RoomListItemView />", () => {
3737
onFocus: jest.fn(),
3838
roomIndex: 0,
3939
roomCount: 1,
40+
listIsScrolling: false,
4041
};
4142

4243
return render(<RoomListItemView {...defaultProps} {...props} />, withClientContextRenderOptions(matrixClient));
@@ -128,6 +129,7 @@ describe("<RoomListItemView />", () => {
128129
onFocus={jest.fn()}
129130
roomIndex={0}
130131
roomCount={1}
132+
listIsScrolling={false}
131133
/>,
132134
);
133135

@@ -191,4 +193,26 @@ describe("<RoomListItemView />", () => {
191193
await user.keyboard("{Escape}");
192194
expect(screen.queryByRole("menu")).toBeNull();
193195
});
196+
197+
test("should not render context menu when list is scrolling", async () => {
198+
const user = userEvent.setup();
199+
200+
mocked(useRoomListItemViewModel).mockReturnValue({
201+
...defaultValue,
202+
showContextMenu: true,
203+
});
204+
205+
renderRoomListItem({
206+
listIsScrolling: true,
207+
});
208+
209+
const button = screen.getByRole("option", { name: `Open room ${room.name}` });
210+
await user.pointer([{ target: button }, { keys: "[MouseRight]", target: button }]);
211+
212+
// Context menu should not appear when scrolling
213+
expect(screen.queryByRole("menu")).toBeNull();
214+
215+
// But the room item itself should still be rendered
216+
expect(button).toBeInTheDocument();
217+
});
194218
});

0 commit comments

Comments
 (0)