Skip to content

Commit e629995

Browse files
qmorozovqmorozov
committed
Fix duplicate context menu on right-click in room list item menus
1 parent 9cecd52 commit e629995

File tree

5 files changed

+160
-73
lines changed

5 files changed

+160
-73
lines changed

‎src/components/views/rooms/RoomListPanel/RoomListItemMenuView.tsx‎

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -72,19 +72,26 @@ function MoreOptionsMenu({ vm, setMenuOpen }: MoreOptionsMenuProps): JSX.Element
7272
const [open, setOpen] = useState(false);
7373

7474
return (
75-
<Menu
76-
open={open}
77-
onOpenChange={(isOpen) => {
78-
setOpen(isOpen);
79-
setMenuOpen(isOpen);
75+
<div
76+
onContextMenu={(e) => {
77+
// Prevent opening duplicate more options menu via right-click
78+
if (open) e.stopPropagation();
8079
}}
81-
title={_t("room_list|room|more_options")}
82-
showTitle={false}
83-
align="start"
84-
trigger={<MoreOptionsButton size="24px" />}
8580
>
86-
<MoreOptionContent vm={vm} />
87-
</Menu>
81+
<Menu
82+
open={open}
83+
onOpenChange={(isOpen) => {
84+
setOpen(isOpen);
85+
setMenuOpen(isOpen);
86+
}}
87+
title={_t("room_list|room|more_options")}
88+
showTitle={false}
89+
align="start"
90+
trigger={<MoreOptionsButton size="24px" />}
91+
>
92+
<MoreOptionContent vm={vm} />
93+
</Menu>
94+
</div>
8895
);
8996
}
9097

@@ -202,6 +209,16 @@ function NotificationMenu({ vm, setMenuOpen }: NotificationMenuProps): JSX.Eleme
202209
<div
203210
// We don't want keyboard navigation events to bubble up to the ListView changing the focused item
204211
onKeyDown={(e) => e.stopPropagation()}
212+
onContextMenu={() => {
213+
if (open) {
214+
// Close notification menu and allow context menu to open via event bubbling
215+
// setTimeout ensures the menu closes after the current event completes bubbling
216+
setTimeout(() => {
217+
setOpen(false);
218+
setMenuOpen(false);
219+
}, 0);
220+
}
221+
}}
205222
>
206223
<Menu
207224
open={open}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,8 +157,8 @@ export const RoomListItemView = memo(function RoomListItemView({
157157
room={room}
158158
setMenuOpen={(isOpen) => {
159159
if (isOpen) {
160-
// To avoid icon blinking when the context menu is re-opened
161-
setTimeout(() => setIsMenuOpen(true), 0);
160+
// When context menu opens, hide hover menu to prevent overlapping menus
161+
setIsMenuOpen(false);
162162
} else {
163163
closeMenu();
164164
}

‎test/unit-tests/components/views/rooms/RoomListPanel/RoomListItemMenuView-test.tsx‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,4 +153,43 @@ describe("<RoomListItemMenuView />", () => {
153153
await user.click(screen.getByRole("menuitem", { name: "Mute room" }));
154154
expect(defaultValue.setRoomNotifState).toHaveBeenCalledWith(RoomNotifState.Mute);
155155
});
156+
157+
it("should prevent context menu event bubbling when right-clicking on open more options menu", async () => {
158+
const user = userEvent.setup();
159+
const { container } = renderMenu();
160+
161+
await user.click(screen.getByRole("button", { name: "More Options" }));
162+
await screen.findByRole("menuitem", { name: "Mark as read" });
163+
164+
const contextMenuHandler = jest.fn();
165+
container.addEventListener("contextmenu", contextMenuHandler);
166+
167+
const openMenu = screen.getByRole("menu", { name: "More Options" });
168+
const menuWrapper = openMenu.parentElement;
169+
const contextMenuEvent = new MouseEvent("contextmenu", { bubbles: true, cancelable: true });
170+
menuWrapper?.dispatchEvent(contextMenuEvent);
171+
172+
expect(contextMenuHandler).not.toHaveBeenCalled();
173+
container.removeEventListener("contextmenu", contextMenuHandler);
174+
});
175+
176+
it("should close notification menu when right-clicking on open notification menu", async () => {
177+
const user = userEvent.setup();
178+
const setMenuOpen = jest.fn();
179+
renderMenu(setMenuOpen);
180+
181+
await user.click(screen.getByRole("button", { name: "Notification options" }));
182+
await screen.findByRole("menuitem", { name: "Match default settings" });
183+
184+
setMenuOpen.mockClear();
185+
186+
const openNotificationMenu = screen.getByRole("menu", { name: "Notification options" });
187+
const notificationMenuWrapper = openNotificationMenu.parentElement; // This should be the div with onContextMenu
188+
const contextMenuEvent = new MouseEvent("contextmenu", { bubbles: true, cancelable: true });
189+
notificationMenuWrapper?.dispatchEvent(contextMenuEvent);
190+
191+
await new Promise(resolve => setTimeout(resolve, 10));
192+
193+
expect(setMenuOpen).toHaveBeenCalledWith(false);
194+
});
156195
});

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,4 +215,31 @@ describe("<RoomListItemView />", () => {
215215
// But the room item itself should still be rendered
216216
expect(button).toBeInTheDocument();
217217
});
218+
219+
test("should hide hover menu when context menu opens", async () => {
220+
const user = userEvent.setup();
221+
222+
mocked(useRoomListItemViewModel).mockReturnValue({
223+
...defaultValue,
224+
showContextMenu: true,
225+
showHoverMenu: true,
226+
});
227+
228+
renderRoomListItem();
229+
230+
const button = screen.getByRole("option", { name: `Open room ${room.name}` });
231+
232+
// First hover to show hover menu
233+
await user.hover(button);
234+
await waitFor(() => expect(screen.getByRole("button", { name: "More Options" })).toBeInTheDocument());
235+
236+
// Then right-click to open context menu
237+
await user.pointer([{ target: button }, { keys: "[MouseRight]", target: button }]);
238+
239+
// Context menu should be open
240+
await waitFor(() => expect(screen.getByRole("menu")).toBeInTheDocument());
241+
242+
// Hover menu (More Options button) should be hidden when context menu is open
243+
expect(screen.queryByRole("button", { name: "More Options" })).toBeNull();
244+
});
218245
});

‎test/unit-tests/components/views/rooms/RoomListPanel/__snapshots__/RoomListItemMenuView-test.tsx.snap‎

Lines changed: 64 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -6,38 +6,40 @@ exports[`<RoomListItemMenuView /> should render the more options menu 1`] = `
66
class="flex mx_RoomListItemMenuView"
77
style="--mx-flex-display: flex; --mx-flex-direction: row; --mx-flex-align: center; --mx-flex-justify: start; --mx-flex-gap: var(--cpd-space-1x); --mx-flex-wrap: nowrap;"
88
>
9-
<button
10-
aria-disabled="false"
11-
aria-expanded="false"
12-
aria-haspopup="menu"
13-
aria-label="More Options"
14-
aria-labelledby="«r2»"
15-
class="_icon-button_1pz9o_8"
16-
data-kind="primary"
17-
data-state="closed"
18-
id="radix-«r0»"
19-
role="button"
20-
style="--cpd-icon-button-size: 24px;"
21-
tabindex="0"
22-
type="button"
23-
>
24-
<div
25-
class="_indicator-icon_zr2a0_17"
26-
style="--cpd-icon-button-size: 100%;"
9+
<div>
10+
<button
11+
aria-disabled="false"
12+
aria-expanded="false"
13+
aria-haspopup="menu"
14+
aria-label="More Options"
15+
aria-labelledby="«r2»"
16+
class="_icon-button_1pz9o_8"
17+
data-kind="primary"
18+
data-state="closed"
19+
id="radix-«r0»"
20+
role="button"
21+
style="--cpd-icon-button-size: 24px;"
22+
tabindex="0"
23+
type="button"
2724
>
28-
<svg
29-
fill="currentColor"
30-
height="1em"
31-
viewBox="0 0 24 24"
32-
width="1em"
33-
xmlns="http://www.w3.org/2000/svg"
25+
<div
26+
class="_indicator-icon_zr2a0_17"
27+
style="--cpd-icon-button-size: 100%;"
3428
>
35-
<path
36-
d="M6 14q-.824 0-1.412-.588A1.93 1.93 0 0 1 4 12q0-.825.588-1.412A1.93 1.93 0 0 1 6 10q.824 0 1.412.588Q8 11.175 8 12t-.588 1.412A1.93 1.93 0 0 1 6 14m6 0q-.825 0-1.412-.588A1.93 1.93 0 0 1 10 12q0-.825.588-1.412A1.93 1.93 0 0 1 12 10q.825 0 1.412.588Q14 11.175 14 12t-.588 1.412A1.93 1.93 0 0 1 12 14m6 0q-.824 0-1.413-.588A1.93 1.93 0 0 1 16 12q0-.825.587-1.412A1.93 1.93 0 0 1 18 10q.824 0 1.413.588Q20 11.175 20 12t-.587 1.412A1.93 1.93 0 0 1 18 14"
37-
/>
38-
</svg>
39-
</div>
40-
</button>
29+
<svg
30+
fill="currentColor"
31+
height="1em"
32+
viewBox="0 0 24 24"
33+
width="1em"
34+
xmlns="http://www.w3.org/2000/svg"
35+
>
36+
<path
37+
d="M6 14q-.824 0-1.412-.588A1.93 1.93 0 0 1 4 12q0-.825.588-1.412A1.93 1.93 0 0 1 6 10q.824 0 1.412.588Q8 11.175 8 12t-.588 1.412A1.93 1.93 0 0 1 6 14m6 0q-.825 0-1.412-.588A1.93 1.93 0 0 1 10 12q0-.825.588-1.412A1.93 1.93 0 0 1 12 10q.825 0 1.412.588Q14 11.175 14 12t-.588 1.412A1.93 1.93 0 0 1 12 14m6 0q-.824 0-1.413-.588A1.93 1.93 0 0 1 16 12q0-.825.587-1.412A1.93 1.93 0 0 1 18 10q.824 0 1.413.588Q20 11.175 20 12t-.587 1.412A1.93 1.93 0 0 1 18 14"
38+
/>
39+
</svg>
40+
</div>
41+
</button>
42+
</div>
4143
<div>
4244
<button
4345
aria-disabled="false"
@@ -85,38 +87,40 @@ exports[`<RoomListItemMenuView /> should render the notification options menu 1`
8587
class="flex mx_RoomListItemMenuView"
8688
style="--mx-flex-display: flex; --mx-flex-direction: row; --mx-flex-align: center; --mx-flex-justify: start; --mx-flex-gap: var(--cpd-space-1x); --mx-flex-wrap: nowrap;"
8789
>
88-
<button
89-
aria-disabled="false"
90-
aria-expanded="false"
91-
aria-haspopup="menu"
92-
aria-label="More Options"
93-
aria-labelledby="«ri»"
94-
class="_icon-button_1pz9o_8"
95-
data-kind="primary"
96-
data-state="closed"
97-
id="radix-«rg»"
98-
role="button"
99-
style="--cpd-icon-button-size: 24px;"
100-
tabindex="0"
101-
type="button"
102-
>
103-
<div
104-
class="_indicator-icon_zr2a0_17"
105-
style="--cpd-icon-button-size: 100%;"
90+
<div>
91+
<button
92+
aria-disabled="false"
93+
aria-expanded="false"
94+
aria-haspopup="menu"
95+
aria-label="More Options"
96+
aria-labelledby="«ri»"
97+
class="_icon-button_1pz9o_8"
98+
data-kind="primary"
99+
data-state="closed"
100+
id="radix-«rg»"
101+
role="button"
102+
style="--cpd-icon-button-size: 24px;"
103+
tabindex="0"
104+
type="button"
106105
>
107-
<svg
108-
fill="currentColor"
109-
height="1em"
110-
viewBox="0 0 24 24"
111-
width="1em"
112-
xmlns="http://www.w3.org/2000/svg"
106+
<div
107+
class="_indicator-icon_zr2a0_17"
108+
style="--cpd-icon-button-size: 100%;"
113109
>
114-
<path
115-
d="M6 14q-.824 0-1.412-.588A1.93 1.93 0 0 1 4 12q0-.825.588-1.412A1.93 1.93 0 0 1 6 10q.824 0 1.412.588Q8 11.175 8 12t-.588 1.412A1.93 1.93 0 0 1 6 14m6 0q-.825 0-1.412-.588A1.93 1.93 0 0 1 10 12q0-.825.588-1.412A1.93 1.93 0 0 1 12 10q.825 0 1.412.588Q14 11.175 14 12t-.588 1.412A1.93 1.93 0 0 1 12 14m6 0q-.824 0-1.413-.588A1.93 1.93 0 0 1 16 12q0-.825.587-1.412A1.93 1.93 0 0 1 18 10q.824 0 1.413.588Q20 11.175 20 12t-.587 1.412A1.93 1.93 0 0 1 18 14"
116-
/>
117-
</svg>
118-
</div>
119-
</button>
110+
<svg
111+
fill="currentColor"
112+
height="1em"
113+
viewBox="0 0 24 24"
114+
width="1em"
115+
xmlns="http://www.w3.org/2000/svg"
116+
>
117+
<path
118+
d="M6 14q-.824 0-1.412-.588A1.93 1.93 0 0 1 4 12q0-.825.588-1.412A1.93 1.93 0 0 1 6 10q.824 0 1.412.588Q8 11.175 8 12t-.588 1.412A1.93 1.93 0 0 1 6 14m6 0q-.825 0-1.412-.588A1.93 1.93 0 0 1 10 12q0-.825.588-1.412A1.93 1.93 0 0 1 12 10q.825 0 1.412.588Q14 11.175 14 12t-.588 1.412A1.93 1.93 0 0 1 12 14m6 0q-.824 0-1.413-.588A1.93 1.93 0 0 1 16 12q0-.825.587-1.412A1.93 1.93 0 0 1 18 10q.824 0 1.413.588Q20 11.175 20 12t-.587 1.412A1.93 1.93 0 0 1 18 14"
119+
/>
120+
</svg>
121+
</div>
122+
</button>
123+
</div>
120124
<div>
121125
<button
122126
aria-disabled="false"

0 commit comments

Comments
 (0)