New room list: add context menu to room list item (#29952)

florianduros · web-flow · commit f707bb410e7b · 2025-06-24T09:50:27.000Z
* chore: update compound-web

* chore: remove unused export

* feat: export content of more option menu

* feat: add context menu

* feat: add `showContextMenu` to vm

* feat: use context menu in new room list

* test: add tests for room list item

* test: fix room list test

* test: add `showContextMenu` test for `useRoomListItemViewModel`

* test: add e2e test for context menu

* chore: update compound

* test: update snapshots and e2e test

* fix: avoid icon blinking when we reopen the context menu

* test: add test for menu closing

* doc: remove useless tsdoc param

* chore: update `@vector-im/compound-web`

* refactor: remove manual focus

* test(e2e): fix focus after closing notification menu

* doc: remove useless jobs
diff --git a/package.json b/package.json
@@ -93,7 +93,7 @@
         "@types/png-chunks-extract": "^1.0.2",
         "@types/react-virtualized": "^9.21.30",
         "@vector-im/compound-design-tokens": "^4.0.0",
-        "@vector-im/compound-web": "^8.0.0",
+        "@vector-im/compound-web": "^8.1.2",
         "@vector-im/matrix-wysiwyg": "2.38.3",
         "@zxcvbn-ts/core": "^3.0.4",
         "@zxcvbn-ts/language-common": "^3.0.4",
diff --git a/playwright/e2e/left-panel/room-list-panel/room-list.spec.ts b/playwright/e2e/left-panel/room-list-panel/room-list.spec.ts
@@ -60,6 +60,12 @@ test.describe("Room list", () => {
             await expect(page.getByRole("heading", { name: "room29", level: 1 })).toBeVisible();
         });
 
+        test("should open the context menu", { tag: "@screenshot" }, async ({ page, app, user }) => {
+            const roomListView = getRoomList(page);
+            await roomListView.getByRole("gridcell", { name: "Open room room29" }).click({ button: "right" });
+            await expect(page.getByRole("menu", { name: "More Options" })).toBeVisible();
+        });
+
         test("should open the more options menu", { tag: "@screenshot" }, async ({ page, app, user }) => {
             const roomListView = getRoomList(page);
             const roomItem = roomListView.getByRole("gridcell", { name: "Open room room29" });
@@ -223,17 +229,17 @@ test.describe("Room list", () => {
                 await expect(notificationButton).toBeFocused();
 
                 // Open the menu
-                await notificationButton.click();
+                await page.keyboard.press("Enter");
                 // Wait for the menu to be open
                 await expect(page.getByRole("menuitem", { name: "Match default settings" })).toHaveAttribute(
                     "aria-selected",
                     "true",
                 );
 
-                // Close the menu
+                await page.keyboard.press("ArrowDown");
                 await page.keyboard.press("Escape");
-                // Focus should be back on the room list item
-                await expect(room29).toBeFocused();
+                // Focus should be back on the notification button
+                await expect(notificationButton).toBeFocused();
             });
         });
     });
diff --git a/src/components/viewmodels/roomlist/RoomListItemViewModel.tsx b/src/components/viewmodels/roomlist/RoomListItemViewModel.tsx
@@ -30,6 +30,10 @@ export interface RoomListItemViewState {
      * The name of the room.
      */
     name: string;
+    /**
+     * Whether the context menu should be shown.
+     */
+    showContextMenu: boolean;
     /**
      * Whether the hover menu should be shown.
      */
@@ -105,12 +109,12 @@ export function useRoomListItemViewModel(room: Room): RoomListItemViewState {
         setNotificationValues(getNotificationValues(notificationState));
     }, [notificationState]);
 
-    // We don't want to show the hover menu if
+    // We don't want to show the menus if
     // - there is an invitation for this room
-    // - the user doesn't have access to both notification and more options menus
+    // - the user doesn't have access to notification and more options menus
+    const showContextMenu = !invited && hasAccessToOptionsMenu(room);
     const showHoverMenu =
