fix(room list): move aria-setsize and aria-posinset on room list cell

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

@@ -35,7 +35,7 @@ test.describe("Room list panel", () => {
     test("should render the room list panel", { tag: "@screenshot" }, async ({ page, app, user }) => {
         const roomListView = getRoomListView(page);
         // Wait for the last room to be visible
-        await expect(roomListView.getByRole("gridcell", { name: "Open room room19" })).toBeVisible();
+        await expect(roomListView.getByRole("option", { name: "Open room room19" })).toBeVisible();
         await expect(roomListView).toMatchScreenshot("room-list-panel.png");
     });
 });

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

@@ -32,19 +32,19 @@ test.describe("Room list", () => {
 
     test("should render the room list", { tag: "@screenshot" }, async ({ page, app, user }) => {
         const roomListView = getRoomList(page);
-        await expect(roomListView.getByRole("gridcell", { name: "Open room room29" })).toBeVisible();
+        await expect(roomListView.getByRole("option", { name: "Open room room29" })).toBeVisible();
         await expect(roomListView).toMatchScreenshot("room-list.png");
 
         await roomListView.hover();
         // Scroll to the end of the room list
         await page.mouse.wheel(0, 1000);
-        await expect(roomListView.getByRole("gridcell", { name: "Open room room0" })).toBeVisible();
+        await expect(roomListView.getByRole("option", { name: "Open room room0" })).toBeVisible();
         await expect(roomListView).toMatchScreenshot("room-list-scrolled.png");
     });
 
     test("should open the room when it is clicked", async ({ page, app, user }) => {
         const roomListView = getRoomList(page);
-        await roomListView.getByRole("gridcell", { name: "Open room room29" }).click();
+        await roomListView.getByRole("option", { name: "Open room room29" }).click();
         await expect(page.getByRole("heading", { name: "room29", level: 1 })).toBeVisible();
     });
 });

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

@@ -5,8 +5,9 @@
  * Please see LICENSE files in the repository root for full details.
  */
 
-import React, { useCallback, type JSX } from "react";
+import React, { useCallback, type JSX, useEffect, useRef, type RefObject } from "react";
 import { AutoSizer, List, type ListRowProps } from "react-virtualized";
+import { type Room } from "matrix-js-sdk/src/matrix";
 
 import { type RoomListViewState } from "../../../viewmodels/roomlist/RoomListViewModel";
 import { _t } from "../../../../languageHandler";
@@ -25,14 +26,23 @@ interface RoomListProps {
 export function RoomList({ vm: { rooms, openRoom } }: RoomListProps): JSX.Element {
     const roomRendererMemoized = useCallback(
         ({ key, index, style }: ListRowProps) => (
-            <RoomListCell room={rooms[index]} key={key} style={style} onClick={() => openRoom(rooms[index].roomId)} />
+            <RoomListCell
+                room={rooms[index]}
+                key={key}
+                style={style}
+                aria-setsize={rooms.length}
+                aria-posinset={index + 1}
+                onClick={() => openRoom(rooms[index].roomId)}
+            />
         ),
         [rooms, openRoom],
     );
 
+    const ref = useAccessibleList(rooms);
+
     // 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">
+        <div className="mx_RoomList" data-testid="room-list" ref={ref}>
             <AutoSizer>
                 {({ height, width }) => (
                     <List
@@ -43,9 +53,58 @@ export function RoomList({ vm: { rooms, openRoom } }: RoomListProps): JSX.Elemen
                         rowHeight={48}
                         height={height}
                         width={width}
+                        role="listbox"
                     />
                 )}
             </AutoSizer>
         </div>
     );
 }
+
+/**
+ * Make the list of rooms accessible. The ref should be put on the list node.
+ *
+ * The list rendered by react-virtualized has not the best role and attributes for accessibility.
+ * The react-virtualized list has the following a11y attributes: "grid" -> "row" -> ["gridcell", "gridcell", "gridcell"].
+ * Using a grid role is not the best choice for a list of items, we want instead a listbox role with children having an option role.
+ *
+ * The listbox and option roles can be set directly in the jsx of the `List` and the `RoomListCell` component but the "gridcell" role is remaining.
+ * This hook removes the "gridcell" role from the list items and set "aria-setsize" on the list too.
+ *
+ * @returns The ref to put on the list node.
+ */
+function useAccessibleList(rooms: Room[]): RefObject<HTMLDivElement> {
+    // To be put on the list node
+    const ref = useRef<HTMLDivElement>(null);
+    useEffect(() => {
+        const list = ref.current?.querySelector('[role="listbox"]');
+        if (!list) return;
+
+        // Determine if a node is a row node
+        const isRowNode = (node: HTMLElement): boolean => node.getAttribute("role") === "row";
+
+        // Watch the node with the "row" role to be added to the dom and remove the role
+        // If the role is re-added/modified later, we remove it too
+        const observer = new MutationObserver((mutationList) => {
+            for (const mutation of mutationList) {
+                if (mutation.type === "childList") {
+                    mutation.addedNodes.forEach((node) => {
+                        if (node instanceof HTMLElement && isRowNode(node)) {
+                            node.removeAttribute("role");
+                        }
+                    });
+                } else if (
+                    mutation.type === "attributes" &&
+                    mutation.target instanceof HTMLElement &&
+                    isRowNode(mutation.target)
+                ) {
+                    mutation.target.removeAttribute("role");
+                }
+            }
+        });
+        observer.observe(list, { childList: true, subtree: true, attributeFilter: ["role"] });
+        return () => observer.disconnect();
+    }, [rooms]);
+
+    return ref;
+}

