Skip to content

Commit 7927637

Browse files
floriandurossnowping
florianduros
authored and
snowping
committed
New room list: add keyboard navigation support (element-hq#29805)
* feat: support up/down arrow navigation in the new room list * feat: support tabbing in the new room list * test: update snapshots * test(e2e): fix room list test * test(new room list): add landmark navigation test * test(e2e): update screenshot test * test: add test to `RoomListItemView` * test(e2e): add keyboard navigation tests * refactor: rename `setIsHover` on `setIsHoverWithDelay`
1 parent c091d8b commit 7927637

File tree

9 files changed

+211
-47
lines changed

9 files changed

+211
-47
lines changed

‎playwright/e2e/left-panel/room-list-panel/room-list.spec.ts

Lines changed: 59 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@ test.describe("Room list", () => {
4343
await expect(roomListView.getByRole("gridcell", { name: "Open room room29" })).toBeVisible();
4444
await expect(roomListView).toMatchScreenshot("room-list.png");
4545

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

108-
// Remove hover on the room list item
109-
await roomListView.hover();
110-
111-
// Scroll to the bottom of the list
112-
await page.getByRole("grid", { name: "Room list" }).evaluate((e) => {
113-
e.scrollTop = e.scrollHeight;
114-
});
109+
// Put focus on the room list
110+
await roomListView.getByRole("gridcell", { name: "Open room room28" }).click();
111+
// Scroll to the end of the room list
112+
await page.mouse.wheel(0, 1000);
115113

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

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

@@ -183,6 +182,57 @@ test.describe("Room list", () => {
183182
await expect(page.getByRole("heading", { name: "1 notification", level: 1 })).toBeVisible();
184183
});
185184
});
185+
186+
test.describe("Keyboard navigation", () => {
187+
test("should navigate to the room list", async ({ page, app, user }) => {
188+
const roomListView = getRoomList(page);
189+
190+
const room29 = roomListView.getByRole("gridcell", { name: "Open room room29" });
191+
const room28 = roomListView.getByRole("gridcell", { name: "Open room room28" });
192+
193+
// open the room
194+
await room29.click();
195+
// put focus back on the room list item
196+
await room29.click();
197+
await expect(room29).toBeFocused();
198+
199+
await page.keyboard.press("ArrowDown");
200+
await expect(room28).toBeFocused();
201+
await expect(room29).not.toBeFocused();
202+
203+
await page.keyboard.press("ArrowUp");
204+
await expect(room29).toBeFocused();
205+
await expect(room28).not.toBeFocused();
206+
});
207+
208+
test("should navigate to the notification menu", async ({ page, app, user }) => {
209+
const roomListView = getRoomList(page);
210+
const room29 = roomListView.getByRole("gridcell", { name: "Open room room29" });
211+
const moreButton = room29.getByRole("button", { name: "More options" });
212+
const notificationButton = room29.getByRole("button", { name: "Notification options" });
213+
214+
await room29.click();
215+
// put focus back on the room list item
216+
await room29.click();
217+
await page.keyboard.press("Tab");
218+
await expect(moreButton).toBeFocused();
219+
await page.keyboard.press("Tab");
220+
await expect(notificationButton).toBeFocused();
221+
222+
// Open the menu
223+
await notificationButton.click();
224+
// Wait for the menu to be open
225+
await expect(page.getByRole("menuitem", { name: "Match default settings" })).toHaveAttribute(
226+
"aria-selected",
227+
"true",
228+
);
229+
230+
// Close the menu
231+
await page.keyboard.press("Escape");
232+
// Focus should be back on the room list item
233+
await expect(room29).toBeFocused();
234+
});
235+
});
186236
});
187237