-        !invited &&
-        (hasAccessToOptionsMenu(room) || hasAccessToNotificationMenu(room, matrixClient.isGuest(), isArchived));
+        !invited && (showContextMenu || hasAccessToNotificationMenu(room, matrixClient.isGuest(), isArchived));
 
     const messagePreview = useRoomMessagePreview(room);
 
@@ -137,6 +141,7 @@ export function useRoomListItemViewModel(room: Room): RoomListItemViewState {
     return {
         name,
         notificationState,
+        showContextMenu,
         showHoverMenu,
         openRoom,
         a11yLabel,
diff --git a/src/components/views/rooms/RoomListPanel/RoomListItemContextMenuView.tsx b/src/components/views/rooms/RoomListPanel/RoomListItemContextMenuView.tsx
@@ -0,0 +1,50 @@
+/*
+ * Copyright 2025 New Vector Ltd.
+ *
+ * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
+ * Please see LICENSE files in the repository root for full details.
+ */
+
+import { type Room } from "matrix-js-sdk/src/matrix";
+import { type JSX, type PropsWithChildren } from "react";
+import { ContextMenu } from "@vector-im/compound-web";
+import React from "react";
+
+import { _t } from "../../../../languageHandler";
+import { MoreOptionContent } from "./RoomListItemMenuView";
+import { useRoomListItemMenuViewModel } from "../../../viewmodels/roomlist/RoomListItemMenuViewModel";
+
+interface RoomListItemContextMenuViewProps {
+    /**
+     * The room to display the menu for.
+     */
+    room: Room;
+    /**
+     * Set the menu open state.
+     */
+    setMenuOpen: (isOpen: boolean) => void;
+}
+
+/**
+ * A view for the room list item context menu.
+ */
+export function RoomListItemContextMenuView({
+    room,
+    setMenuOpen,
+    children,
+}: PropsWithChildren<RoomListItemContextMenuViewProps>): JSX.Element {
+    const vm = useRoomListItemMenuViewModel(room);
+
+    return (
+        <ContextMenu
+            title={_t("room_list|room|more_options")}
+            showTitle={false}
+            // To not mess with the roving tab index of the button
+            hasAccessibleAlternative={true}
+            trigger={children}
+            onOpenChange={setMenuOpen}
+        >
+            <MoreOptionContent vm={vm} />
+        </ContextMenu>
+    );
+}
diff --git a/src/components/views/rooms/RoomListPanel/RoomListItemMenuView.tsx b/src/components/views/rooms/RoomListPanel/RoomListItemMenuView.tsx
@@ -35,7 +35,6 @@ interface RoomListItemMenuViewProps {
     room: Room;
     /**
      * Set the menu open state.
-     * @param isOpen
      */
     setMenuOpen: (isOpen: boolean) => void;
 }
@@ -84,6 +83,21 @@ function MoreOptionsMenu({ vm, setMenuOpen }: MoreOptionsMenuProps): JSX.Element
             align="start"
             trigger={<MoreOptionsButton size="24px" />}
         >
+            <MoreOptionContent vm={vm} />
+        </Menu>
+    );
+}
+
+interface MoreOptionContentProps {
+    /**
+     * The view model state for the menu.
+     */
+    vm: RoomListItemMenuViewState;
+}
+
+export function MoreOptionContent({ vm }: MoreOptionContentProps): JSX.Element {
+    return (
+        <>
             {vm.canMarkAsRead && (
                 <MenuItem
                     Icon={MarkAsReadIcon}
@@ -143,7 +157,7 @@ function MoreOptionsMenu({ vm, setMenuOpen }: MoreOptionsMenuProps): JSX.Element
                 onClick={(evt) => evt.stopPropagation()}
                 hideChevron={true}
             />
-        </Menu>
+        </>
     );
 }
 
@@ -154,7 +168,7 @@ interface MoreOptionsButtonProps extends ComponentProps<typeof IconButton> {
 /**
  * A button to trigger the more options menu.
  */
-export const MoreOptionsButton = function MoreOptionsButton(props: MoreOptionsButtonProps): JSX.Element {
+const MoreOptionsButton = function MoreOptionsButton(props: MoreOptionsButtonProps): JSX.Element {
     return (
         <Tooltip label={_t("room_list|room|more_options")}>
             <IconButton aria-label={_t("room_list|room|more_options")} {...props}>
@@ -244,7 +258,7 @@ interface NotificationButtonProps extends ComponentProps<typeof IconButton> {
 /**
  * A button to trigger the notification menu.
  */
-export const NotificationButton = function MoreOptionsButton({
+const NotificationButton = function MoreOptionsButton({
     isRoomMuted,
     ref,
     ...props
diff --git a/src/components/views/rooms/RoomListPanel/RoomListItemView.tsx b/src/components/views/rooms/RoomListPanel/RoomListItemView.tsx
@@ -15,6 +15,7 @@ import { RoomListItemMenuView } from "./RoomListItemMenuView";
 import { NotificationDecoration } from "../NotificationDecoration";
 import { RoomAvatarView } from "../../avatars/RoomAvatarView";
 import { useRovingTabIndex } from "../../../../accessibility/RovingTabIndex";
+import { RoomListItemContextMenuView } from "./RoomListItemContextMenuView";
 
 interface RoomListItemViewProps extends React.HTMLAttributes<HTMLButtonElement> {
     /**
@@ -47,7 +48,13 @@ export const RoomListItemView = memo(function RoomListItemView({
     const showHoverDecoration = isMenuOpen || isHover;
     const showHoverMenu = showHoverDecoration && vm.showHoverMenu;
 
-    return (
+    const closeMenu = useCallback(() => {
+        // To avoid icon blinking when closing the menu, we delay the state update
+        // Also, let the focus move to the menu trigger before closing the menu
+        setTimeout(() => setIsMenuOpen(false), 10);
+    }, []);
+
+    const content = (
         <button
             ref={ref}
             className={classNames("mx_RoomListItemView", {
@@ -92,17 +99,7 @@ export const RoomListItemView = memo(function RoomListItemView({
                     {showHoverMenu ? (
                         <RoomListItemMenuView
                             room={room}
-                            setMenuOpen={(isOpen) => {
-                                if (isOpen) {
-                                    setIsMenuOpen(isOpen);
-                                } else {
-                                    // To avoid icon blinking when closing the menu, we delay the state update
-                                    setTimeout(() => setIsMenuOpen(isOpen), 0);
-                                    // After closing the menu, we need to set the focus back to the button
-                                    // 10ms because the focus moves to the body and we put back the focus on the button
-                                    setTimeout(() => buttonRef.current?.focus(), 10);
-                                }
-                            }}
+                            setMenuOpen={(isOpen) => (isOpen ? setIsMenuOpen(true) : closeMenu())}
                         />
                     ) : (
                         <>
@@ -120,6 +117,24 @@ export const RoomListItemView = memo(function RoomListItemView({
             </Flex>
         </button>
     );
+
+    if (!vm.showContextMenu) return content;
+
+    return (
+        <RoomListItemContextMenuView
+            room={room}
+            setMenuOpen={(isOpen) => {
+                if (isOpen) {
+                    // To avoid icon blinking when the context menu is re-opened
+                    setTimeout(() => setIsMenuOpen(true), 0);
+                } else {
+                    closeMenu();
+                }
+            }}
+        >
+            {content}
+        </RoomListItemContextMenuView>
+    );
 });
 
 /**
diff --git a/test/unit-tests/components/viewmodels/roomlist/RoomListItemViewModel-test.tsx b/test/unit-tests/components/viewmodels/roomlist/RoomListItemViewModel-test.tsx
@@ -73,6 +73,15 @@ describe("RoomListItemViewModel", () => {
         );
     });
 
+    it("should show context menu if user has access to options menu", async () => {
+        mocked(hasAccessToOptionsMenu).mockReturnValue(true);
+        const { result: vm } = renderHook(
+            () => useRoomListItemViewModel(room),
+            withClientContextRenderOptions(room.client),
+        );
+        expect(vm.current.showContextMenu).toBe(true);
+    });
+
     it("should show hover menu if user has access to options menu", async () => {
         mocked(hasAccessToOptionsMenu).mockReturnValue(true);
         const { result: vm } = renderHook(
diff --git a/test/unit-tests/components/views/rooms/RoomListPanel/RoomList-test.tsx b/test/unit-tests/components/views/rooms/RoomListPanel/RoomList-test.tsx
@@ -10,7 +10,7 @@ import { type MatrixClient } from "matrix-js-sdk/src/matrix";
 import { render } from "jest-matrix-react";
 import { fireEvent } from "@testing-library/dom";
 
-import { mkRoom, stubClient } from "../../../../../test-utils";
+import { mkRoom, stubClient, withClientContextRenderOptions } from "../../../../../test-utils";
 import { type RoomListViewState } from "../../../../../../src/components/viewmodels/roomlist/RoomListViewModel";
 import { RoomList } from "../../../../../../src/components/views/rooms/RoomListPanel/RoomList";
 import DMRoomMap from "../../../../../../src/utils/DMRoomMap";
@@ -44,7 +44,7 @@ describe("<RoomList />", () => {
     });
 
     it("should render a room list", () => {
-        const { asFragment } = render(<RoomList vm={vm} />);
+        const { asFragment } = render(<RoomList vm={vm} />, withClientContextRenderOptions(matrixClient));
         expect(asFragment()).toMatchSnapshot();
     });
 
@@ -53,7 +53,7 @@ describe("<RoomList />", () => {
         { shortcut: { key: "F6", ctrlKey: true }, isPreviousLandmark: false, label: "NextLandmark" },
     ])("should navigate to the landmark on NextLandmark.$label action", ({ shortcut, isPreviousLandmark }) => {
         const spyFindLandmark = jest.spyOn(LandmarkNavigation, "findAndFocusNextLandmark").mockReturnValue();
-        const { getByTestId } = render(<RoomList vm={vm} />);
+        const { getByTestId } = render(<RoomList vm={vm} />, withClientContextRenderOptions(matrixClient));
         const roomList = getByTestId("room-list");
         fireEvent.keyDown(roomList, shortcut);
 
diff --git a/test/unit-tests/components/views/rooms/RoomListPanel/RoomListItemView-test.tsx b/test/unit-tests/components/views/rooms/RoomListPanel/RoomListItemView-test.tsx
@@ -42,6 +42,7 @@ describe("<RoomListItemView />", () => {
 
         defaultValue = {
             openRoom: jest.fn(),
+            showContextMenu: false,
             showHoverMenu: false,
             notificationState,
             a11yLabel: "Open room room1",
@@ -136,4 +137,21 @@ describe("<RoomListItemView />", () => {
 
         expect(screen.queryByRole("notification-decoration")).toBeNull();
     });
+
+    test("should render the context menu", async () => {
+        const user = userEvent.setup();
+
+        mocked(useRoomListItemViewModel).mockReturnValue({
+            ...defaultValue,
+            showContextMenu: true,
+        });
+
+        render(<RoomListItemView room={room} isSelected={false} />, withClientContextRenderOptions(matrixClient));
+        const button = screen.getByRole("button", { name: `Open room ${room.name}` });
+        await user.pointer([{ target: button }, { keys: "[MouseRight]", target: button }]);
+        await waitFor(() => expect(screen.getByRole("menu")).toBeInTheDocument());
+        // Menu should close
+        await user.keyboard("{Escape}");
+        expect(screen.queryByRole("menu")).toBeNull();
+    });
 });
diff --git a/test/unit-tests/components/views/rooms/RoomListPanel/__snapshots__/RoomList-test.tsx.snap b/test/unit-tests/components/views/rooms/RoomListPanel/__snapshots__/RoomList-test.tsx.snap
diff --git a/yarn.lock b/yarn.lock
