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

* 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
This commit is contained in:
Florian Duros
2025-06-24 11:50:27 +02:00
committed by GitHub
parent 52f836a0dd
commit f707bb410e
11 changed files with 169 additions and 32 deletions

View File

@@ -93,7 +93,7 @@
"@types/png-chunks-extract": "^1.0.2", "@types/png-chunks-extract": "^1.0.2",
"@types/react-virtualized": "^9.21.30", "@types/react-virtualized": "^9.21.30",
"@vector-im/compound-design-tokens": "^4.0.0", "@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", "@vector-im/matrix-wysiwyg": "2.38.3",
"@zxcvbn-ts/core": "^3.0.4", "@zxcvbn-ts/core": "^3.0.4",
"@zxcvbn-ts/language-common": "^3.0.4", "@zxcvbn-ts/language-common": "^3.0.4",

View File

@@ -60,6 +60,12 @@ test.describe("Room list", () => {
await expect(page.getByRole("heading", { name: "room29", level: 1 })).toBeVisible(); 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 }) => { test("should open the more options menu", { tag: "@screenshot" }, async ({ page, app, user }) => {
const roomListView = getRoomList(page); const roomListView = getRoomList(page);
const roomItem = roomListView.getByRole("gridcell", { name: "Open room room29" }); const roomItem = roomListView.getByRole("gridcell", { name: "Open room room29" });
@@ -223,17 +229,17 @@ test.describe("Room list", () => {
await expect(notificationButton).toBeFocused(); await expect(notificationButton).toBeFocused();
// Open the menu // Open the menu
await notificationButton.click(); await page.keyboard.press("Enter");
// Wait for the menu to be open // Wait for the menu to be open
await expect(page.getByRole("menuitem", { name: "Match default settings" })).toHaveAttribute( await expect(page.getByRole("menuitem", { name: "Match default settings" })).toHaveAttribute(
"aria-selected", "aria-selected",
"true", "true",
); );
// Close the menu await page.keyboard.press("ArrowDown");
await page.keyboard.press("Escape"); await page.keyboard.press("Escape");
// Focus should be back on the room list item // Focus should be back on the notification button
await expect(room29).toBeFocused(); await expect(notificationButton).toBeFocused();
}); });
}); });
}); });

View File

