Skip to content

New room list: add keyboard navigation support #29805

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.

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
merged 9 commits into from
May 6, 2025
68 changes: 59 additions & 9 deletions playwright/e2e/left-panel/room-list-panel/room-list.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ test.describe("Room list", () => {
await expect(roomListView.getByRole("gridcell", { name: "Open room room29" })).toBeVisible();
await expect(roomListView).toMatchScreenshot("room-list.png");

await roomListView.hover();
// Put focus on the room list
await roomListView.getByRole("gridcell", { name: "Open room room29" }).click();
// Scroll to the end of the room list
await page.mouse.wheel(0, 1000);
await expect(roomListView.getByRole("gridcell", { name: "Open room room0" })).toBeVisible();
Expand Down Expand Up @@ -105,13 +106,10 @@ test.describe("Room list", () => {
// It should make the room muted
await page.getByRole("menuitem", { name: "Mute room" }).click();

// Remove hover on the room list item
await roomListView.hover();

// Scroll to the bottom of the list
await page.getByRole("grid", { name: "Room list" }).evaluate((e) => {
e.scrollTop = e.scrollHeight;
});
// Put focus on the room list
await roomListView.getByRole("gridcell", { name: "Open room room28" }).click();
// Scroll to the end of the room list
await page.mouse.wheel(0, 1000);

// The room decoration should have the muted icon
await expect(roomItem.getByTestId("notification-decoration")).toBeVisible();
Expand All @@ -129,7 +127,8 @@ test.describe("Room list", () => {

test("should scroll to the current room", async ({ page, app, user }) => {
const roomListView = getRoomList(page);
await roomListView.hover();
// Put focus on the room list
await roomListView.getByRole("gridcell", { name: "Open room room29" }).click();
// Scroll to the end of the room list
await page.mouse.wheel(0, 1000);

Expand Down Expand Up @@ -183,6 +182,57 @@ test.describe("Room list", () => {
await expect(page.getByRole("heading", { name: "1 notification", level: 1 })).toBeVisible();
});
});

test.describe("Keyboard navigation", () => {
test("should navigate to the room list", async ({ page, app, user }) => {
const roomListView = getRoomList(page);

const room29 = roomListView.getByRole("gridcell", { name: "Open room room29" });
const room28 = roomListView.getByRole("gridcell", { name: "Open room room28" });

// open the room
await room29.click();
// put focus back on the room list item
await room29.click();
await expect(room29).toBeFocused();

await page.keyboard.press("ArrowDown");
await expect(room28).toBeFocused();
await expect(room29).not.toBeFocused();

await page.keyboard.press("ArrowUp");
await expect(room29).toBeFocused();
await expect(room28).not.toBeFocused();
});

test("should navigate to the notification menu", async ({ page, app, user }) => {
const roomListView = getRoomList(page);
const room29 = roomListView.getByRole("gridcell", { name: "Open room room29" });
const moreButton = room29.getByRole("button", { name: "More options" });
const notificationButton = room29.getByRole("button", { name: "Notification options" });

await room29.click();
// put focus back on the room list item
await room29.click();
await page.keyboard.press("Tab");
await expect(moreButton).toBeFocused();
await page.keyboard.press("Tab");
await expect(notificationButton).toBeFocused();

// Open the menu
await notificationButton.click();
// 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("Escape");
// Focus should be back on the room list item
await expect(room29).toBeFocused();
});
});
});

test.describe("Avatar decoration", () => {
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
12 changes: 4 additions & 8 deletions res/css/views/rooms/RoomListPanel/_RoomListItemView.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,6 @@
all: unset;
cursor: pointer;

&:hover {
background-color: var(--cpd-color-bg-action-secondary-hovered);
}

.mx_RoomListItemView_container {
padding-left: var(--cpd-space-2x);
font: var(--cpd-font-body-md-regular);
Expand Down Expand Up @@ -56,12 +52,12 @@
}
}

.mx_RoomListItemView_menu_open {
.mx_RoomListItemView_hover {
background-color: var(--cpd-color-bg-action-secondary-hovered);
}

.mx_RoomListItemView_content {
padding-right: var(--cpd-space-1-5x);
}
.mx_RoomListItemView_menu_open .mx_RoomListItemView_content {
padding-right: var(--cpd-space-1-5x);
}

.mx_RoomListItemView_selected {
Expand Down
60 changes: 44 additions & 16 deletions src/components/views/rooms/RoomListPanel/RoomList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ import { AutoSizer, List, type ListRowProps } from "react-virtualized";
import { type RoomListViewState } from "../../../viewmodels/roomlist/RoomListViewModel";
import { _t } from "../../../../languageHandler";
import { RoomListItemView } from "./RoomListItemView";
import { RovingTabIndexProvider } from "../../../../accessibility/RovingTabIndex";
import { getKeyBindingsManager } from "../../../../KeyBindingsManager";
import { KeyBindingAction } from "../../../../accessibility/KeyboardShortcuts";
import { Landmark, LandmarkNavigation } from "../../../../accessibility/LandmarkNavigation";

interface RoomListProps {
/**
Expand All @@ -32,21 +36,45 @@ export function RoomList({ vm: { rooms, activeIndex } }: RoomListProps): JSX.Ele

// The first div is needed to make the virtualized list take all the remaining space and scroll correctly
return (
<div className="mx_RoomList" data-testid="room-list">
<AutoSizer>
{({ height, width }) => (
<List
aria-label={_t("room_list|list_title")}
className="mx_RoomList_List"
rowRenderer={roomRendererMemoized}
rowCount={rooms.length}
rowHeight={48}
height={height}
width={width}
scrollToIndex={activeIndex ?? 0}
/>
)}
</AutoSizer>
</div>
<RovingTabIndexProvider handleHomeEnd={true} handleUpDown={true}>
{({ onKeyDownHandler }) => (
<div
className="mx_RoomList"
data-testid="room-list"
onKeyDown={(ev) => {
const navAction = getKeyBindingsManager().getNavigationAction(ev);
if (
navAction === KeyBindingAction.NextLandmark ||
navAction === KeyBindingAction.PreviousLandmark
) {
LandmarkNavigation.findAndFocusNextLandmark(
Landmark.ROOM_LIST,
navAction === KeyBindingAction.PreviousLandmark,
);
ev.stopPropagation();
ev.preventDefault();
return;
}
onKeyDownHandler(ev);
}}
>
<AutoSizer>
{({ height, width }) => (
<List
aria-label={_t("room_list|list_title")}
className="mx_RoomList_List"
rowRenderer={roomRendererMemoized}
rowCount={rooms.length}
rowHeight={48}
height={height}
width={width}
scrollToIndex={activeIndex ?? 0}
tabIndex={-1}
/>
)}
</AutoSizer>
</div>
)}
</RovingTabIndexProvider>
);
}
73 changes: 62 additions & 11 deletions src/components/views/rooms/RoomListPanel/RoomListItemView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* Please see LICENSE files in the repository root for full details.
*/

import React, { type JSX, memo, useState } from "react";
import React, { type JSX, memo, useCallback, useRef, useState } from "react";
import { type Room } from "matrix-js-sdk/src/matrix";
import classNames from "classnames";

Expand All @@ -14,6 +14,7 @@ import { Flex } from "../../../utils/Flex";
import { RoomListItemMenuView } from "./RoomListItemMenuView";
import { NotificationDecoration } from "../NotificationDecoration";
import { RoomAvatarView } from "../../avatars/RoomAvatarView";
import { useRovingTabIndex } from "../../../../accessibility/RovingTabIndex";

interface RoomListItemViewProps extends React.HTMLAttributes<HTMLButtonElement> {
/**
Expand All @@ -34,22 +35,29 @@ export const RoomListItemView = memo(function RoomListItemView({
isSelected,
...props
}: RoomListItemViewProps): JSX.Element {
const buttonRef = useRef<HTMLButtonElement>(null);
const [onFocus, isActive, ref] = useRovingTabIndex(buttonRef);

const vm = useRoomListItemViewModel(room);

const [isHover, setIsHover] = useState(false);
const [isHover, setIsHover] = useIsHover();
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const [isHover, setIsHover] = useIsHover();
const [isHover, setIsHoverWithDelay] = useIsHover();

to match maybe?

const [isMenuOpen, setIsMenuOpen] = useState(false);
// The compound menu in RoomListItemMenuView needs to be rendered when the hover menu is shown
// Using display: none; and then display:flex when hovered in CSS causes the menu to be misaligned
const showHoverDecoration = (isMenuOpen || isHover) && vm.showHoverMenu;
const showHoverDecoration = isMenuOpen || isHover;
const showHoverMenu = showHoverDecoration && vm.showHoverMenu;

const isNotificationDecorationVisible = !showHoverDecoration && vm.showNotificationDecoration;
const isInvitation = vm.notificationState.invited;
const isNotificationDecorationVisible = isInvitation || (!showHoverDecoration && vm.showNotificationDecoration);

return (
<button
ref={ref}
className={classNames("mx_RoomListItemView", {
mx_RoomListItemView_empty: !isNotificationDecorationVisible && !showHoverDecoration,
mx_RoomListItemView_notification_decoration: isNotificationDecorationVisible,
mx_RoomListItemView_menu_open: showHoverDecoration,
mx_RoomListItemView_hover: showHoverDecoration,
mx_RoomListItemView_menu_open: showHoverMenu,
mx_RoomListItemView_selected: isSelected,
mx_RoomListItemView_bold: vm.isBold,
})}
Expand All @@ -59,8 +67,15 @@ export const RoomListItemView = memo(function RoomListItemView({
onClick={() => vm.openRoom()}
onMouseOver={() => setIsHover(true)}
onMouseOut={() => setIsHover(false)}
onFocus={() => setIsHover(true)}
onBlur={() => setIsHover(false)}
onFocus={() => {
setIsHover(true);
onFocus();
}}
// Adding a timeout because when tabbing to go to the more options and notification menu, the focus moves out of the button
// The blur makes the button lose the hover state and these menu are not shown
// We delay the blur event to give time to the focus to move to the menu
onBlur={() => setIsHover(false, 10)}
tabIndex={isActive ? 0 : -1}
{...props}
>
{/* We need this extra div between the button and the content in order to add a padding which is not messing with the virtualized list */}
Expand All @@ -79,13 +94,19 @@ export const RoomListItemView = memo(function RoomListItemView({
</div>
<div className="mx_RoomListItemView_messagePreview">{vm.messagePreview}</div>
</div>
{showHoverDecoration ? (
{showHoverMenu ? (
<RoomListItemMenuView
room={room}
setMenuOpen={(isOpen) => {
if (isOpen) setIsMenuOpen(isOpen);
// To avoid icon blinking when closing the menu, we delay the state update
else setTimeout(() => setIsMenuOpen(isOpen), 0);
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);
}
}}
/>
) : (
Expand All @@ -105,3 +126,33 @@ export const RoomListItemView = memo(function RoomListItemView({
</button>
);
});

/**
* Custom hook to manage the hover state of the room list item
* If the timeout is set, it will set the hover state after the timeout
* If the timeout is not set, it will set the hover state immediately
* When the set method is called, it will clear any existing timeout
*
* @returns {boolean} isHover - The hover state
*/
function useIsHover(): [boolean, (value: boolean, timeout?: number) => void] {
const [isHover, setIsHover] = useState(false);
// Store the timeout ID
const timeoutRef = useRef<number | undefined>(undefined);

const setIsHoverWithDelay = useCallback((value: boolean, timeout?: number): void => {
// Clear the timeout if it exists
clearTimeout(timeoutRef.current);

// No delay, set the value immediately
if (timeout === undefined) {
setIsHover(value);
return;
}

// Set a timeout to set the value after the delay
timeoutRef.current = setTimeout(() => setIsHover(value), timeout);
}, []);

return [isHover, setIsHoverWithDelay];
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,15 @@
import React from "react";
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 { type RoomListViewState } from "../../../../../../src/components/viewmodels/roomlist/RoomListViewModel";
import { RoomList } from "../../../../../../src/components/views/rooms/RoomListPanel/RoomList";
import DMRoomMap from "../../../../../../src/utils/DMRoomMap";
import { SecondaryFilters } from "../../../../../../src/components/viewmodels/roomlist/useFilteredRooms";
import { SortOption } from "../../../../../../src/components/viewmodels/roomlist/useSorter";
import { Landmark, LandmarkNavigation } from "../../../../../../src/accessibility/LandmarkNavigation";

describe("<RoomList />", () => {
let matrixClient: MatrixClient;
Expand Down Expand Up @@ -53,4 +55,16 @@ describe("<RoomList />", () => {
const { asFragment } = render(<RoomList vm={vm} />);
expect(asFragment()).toMatchSnapshot();
});

it.each([
{ shortcut: { key: "F6", ctrlKey: true, shiftKey: true }, isPreviousLandmark: true, label: "PreviousLandmark" },
{ 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 roomList = getByTestId("room-list");
fireEvent.keyDown(roomList, shortcut);

expect(spyFindLandmark).toHaveBeenCalledWith(Landmark.ROOM_LIST, isPreviousLandmark);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,17 @@ describe("<RoomListItemView />", () => {
await waitFor(() => expect(screen.getByRole("button", { name: "More Options" })).toBeInTheDocument());
});

test("should hover decoration if focused", async () => {
const user = userEvent.setup();
render(<RoomListItemView room={room} isSelected={false} />, withClientContextRenderOptions(matrixClient));
const listItem = screen.getByRole("button", { name: `Open room ${room.name}` });
await user.click(listItem);
expect(listItem).toHaveClass("mx_RoomListItemView_hover");

await user.tab();
await waitFor(() => expect(listItem).not.toHaveClass("mx_RoomListItemView_hover"));
});

test("should be selected if isSelected=true", async () => {
const { asFragment } = render(<RoomListItemView room={room} isSelected={true} />);
expect(screen.queryByRole("button", { name: `Open room ${room.name}` })).toHaveAttribute(
Expand Down
Loading
Loading