src/components/views/rooms/RoomListPanel/RoomListCell.tsx

@@ -29,6 +29,8 @@ export function RoomListCell({ room, ...props }: RoomListCellProps): JSX.Element
             type="button"
             aria-label={_t("room_list|room|open_room", { roomName: room.name })}
             {...props}
+            role="option"
+            aria-selected={false}
         >
             {/* 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 */}
             <Flex className="mx_RoomListCell_container" gap="var(--cpd-space-3x)" align="center">

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

@@ -34,19 +34,36 @@ describe("<RoomList />", () => {
         jest.spyOn(DMRoomMap.shared(), "getUserIdForRoomId").mockReturnValue(null);
     });
 
-    it("should render a room list", () => {
-        const { asFragment } = render(<RoomList vm={vm} />);
+    async function renderList() {
+        const renderResult = render(<RoomList vm={vm} />);
+        // Wait for the row role to be removed
+        await waitFor(() => expect(screen.queryByRole("row")).toBeNull());
+        return renderResult;
+    }
+
+    it("should render a room list", async () => {
+        const { asFragment } = await renderList();
         expect(asFragment()).toMatchSnapshot();
     });
 
     it("should open the room", async () => {
         const user = userEvent.setup();
 
-        render(<RoomList vm={vm} />);
+        await renderList();
         await waitFor(async () => {
-            expect(screen.getByRole("gridcell", { name: "Open room room9" })).toBeVisible();
-            await user.click(screen.getByRole("gridcell", { name: "Open room room9" }));
+            expect(screen.getByRole("option", { name: "Open room room9" })).toBeVisible();
+            await user.click(screen.getByRole("option", { name: "Open room room9" }));
         });
         expect(vm.openRoom).toHaveBeenCalledWith(vm.rooms[9].roomId);
     });
+
+    it("should remove the role attribute", async () => {
+        await renderList();
+
+        const rowElement = screen.getByRole("listbox").children.item(0)!;
+        expect(rowElement).not.toHaveAttribute("role");
+
+        rowElement.setAttribute("role", "row");
+        await waitFor(() => expect(rowElement).not.toHaveAttribute("role"));
+    });
 });

test/unit-tests/components/views/rooms/RoomListPanel/RoomListCell-test.tsx

@@ -38,7 +38,7 @@ describe("<RoomListCell />", () => {
         const onClick = jest.fn();
         render(<RoomListCell room={room} onClick={onClick} />);
 
-        await user.click(screen.getByRole("button", { name: `Open room ${room.name}` }));
+        await user.click(screen.getByRole("option", { name: `Open room ${room.name}` }));
         expect(onClick).toHaveBeenCalled();
     });
 });

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

@@ -13,19 +13,21 @@ exports[`<RoomList /> should render a room list 1`] = `
         aria-label="Room list"
         aria-readonly="true"
         class="ReactVirtualized__Grid ReactVirtualized__List mx_RoomList_List"
-        role="grid"
+        role="listbox"
         style="box-sizing: border-box; direction: ltr; height: 1500px; position: relative; width: 1500px; will-change: transform; overflow-x: hidden; overflow-y: hidden;"
         tabindex="0"
       >
         <div
           class="ReactVirtualized__Grid__innerScrollContainer"
-          role="row"
           style="width: auto; height: 480px; max-width: 1500px; max-height: 480px; overflow: hidden; position: relative;"
         >
           <button
             aria-label="Open room room0"
+            aria-posinset="1"
+            aria-selected="false"
+            aria-setsize="10"
             class="mx_RoomListCell"
-            role="gridcell"
+            role="option"
             style="height: 48px; left: 0px; position: absolute; top: 0px; width: 100%;"
             type="button"
           >
@@ -70,8 +72,11 @@ exports[`<RoomList /> should render a room list 1`] = `
           </button>
           <button
             aria-label="Open room room1"
+            aria-posinset="2"
+            aria-selected="false"
+            aria-setsize="10"
             class="mx_RoomListCell"
-            role="gridcell"
+            role="option"
             style="height: 48px; left: 0px; position: absolute; top: 48px; width: 100%;"
             type="button"
           >