@@ -30,6 +30,10 @@ export interface RoomListItemViewState {
* The name of the room. * The name of the room.
*/ */
name: string; name: string;
/**
* Whether the context menu should be shown.
*/
showContextMenu: boolean;
/** /**
* Whether the hover menu should be shown. * Whether the hover menu should be shown.
*/ */
@@ -105,12 +109,12 @@ export function useRoomListItemViewModel(room: Room): RoomListItemViewState {
setNotificationValues(getNotificationValues(notificationState)); setNotificationValues(getNotificationValues(notificationState));
}, [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 // - 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 = const showHoverMenu =
!invited && !invited && (showContextMenu || hasAccessToNotificationMenu(room, matrixClient.isGuest(), isArchived));
(hasAccessToOptionsMenu(room) || hasAccessToNotificationMenu(room, matrixClient.isGuest(), isArchived));
const messagePreview = useRoomMessagePreview(room); const messagePreview = useRoomMessagePreview(room);
@@ -137,6 +141,7 @@ export function useRoomListItemViewModel(room: Room): RoomListItemViewState {
return { return {
name, name,
notificationState, notificationState,
showContextMenu,
showHoverMenu, showHoverMenu,
openRoom, openRoom,
a11yLabel, a11yLabel,

View File

@@ -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>
);
}

View File

@@ -35,7 +35,6 @@ interface RoomListItemMenuViewProps {
room: Room; room: Room;
/** /**
* Set the menu open state. * Set the menu open state.
* @param isOpen
*/ */
setMenuOpen: (isOpen: boolean) => void; setMenuOpen: (isOpen: boolean) => void;
} }
@@ -84,6 +83,21 @@ function MoreOptionsMenu({ vm, setMenuOpen }: MoreOptionsMenuProps): JSX.Element
align="start" align="start"
trigger={<MoreOptionsButton size="24px" />} 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 && ( {vm.canMarkAsRead && (
<MenuItem <MenuItem
Icon={MarkAsReadIcon} Icon={MarkAsReadIcon}
@@ -143,7 +157,7 @@ function MoreOptionsMenu({ vm, setMenuOpen }: MoreOptionsMenuProps): JSX.Element
onClick={(evt) => evt.stopPropagation()} onClick={(evt) => evt.stopPropagation()}
hideChevron={true} hideChevron={true}
/> />
</Menu> </>
); );
} }
@@ -154,7 +168,7 @@ interface MoreOptionsButtonProps extends ComponentProps<typeof IconButton> {
/** /**
* A button to trigger the more options menu. * 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 ( return (
<Tooltip label={_t("room_list|room|more_options")}> <Tooltip label={_t("room_list|room|more_options")}>
<IconButton aria-label={_t("room_list|room|more_options")} {...props}> <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. * A button to trigger the notification menu.
*/ */
export const NotificationButton = function MoreOptionsButton({ const NotificationButton = function MoreOptionsButton({
isRoomMuted, isRoomMuted,
ref, ref,
...props ...props

View File

@@ -15,6 +15,7 @@ import { RoomListItemMenuView } from "./RoomListItemMenuView";
import { NotificationDecoration } from "../NotificationDecoration"; import { NotificationDecoration } from "../NotificationDecoration";
import { RoomAvatarView } from "../../avatars/RoomAvatarView"; import { RoomAvatarView } from "../../avatars/RoomAvatarView";
import { useRovingTabIndex } from "../../../../accessibility/RovingTabIndex"; import { useRovingTabIndex } from "../../../../accessibility/RovingTabIndex";
import { RoomListItemContextMenuView } from "./RoomListItemContextMenuView";
interface RoomListItemViewProps extends React.HTMLAttributes<HTMLButtonElement> { interface RoomListItemViewProps extends React.HTMLAttributes<HTMLButtonElement> {
/** /**
@@ -47,7 +48,13 @@ export const RoomListItemView = memo(function RoomListItemView({
const showHoverDecoration = isMenuOpen || isHover; const showHoverDecoration = isMenuOpen || isHover;
const showHoverMenu = showHoverDecoration && vm.showHoverMenu; 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 <button
ref={ref} ref={ref}
className={classNames("mx_RoomListItemView", { className={classNames("mx_RoomListItemView", {
@@ -92,17 +99,7 @@ export const RoomListItemView = memo(function RoomListItemView({
{showHoverMenu ? ( {showHoverMenu ? (
<RoomListItemMenuView <RoomListItemMenuView
room={room} room={room}
setMenuOpen={(isOpen) => { setMenuOpen={(isOpen) => (isOpen ? setIsMenuOpen(true) : closeMenu())}
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);
}
}}
/> />
) : ( ) : (
<> <>
@@ -120,6 +117,24 @@ export const RoomListItemView = memo(function RoomListItemView({
</Flex> </Flex>
</button> </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>
);
}); });
/** /**

View File

@@ -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 () => { it("should show hover menu if user has access to options menu", async () => {
mocked(hasAccessToOptionsMenu).mockReturnValue(true); mocked(hasAccessToOptionsMenu).mockReturnValue(true);
const { result: vm } = renderHook( const { result: vm } = renderHook(

View File

@@ -10,7 +10,7 @@ import { type MatrixClient } from "matrix-js-sdk/src/matrix";
import { render } from "jest-matrix-react"; import { render } from "jest-matrix-react";
import { fireEvent } from "@testing-library/dom"; 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 { type RoomListViewState } from "../../../../../../src/components/viewmodels/roomlist/RoomListViewModel";
import { RoomList } from "../../../../../../src/components/views/rooms/RoomListPanel/RoomList"; import { RoomList } from "../../../../../../src/components/views/rooms/RoomListPanel/RoomList";
import DMRoomMap from "../../../../../../src/utils/DMRoomMap"; import DMRoomMap from "../../../../../../src/utils/DMRoomMap";
@@ -44,7 +44,7 @@ describe("<RoomList />", () => {
}); });
it("should render a room list", () => { it("should render a room list", () => {
const { asFragment } = render(<RoomList vm={vm} />); const { asFragment } = render(<RoomList vm={vm} />, withClientContextRenderOptions(matrixClient));
expect(asFragment()).toMatchSnapshot(); expect(asFragment()).toMatchSnapshot();
}); });
@@ -53,7 +53,7 @@ describe("<RoomList />", () => {
{ shortcut: { key: "F6", ctrlKey: true }, isPreviousLandmark: false, label: "NextLandmark" }, { shortcut: { key: "F6", ctrlKey: true }, isPreviousLandmark: false, label: "NextLandmark" },
])("should navigate to the landmark on NextLandmark.$label action", ({ shortcut, isPreviousLandmark }) => { ])("should navigate to the landmark on NextLandmark.$label action", ({ shortcut, isPreviousLandmark }) => {
const spyFindLandmark = jest.spyOn(LandmarkNavigation, "findAndFocusNextLandmark").mockReturnValue(); 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"); const roomList = getByTestId("room-list");
fireEvent.keyDown(roomList, shortcut); fireEvent.keyDown(roomList, shortcut);

View File

@@ -42,6 +42,7 @@ describe("<RoomListItemView />", () => {
defaultValue = { defaultValue = {
openRoom: jest.fn(), openRoom: jest.fn(),
showContextMenu: false,
showHoverMenu: false, showHoverMenu: false,
notificationState, notificationState,
a11yLabel: "Open room room1", a11yLabel: "Open room room1",
@@ -136,4 +137,21 @@ describe("<RoomListItemView />", () => {
expect(screen.queryByRole("notification-decoration")).toBeNull(); 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();
});
}); });

View File

@@ -23,9 +23,11 @@ exports[`<RoomList /> should render a room list 1`] = `
style="width: auto; height: 480px; max-width: 1500px; max-height: 480px; overflow: hidden; position: relative;" style="width: auto; height: 480px; max-width: 1500px; max-height: 480px; overflow: hidden; position: relative;"
> >
<button <button
aria-haspopup="menu"
aria-label="Open room room0" aria-label="Open room room0"
aria-selected="false" aria-selected="false"
class="mx_RoomListItemView" class="mx_RoomListItemView"
data-state="closed"
role="gridcell" role="gridcell"
style="height: 48px; left: 0px; position: absolute; top: 0px; width: 100%;" style="height: 48px; left: 0px; position: absolute; top: 0px; width: 100%;"
tabindex="0" tabindex="0"
@@ -75,9 +77,11 @@ exports[`<RoomList /> should render a room list 1`] = `
</div> </div>
</button> </button>
<button <button
aria-haspopup="menu"
aria-label="Open room room1" aria-label="Open room room1"
aria-selected="false" aria-selected="false"
class="mx_RoomListItemView" class="mx_RoomListItemView"
data-state="closed"
role="gridcell" role="gridcell"
style="height: 48px; left: 0px; position: absolute; top: 48px; width: 100%;" style="height: 48px; left: 0px; position: absolute; top: 48px; width: 100%;"
tabindex="-1" tabindex="-1"
@@ -127,9 +131,11 @@ exports[`<RoomList /> should render a room list 1`] = `
</div> </div>
</button> </button>
<button <button
aria-haspopup="menu"
aria-label="Open room room2" aria-label="Open room room2"
aria-selected="false" aria-selected="false"
class="mx_RoomListItemView" class="mx_RoomListItemView"
data-state="closed"
role="gridcell" role="gridcell"
style="height: 48px; left: 0px; position: absolute; top: 96px; width: 100%;" style="height: 48px; left: 0px; position: absolute; top: 96px; width: 100%;"
tabindex="-1" tabindex="-1"
@@ -179,9 +185,11 @@ exports[`<RoomList /> should render a room list 1`] = `
</div> </div>
</button> </button>
<button <button
aria-haspopup="menu"
aria-label="Open room room3" aria-label="Open room room3"
aria-selected="false" aria-selected="false"
class="mx_RoomListItemView" class="mx_RoomListItemView"
data-state="closed"
role="gridcell" role="gridcell"
style="height: 48px; left: 0px; position: absolute; top: 144px; width: 100%;" style="height: 48px; left: 0px; position: absolute; top: 144px; width: 100%;"
tabindex="-1" tabindex="-1"
@@ -231,9 +239,11 @@ exports[`<RoomList /> should render a room list 1`] = `
</div> </div>
</button> </button>
<button <button
aria-haspopup="menu"
aria-label="Open room room4" aria-label="Open room room4"
aria-selected="false" aria-selected="false"
class="mx_RoomListItemView" class="mx_RoomListItemView"
data-state="closed"
role="gridcell" role="gridcell"
style="height: 48px; left: 0px; position: absolute; top: 192px; width: 100%;" style="height: 48px; left: 0px; position: absolute; top: 192px; width: 100%;"
tabindex="-1" tabindex="-1"
@@ -283,9 +293,11 @@ exports[`<RoomList /> should render a room list 1`] = `
</div> </div>
</button> </button>
<button <button
aria-haspopup="menu"
aria-label="Open room room5" aria-label="Open room room5"
aria-selected="false" aria-selected="false"
class="mx_RoomListItemView" class="mx_RoomListItemView"
data-state="closed"
role="gridcell" role="gridcell"
style="height: 48px; left: 0px; position: absolute; top: 240px; width: 100%;" style="height: 48px; left: 0px; position: absolute; top: 240px; width: 100%;"
tabindex="-1" tabindex="-1"
@@ -335,9 +347,11 @@ exports[`<RoomList /> should render a room list 1`] = `
</div> </div>
</button> </button>
<button <button
aria-haspopup="menu"
aria-label="Open room room6" aria-label="Open room room6"
aria-selected="false" aria-selected="false"
class="mx_RoomListItemView" class="mx_RoomListItemView"
data-state="closed"
role="gridcell" role="gridcell"
style="height: 48px; left: 0px; position: absolute; top: 288px; width: 100%;" style="height: 48px; left: 0px; position: absolute; top: 288px; width: 100%;"
tabindex="-1" tabindex="-1"
@@ -387,9 +401,11 @@ exports[`<RoomList /> should render a room list 1`] = `
</div> </div>
</button> </button>
<button <button
aria-haspopup="menu"
aria-label="Open room room7" aria-label="Open room room7"
aria-selected="false" aria-selected="false"
class="mx_RoomListItemView" class="mx_RoomListItemView"
data-state="closed"
role="gridcell" role="gridcell"
style="height: 48px; left: 0px; position: absolute; top: 336px; width: 100%;" style="height: 48px; left: 0px; position: absolute; top: 336px; width: 100%;"
tabindex="-1" tabindex="-1"
@@ -439,9 +455,11 @@ exports[`<RoomList /> should render a room list 1`] = `
</div> </div>
</button> </button>
<button <button
aria-haspopup="menu"
aria-label="Open room room8" aria-label="Open room room8"
aria-selected="false" aria-selected="false"
class="mx_RoomListItemView" class="mx_RoomListItemView"
data-state="closed"
role="gridcell" role="gridcell"
style="height: 48px; left: 0px; position: absolute; top: 384px; width: 100%;" style="height: 48px; left: 0px; position: absolute; top: 384px; width: 100%;"
tabindex="-1" tabindex="-1"
@@ -491,9 +509,11 @@ exports[`<RoomList /> should render a room list 1`] = `
</div> </div>
</button> </button>
<button <button
aria-haspopup="menu"
aria-label="Open room room9" aria-label="Open room room9"
aria-selected="false" aria-selected="false"
class="mx_RoomListItemView" class="mx_RoomListItemView"
data-state="closed"
role="gridcell" role="gridcell"
style="height: 48px; left: 0px; position: absolute; top: 432px; width: 100%;" style="height: 48px; left: 0px; position: absolute; top: 432px; width: 100%;"
tabindex="-1" tabindex="-1"

View File

@@ -3844,10 +3844,10 @@
resolved "https://registry.yarnpkg.com/@vector-im/compound-design-tokens/-/compound-design-tokens-4.0.2.tgz#27363d26446eaa21880ab126fa51fec112e6fd86" resolved "https://registry.yarnpkg.com/@vector-im/compound-design-tokens/-/compound-design-tokens-4.0.2.tgz#27363d26446eaa21880ab126fa51fec112e6fd86"
integrity sha512-y13bhPyJ5OzbGRl21F6+Y2adrjyK+mu67yKTx+o8MfmIpJzMSn4KkHZtcujMquWSh0e5ZAufsnk4VYvxbSpr1A== integrity sha512-y13bhPyJ5OzbGRl21F6+Y2adrjyK+mu67yKTx+o8MfmIpJzMSn4KkHZtcujMquWSh0e5ZAufsnk4VYvxbSpr1A==
"@vector-im/compound-web@^8.0.0": "@vector-im/compound-web@^8.1.2":
version "8.0.0" version "8.1.2"
resolved "https://registry.yarnpkg.com/@vector-im/compound-web/-/compound-web-8.0.0.tgz#1eeaf54c253730752393d0e5e7c8cd81030b80af" resolved "https://registry.yarnpkg.com/@vector-im/compound-web/-/compound-web-8.1.2.tgz#a8af8681b442e63e07b8f25204528b1b022c1db2"
integrity sha512-VAwCGl0KMjN+qEKflnQOB1LidSsxSDiczEWka1IGKj52EzrNOfY0wlfCs73v+H84zUanYKlgOwnQgWU5at9Q/w== integrity sha512-F9UyQBwRThwju+STz84iJy6JGWQ7UIxaprstfsGpiyS/3ror4E6m/mfwbrNjT0l3fhrhk6sRiTAMlcBRzYgdMQ==
dependencies: dependencies:
"@floating-ui/react" "^0.27.0" "@floating-ui/react" "^0.27.0"
"@radix-ui/react-context-menu" "^2.2.1" "@radix-ui/react-context-menu" "^2.2.1"