188238
test.describe("Avatar decoration", () => {

‎playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-open-notification-options-selection-linux.png

18.5 KB
Loading

‎res/css/views/rooms/RoomListPanel/_RoomListItemView.pcss

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,6 @@
1818
all: unset;
1919
cursor: pointer;
2020

21-
&:hover {
22-
background-color: var(--cpd-color-bg-action-secondary-hovered);
23-
}
24-
2521
.mx_RoomListItemView_container {
2622
padding-left: var(--cpd-space-2x);
2723
font: var(--cpd-font-body-md-regular);
@@ -56,12 +52,12 @@
5652
}
5753
}
5854

59-
.mx_RoomListItemView_menu_open {
55+
.mx_RoomListItemView_hover {
6056
background-color: var(--cpd-color-bg-action-secondary-hovered);
57+
}
6158

62-
.mx_RoomListItemView_content {
63-
padding-right: var(--cpd-space-1-5x);
64-
}
59+
.mx_RoomListItemView_menu_open .mx_RoomListItemView_content {
60+
padding-right: var(--cpd-space-1-5x);
6561
}
6662

6763
.mx_RoomListItemView_selected {

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

Lines changed: 44 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@ import { AutoSizer, List, type ListRowProps } from "react-virtualized";
1111
import { type RoomListViewState } from "../../../viewmodels/roomlist/RoomListViewModel";
1212
import { _t } from "../../../../languageHandler";
1313
import { RoomListItemView } from "./RoomListItemView";
14+
import { RovingTabIndexProvider } from "../../../../accessibility/RovingTabIndex";
15+
import { getKeyBindingsManager } from "../../../../KeyBindingsManager";
16+
import { KeyBindingAction } from "../../../../accessibility/KeyboardShortcuts";
17+
import { Landmark, LandmarkNavigation } from "../../../../accessibility/LandmarkNavigation";
1418

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

3337
// The first div is needed to make the virtualized list take all the remaining space and scroll correctly
3438
return (
35-
<div className="mx_RoomList" data-testid="room-list">
36-
<AutoSizer>
37-
{({ height, width }) => (
38-
<List
39-
aria-label={_t("room_list|list_title")}
40-
className="mx_RoomList_List"
41-
rowRenderer={roomRendererMemoized}
42-
rowCount={rooms.length}
43-
rowHeight={48}
44-
height={height}
45-
width={width}
46-
scrollToIndex={activeIndex ?? 0}
47-
/>
48-
)}
49-
</AutoSizer>
50-
</div>
39+
<RovingTabIndexProvider handleHomeEnd={true} handleUpDown={true}>
40+
{({ onKeyDownHandler }) => (
41+
<div
42+
className="mx_RoomList"
43+
data-testid="room-list"
44+
onKeyDown={(ev) => {
45+
const navAction = getKeyBindingsManager().getNavigationAction(ev);
46+
if (
47+
navAction === KeyBindingAction.NextLandmark ||
48+
navAction === KeyBindingAction.PreviousLandmark
49+
) {
50+
LandmarkNavigation.findAndFocusNextLandmark(
51+
Landmark.ROOM_LIST,
52+
navAction === KeyBindingAction.PreviousLandmark,
53+
);
54+
ev.stopPropagation();
55+
ev.preventDefault();
56+
return;
57+
}
58+
onKeyDownHandler(ev);
59+
}}
60+
>
61+
<AutoSizer>
62+
{({ height, width }) => (
63+
<List
64+
aria-label={_t("room_list|list_title")}
65+
className="mx_RoomList_List"
66+
rowRenderer={roomRendererMemoized}
67+
rowCount={rooms.length}
68+
rowHeight={48}
69+
height={height}
70+
width={width}
71+
scrollToIndex={activeIndex ?? 0}
72+
tabIndex={-1}
73+
/>
74+
)}
75+
</AutoSizer>
76+
</div>
77+
)}
78+
</RovingTabIndexProvider>
5179
);
5280
}

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

Lines changed: 64 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
* Please see LICENSE files in the repository root for full details.
66
*/
77

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

@@ -14,6 +14,7 @@ import { Flex } from "../../../utils/Flex";
1414
import { RoomListItemMenuView } from "./RoomListItemMenuView";
1515
import { NotificationDecoration } from "../NotificationDecoration";
1616
import { RoomAvatarView } from "../../avatars/RoomAvatarView";
17+
import { useRovingTabIndex } from "../../../../accessibility/RovingTabIndex";
1718

1819
interface RoomListItemViewProps extends React.HTMLAttributes<HTMLButtonElement> {
1920
/**
@@ -34,33 +35,47 @@ export const RoomListItemView = memo(function RoomListItemView({
3435
isSelected,
3536
...props
3637
}: RoomListItemViewProps): JSX.Element {
38+
const buttonRef = useRef<HTMLButtonElement>(null);
39+
const [onFocus, isActive, ref] = useRovingTabIndex(buttonRef);
40+
3741
const vm = useRoomListItemViewModel(room);
3842

39-
const [isHover, setIsHover] = useState(false);
43+
const [isHover, setIsHoverWithDelay] = useIsHover();
4044
const [isMenuOpen, setIsMenuOpen] = useState(false);
4145
// The compound menu in RoomListItemMenuView needs to be rendered when the hover menu is shown
4246
// Using display: none; and then display:flex when hovered in CSS causes the menu to be misaligned
43-
const showHoverDecoration = (isMenuOpen || isHover) && vm.showHoverMenu;
47+
const showHoverDecoration = isMenuOpen || isHover;
48+
const showHoverMenu = showHoverDecoration && vm.showHoverMenu;
4449

45-
const isNotificationDecorationVisible = !showHoverDecoration && vm.showNotificationDecoration;
50+
const isInvitation = vm.notificationState.invited;
51+
const isNotificationDecorationVisible = isInvitation || (!showHoverDecoration && vm.showNotificationDecoration);
4652

4753
return (
4854
<button
55+
ref={ref}
4956
className={classNames("mx_RoomListItemView", {
5057
mx_RoomListItemView_empty: !isNotificationDecorationVisible && !showHoverDecoration,
5158
mx_RoomListItemView_notification_decoration: isNotificationDecorationVisible,
52-
mx_RoomListItemView_menu_open: showHoverDecoration,
59+
mx_RoomListItemView_hover: showHoverDecoration,
60+
mx_RoomListItemView_menu_open: showHoverMenu,
5361
mx_RoomListItemView_selected: isSelected,
5462
mx_RoomListItemView_bold: vm.isBold,
5563
})}
5664
type="button"
5765
aria-selected={isSelected}
5866
aria-label={vm.a11yLabel}
5967
onClick={() => vm.openRoom()}
60-
onMouseOver={() => setIsHover(true)}
61-
onMouseOut={() => setIsHover(false)}
62-
onFocus={() => setIsHover(true)}
63-
onBlur={() => setIsHover(false)}
68+
onMouseOver={() => setIsHoverWithDelay(true)}
69+
onMouseOut={() => setIsHoverWithDelay(false)}
70+
onFocus={() => {
71+
setIsHoverWithDelay(true);
72+
onFocus();
73+
}}
74+
// Adding a timeout because when tabbing to go to the more options and notification menu, the focus moves out of the button
75+
// The blur makes the button lose the hover state and these menu are not shown
76+
// We delay the blur event to give time to the focus to move to the menu
77+
onBlur={() => setIsHoverWithDelay(false, 10)}
78+
tabIndex={isActive ? 0 : -1}
6479
{...props}
6580
>
6681
{/* 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 */}
@@ -79,13 +94,19 @@ export const RoomListItemView = memo(function RoomListItemView({
7994
</div>
8095
<div className="mx_RoomListItemView_messagePreview">{vm.messagePreview}</div>
8196
</div>
82-
{showHoverDecoration ? (
97+
{showHoverMenu ? (
8398
<RoomListItemMenuView
8499
room={room}
85100
setMenuOpen={(isOpen) => {
86-
if (isOpen) setIsMenuOpen(isOpen);
87-
// To avoid icon blinking when closing the menu, we delay the state update
88-
else setTimeout(() => setIsMenuOpen(isOpen), 0);
101+
if (isOpen) {
102+
setIsMenuOpen(isOpen);
103+
} else {
104+
// To avoid icon blinking when closing the menu, we delay the state update
105+
setTimeout(() => setIsMenuOpen(isOpen), 0);
106+
// After closing the menu, we need to set the focus back to the button
107+
// 10ms because the focus moves to the body and we put back the focus on the button
108+
setTimeout(() => buttonRef.current?.focus(), 10);
109+
}
89110
}}
90111
/>
91112
) : (
@@ -105,3 +126,33 @@ export const RoomListItemView = memo(function RoomListItemView({
105126
</button>
106127
);
107128
});
129+
130+
/**
131+
* Custom hook to manage the hover state of the room list item
132+
* If the timeout is set, it will set the hover state after the timeout
133+
* If the timeout is not set, it will set the hover state immediately
134+
* When the set method is called, it will clear any existing timeout
135+
*
136+
* @returns {boolean} isHover - The hover state
137+
*/
138+
function useIsHover(): [boolean, (value: boolean, timeout?: number) => void] {
139+
const [isHover, setIsHover] = useState(false);
140+
// Store the timeout ID
141+
const timeoutRef = useRef<number | undefined>(undefined);
142+
143+
const setIsHoverWithDelay = useCallback((value: boolean, timeout?: number): void => {
144+
// Clear the timeout if it exists
145+
clearTimeout(timeoutRef.current);
146+
147+
// No delay, set the value immediately
148+
if (timeout === undefined) {
149+
setIsHover(value);
150+
return;
151+
}
152+
153+
// Set a timeout to set the value after the delay
154+
timeoutRef.current = setTimeout(() => setIsHover(value), timeout);
155+
}, []);
156+
157+
return [isHover, setIsHoverWithDelay];
158+
}

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,15 @@
88
import React from "react";
99
import { type MatrixClient } from "matrix-js-sdk/src/matrix";
1010
import { render } from "jest-matrix-react";
11+
import { fireEvent } from "@testing-library/dom";
1112

1213
import { mkRoom, stubClient } from "../../../../../test-utils";
1314
import { type RoomListViewState } from "../../../../../../src/components/viewmodels/roomlist/RoomListViewModel";
1415
import { RoomList } from "../../../../../../src/components/views/rooms/RoomListPanel/RoomList";
1516
import DMRoomMap from "../../../../../../src/utils/DMRoomMap";
1617
import { SecondaryFilters } from "../../../../../../src/components/viewmodels/roomlist/useFilteredRooms";
1718
import { SortOption } from "../../../../../../src/components/viewmodels/roomlist/useSorter";
19+
import { Landmark, LandmarkNavigation } from "../../../../../../src/accessibility/LandmarkNavigation";
1820

1921
describe("<RoomList />", () => {
2022
let matrixClient: MatrixClient;
@@ -53,4 +55,16 @@ describe("<RoomList />", () => {
5355
const { asFragment } = render(<RoomList vm={vm} />);
5456
expect(asFragment()).toMatchSnapshot();
5557
});
58+
59+
it.each([
60+
{ shortcut: { key: "F6", ctrlKey: true, shiftKey: true }, isPreviousLandmark: true, label: "PreviousLandmark" },
61+
{ shortcut: { key: "F6", ctrlKey: true }, isPreviousLandmark: false, label: "NextLandmark" },
62+
])("should navigate to the landmark on NextLandmark.$label action", ({ shortcut, isPreviousLandmark }) => {
63+
const spyFindLandmark = jest.spyOn(LandmarkNavigation, "findAndFocusNextLandmark").mockReturnValue();
64+
const { getByTestId } = render(<RoomList vm={vm} />);
65+
const roomList = getByTestId("room-list");
66+
fireEvent.keyDown(roomList, shortcut);
67+
68+
expect(spyFindLandmark).toHaveBeenCalledWith(Landmark.ROOM_LIST, isPreviousLandmark);
69+
});
5670
});

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,17 @@ describe("<RoomListItemView />", () => {
9191
await waitFor(() => expect(screen.getByRole("button", { name: "More Options" })).toBeInTheDocument());
9292
});
9393

94+
test("should hover decoration if focused", async () => {
95+
const user = userEvent.setup();
96+
render(<RoomListItemView room={room} isSelected={false} />, withClientContextRenderOptions(matrixClient));
97+
const listItem = screen.getByRole("button", { name: `Open room ${room.name}` });
98+
await user.click(listItem);
99+
expect(listItem).toHaveClass("mx_RoomListItemView_hover");
100+
101+
await user.tab();
102+
await waitFor(() => expect(listItem).not.toHaveClass("mx_RoomListItemView_hover"));
103+
});
104+
94105
test("should be selected if isSelected=true", async () => {
95106
const { asFragment } = render(<RoomListItemView room={room} isSelected={true} />);
96107
expect(screen.queryByRole("button", { name: `Open room ${room.name}` })).toHaveAttribute(

0 commit comments

Comments
 (0)