Skip to content

Room list: display the menu option on the room list item when clicked/opened #31380

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
florianduros merged 3 commits into from
Dec 2, 2025
Merged

Room list: display the menu option on the room list item when clicked/opened #31380

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a7d5e5f
fix: display the menu option on the room list item when clicked/opened
florianduros Dec 1, 2025
35082c4
test: update room list e2e tests
florianduros Dec 1, 2025
8c38fe2
test: update marked as unread state
florianduros Dec 1, 2025
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
15 changes: 6 additions & 9 deletions playwright/e2e/left-panel/room-list-panel/room-list.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,18 +299,16 @@ test.describe("Room list", () => {
const publicRoom = roomListView.getByRole("option", { name: "low priority room" });

// Make room low priority
await publicRoom.hover();
const roomItemMenu = publicRoom.getByRole("button", { name: "More Options" });
await roomItemMenu.click();
await publicRoom.click({ button: "right" });
await page.getByRole("menuitemcheckbox", { name: "Low priority" }).click();

// Should have low priority decoration
await expect(publicRoom.locator(".mx_RoomAvatarView_icon")).toHaveAccessibleName(
"This is a low priority room",
);

// focus the user menu to avoid to have hover decoration
await page.getByRole("button", { name: "User menu" }).focus();
// focus the header to avoid to have hover decoration
await page.getByTestId("room-list-header").click();
await expect(publicRoom).toMatchScreenshot("room-list-item-low-priority.png");
});

Expand Down Expand Up @@ -450,12 +448,11 @@ test.describe("Room list", () => {
await bot.joinRoom(roomId);

const room = roomListView.getByRole("option", { name: "mark as unread" });
await room.hover();
await room.getByRole("button", { name: "More Options" }).click();
await room.click({ button: "right" });
await page.getByRole("menuitem", { name: "mark as unread" }).click();

// focus the user menu to avoid to have hover decoration
await page.getByRole("button", { name: "User menu" }).focus();
// focus the header to avoid to have hover decoration
await page.getByTestId("room-list-header").click();

await expect(room).toMatchScreenshot("room-list-item-mark-as-unread.png");
});
Expand Down
7 changes: 3 additions & 4 deletions playwright/e2e/room_options/marked_unread.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,11 @@ test.describe("Mark as Unread", () => {
await page.goto("/#/room/" + dummyRoomId);

const roomTile = page.getByLabel(TEST_ROOM_NAME);
await roomTile.focus();
await roomTile.getByRole("button", { name: "More Options" }).click();
await roomTile.click({ button: "right" });
await page.getByRole("menuitem", { name: "Mark as unread" }).click();

// focus the user menu to avoid to have hover decoration
await page.getByRole("button", { name: "User menu" }).focus();
// focus another room to make the notification decoration appear (room options are display on hover)
await page.getByRole("option", { name: "Open room Room of no consequence" }).click();

await expect(roomTile.getByTestId("notification-decoration")).toBeVisible();
});
Expand Down
Binary file modified ...t-panel/room-list-panel/room-list.spec.ts/room-list-item-low-priority-linux.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 0 additions & 1 deletion src/components/views/rooms/RoomListPanel/RoomListItemView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ export const RoomListItemView = memo(function RoomListItemView({
onFocus={(e: React.FocusEvent<HTMLButtonElement>) => onFocus(room, e)}
onMouseOver={() => setHover(true)}
onMouseOut={() => setHover(false)}
onBlur={() => setHover(false)}
Comment on lines 98 to -101
Copy link
Copy Markdown
Member

@t3chguy t3chguy Dec 1, 2025

Choose a reason for hiding this comment

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

Won't this break keyboard accessibility? Navigating by keyboard seemingly will fire onFocus but never onBlur so will "hover" but never "unhover"

Copy link
Copy Markdown
Member Author

@florianduros florianduros Dec 1, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

No because in case of keyboard navigation, we rely on the isFocused props to display this menu. The react virtuoso wrapper tracks the current focus item (see the onFocus props in RoomListItemView). When the focus change, RoomListItemView#isFocused=false and the menu is not displayed anymore.

nb: the updated screenshots are incorrects, i have to update the tests too, to move the focus out of the item

tabIndex={isFocused ? 0 : -1}
{...props}
>
Expand Down
Loading