Add axe compliance for new room list (#30700)

* Add tests for axe violations for the new room list

* axe doesn't like a ul/li with roles listbox/option. Changing to div/button as we have elsewhere like RoomListitemView.

* Fix RoomListPrimaryFilters test

* Justify the items in the primary filter container

to get the dropdown button on the right again

* Update snapshot

* Make the room list itself focusable

As the comment said, there was no real reason it needed to be, except
that there was because of axe. Probably having the children focusable
would be better, but Virtuoso wraps them in more divs which doesn't
satisfy axe's requirements since those inner divs are not the scrollable
ones. I can't see a better option than this right now.

* Update snapshot

---------

Co-authored-by: David Baker <dbkr@users.noreply.github.com>

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

@@ -41,7 +41,7 @@ test.describe("Room list", () => {
             }
         });
 
-        test("should render the room list", { tag: "@screenshot" }, async ({ page, app, user }) => {
+        test("should render the room list", { tag: "@screenshot" }, async ({ page, app, user, axe }) => {
             const roomListView = getRoomList(page);
             await expect(roomListView.getByRole("option", { name: "Open room room29" })).toBeVisible();
             await expect(roomListView).toMatchScreenshot("room-list.png");
@@ -54,6 +54,7 @@ test.describe("Room list", () => {
             // scrollListToBottom seems to leave the mouse hovered over the list, move it away.
             await page.getByRole("button", { name: "User menu" }).hover();
 
+            await expect(axe).toHaveNoViolations();
             await expect(roomListView).toMatchScreenshot("room-list-scrolled.png");
         });
 

src/components/utils/ListView.tsx

@@ -285,7 +285,10 @@ export function ListView<Item, Context = any>(props: IListViewProps<Item, Contex
 
     return (
         <Virtuoso
-            tabIndex={props.tabIndex || undefined} // We don't need to focus the container, so leave it undefined by default
+            // note that either the container of direct children must be focusable to be axe
+            // compliant, so we leave tabIndex as the default so the container can be focused
+            // (virtuoso wraps the children inside another couple of elements so setting it
+            // on those doesn't seem to work, unfortunately)
             ref={virtuosoHandleRef}
             scrollerRef={scrollerRef}
             onKeyDown={keyDownCallback}

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

@@ -36,6 +36,7 @@ export function RoomListPrimaryFilters({ vm }: RoomListPrimaryFiltersProps): JSX
             data-testid="primary-filters"
             gap="var(--cpd-space-3x)"
             direction="row-reverse"
+            justify="space-between"
         >
             {displayChevron && (
                 <IconButton
@@ -52,7 +53,7 @@ export function RoomListPrimaryFilters({ vm }: RoomListPrimaryFiltersProps): JSX
             )}
             <Flex
                 id={id}
-                as="ul"
+                as="div"
                 role="listbox"
                 aria-label={_t("room_list|primary_filters")}
                 align="center"
@@ -61,11 +62,9 @@ export function RoomListPrimaryFilters({ vm }: RoomListPrimaryFiltersProps): JSX
                 ref={ref}
             >
                 {filters.map((filter, i) => (
-                    <li role="option" aria-selected={filter.active} key={i}>
-                        <ChatFilter selected={filter.active} onClick={() => filter.toggle()}>
-                            {filter.name}
-                        </ChatFilter>
-                    </li>
+                    <ChatFilter key={i} role="option" selected={filter.active} onClick={() => filter.toggle()}>
+                        {filter.name}
+                    </ChatFilter>
                 ))}
             </Flex>
         </Flex>

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

@@ -63,7 +63,7 @@ describe("<RoomListPrimaryFilters />", () => {
         render(<RoomListPrimaryFilters vm={vm} />);
 
         // Click on an inactive filter
-        await user.click(screen.getByRole("button", { name: "People" }));
+        await user.click(screen.getByRole("option", { name: "People" }));
 
         // Check that the toggle function was called
         expect(filterToggleMocks[0]).toHaveBeenCalledTimes(1);

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

@@ -9,6 +9,7 @@ exports[`<RoomList /> should render a room list 1`] = `
     data-virtuoso-scroller="true"
     role="listbox"
     style="height: 100%; outline: none; overflow-y: auto; position: relative;"
+    tabindex="0"
   >
     <div
       data-viewport-type="element"

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

@@ -5,58 +5,43 @@ exports[`<RoomListPrimaryFilters /> should renders all filters correctly 1`] = `
   <div
     class="flex mx_RoomListPrimaryFilters"
     data-testid="primary-filters"
-    style="--mx-flex-display: flex; --mx-flex-direction: row-reverse; --mx-flex-align: start; --mx-flex-justify: start; --mx-flex-gap: var(--cpd-space-3x); --mx-flex-wrap: nowrap;"
+    style="--mx-flex-display: flex; --mx-flex-direction: row-reverse; --mx-flex-align: start; --mx-flex-justify: space-between; --mx-flex-gap: var(--cpd-space-3x); --mx-flex-wrap: nowrap;"
   >
-    <ul
+    <div
       aria-label="Room list filters"
       class="flex"
       id="«r0»"
       role="listbox"
       style="--mx-flex-display: flex; --mx-flex-direction: row; --mx-flex-align: center; --mx-flex-justify: start; --mx-flex-gap: var(--cpd-space-2x); --mx-flex-wrap: wrap;"
     >
-      <li
+      <button
         aria-hidden="false"
         aria-selected="false"
+        class="_chat-filter_5qdp0_8"
         role="option"
+        tabindex="0"
       >
-        <button
-          aria-selected="false"
-          class="_chat-filter_5qdp0_8"
-          role="button"
-          tabindex="0"
-        >
-          People
-        </button>
-      </li>
-      <li
+        People
+      </button>
+      <button
         aria-hidden="false"
         aria-selected="true"
+        class="_chat-filter_5qdp0_8"
         role="option"
+        tabindex="0"
       >
-        <button
-          aria-selected="true"
-          class="_chat-filter_5qdp0_8"
-          role="button"
-          tabindex="0"
-        >
-          Rooms
-        </button>
-      </li>
-      <li
+        Rooms
+      </button>
+      <button
         aria-hidden="false"
         aria-selected="false"
+        class="_chat-filter_5qdp0_8"
         role="option"
+        tabindex="0"
       >
-        <button
-          aria-selected="false"
-          class="_chat-filter_5qdp0_8"
-          role="button"
-          tabindex="0"
-        >
-          Unreads
-        </button>
-      </li>
-    </ul>
+        Unreads
+      </button>
+    </div>
   </div>
 </DocumentFragment>
 `;