@@ -116,8 +121,11 @@ exports[`<RoomList /> should render a room list 1`] = `
           </button>
           <button
             aria-label="Open room room2"
+            aria-posinset="3"
+            aria-selected="false"
+            aria-setsize="10"
             class="mx_RoomListCell"
-            role="gridcell"
+            role="option"
             style="height: 48px; left: 0px; position: absolute; top: 96px; width: 100%;"
             type="button"
           >
@@ -162,8 +170,11 @@ exports[`<RoomList /> should render a room list 1`] = `
           </button>
           <button
             aria-label="Open room room3"
+            aria-posinset="4"
+            aria-selected="false"
+            aria-setsize="10"
             class="mx_RoomListCell"
-            role="gridcell"
+            role="option"
             style="height: 48px; left: 0px; position: absolute; top: 144px; width: 100%;"
             type="button"
           >
@@ -208,8 +219,11 @@ exports[`<RoomList /> should render a room list 1`] = `
           </button>
           <button
             aria-label="Open room room4"
+            aria-posinset="5"
+            aria-selected="false"
+            aria-setsize="10"
             class="mx_RoomListCell"
-            role="gridcell"
+            role="option"
             style="height: 48px; left: 0px; position: absolute; top: 192px; width: 100%;"
             type="button"
           >
@@ -254,8 +268,11 @@ exports[`<RoomList /> should render a room list 1`] = `
           </button>
           <button
             aria-label="Open room room5"
+            aria-posinset="6"
+            aria-selected="false"
+            aria-setsize="10"
             class="mx_RoomListCell"
-            role="gridcell"
+            role="option"
             style="height: 48px; left: 0px; position: absolute; top: 240px; width: 100%;"
             type="button"
           >
@@ -300,8 +317,11 @@ exports[`<RoomList /> should render a room list 1`] = `
           </button>
           <button
             aria-label="Open room room6"
+            aria-posinset="7"
+            aria-selected="false"
+            aria-setsize="10"
             class="mx_RoomListCell"
-            role="gridcell"
+            role="option"
             style="height: 48px; left: 0px; position: absolute; top: 288px; width: 100%;"
             type="button"
           >
@@ -346,8 +366,11 @@ exports[`<RoomList /> should render a room list 1`] = `
           </button>
           <button
             aria-label="Open room room7"
+            aria-posinset="8"
+            aria-selected="false"
+            aria-setsize="10"
             class="mx_RoomListCell"
-            role="gridcell"
+            role="option"
             style="height: 48px; left: 0px; position: absolute; top: 336px; width: 100%;"
             type="button"
           >
@@ -392,8 +415,11 @@ exports[`<RoomList /> should render a room list 1`] = `
           </button>
           <button
             aria-label="Open room room8"
+            aria-posinset="9"
+            aria-selected="false"
+            aria-setsize="10"
             class="mx_RoomListCell"
-            role="gridcell"
+            role="option"
             style="height: 48px; left: 0px; position: absolute; top: 384px; width: 100%;"
             type="button"
           >
@@ -438,8 +464,11 @@ exports[`<RoomList /> should render a room list 1`] = `
           </button>
           <button
             aria-label="Open room room9"
+            aria-posinset="10"
+            aria-selected="false"
+            aria-setsize="10"
             class="mx_RoomListCell"
-            role="gridcell"
+            role="option"
             style="height: 48px; left: 0px; position: absolute; top: 432px; width: 100%;"
             type="button"
           >

test/unit-tests/components/views/rooms/RoomListPanel/__snapshots__/RoomListCell-test.tsx.snap

@@ -4,7 +4,9 @@ exports[`<RoomListCell /> should render a room cell 1`] = `
 <DocumentFragment>
   <button
     aria-label="Open room room1"
+    aria-selected="false"
     class="mx_RoomListCell"
+    role="option"
     type="button"
   >
     <div

test/unit-tests/components/views/rooms/RoomListPanel/__snapshots__/RoomListPanel-test.tsx.snap

@@ -35,7 +35,7 @@ exports[`<RoomListPanel /> should not render the RoomListSearch component when U
           aria-label="Room list"
           aria-readonly="true"
           class="ReactVirtualized__Grid ReactVirtualized__List mx_RoomList_List"
-          role="grid"
+          role="listbox"
           style="box-sizing: border-box; direction: ltr; height: 0px; position: relative; width: 0px; will-change: transform; overflow-x: hidden; overflow-y: hidden;"
           tabindex="0"
         />
@@ -185,7 +185,7 @@ exports[`<RoomListPanel /> should render the RoomListSearch component when UICom
           aria-label="Room list"
           aria-readonly="true"
           class="ReactVirtualized__Grid ReactVirtualized__List mx_RoomList_List"
-          role="grid"
+          role="listbox"
           style="box-sizing: border-box; direction: ltr; height: 0px; position: relative; width: 0px; will-change: transform; overflow-x: hidden; overflow-y: hidden;"
           tabindex="0"
         />