From dba4ca26e869f0245e8955fa5e46f128a9c49bc9 Mon Sep 17 00:00:00 2001 From: David Langley Date: Tue, 9 Sep 2025 17:54:13 +0100 Subject: [PATCH] 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 --- .../room-list-panel/room-list.spec.ts | 3 +- src/components/utils/ListView.tsx | 5 +- .../RoomListPanel/RoomListPrimaryFilters.tsx | 11 ++-- .../RoomListPrimaryFilters-test.tsx | 2 +- .../__snapshots__/RoomList-test.tsx.snap | 1 + .../RoomListPrimaryFilters-test.tsx.snap | 51 +++++++------------ 6 files changed, 31 insertions(+), 42 deletions(-) diff --git a/playwright/e2e/left-panel/room-list-panel/room-list.spec.ts b/playwright/e2e/left-panel/room-list-panel/room-list.spec.ts index 67910f61e8..0b6aa34029 100644 --- a/playwright/e2e/left-panel/room-list-panel/room-list.spec.ts +++ b/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"); }); diff --git a/src/components/utils/ListView.tsx b/src/components/utils/ListView.tsx index b8425b366f..235a706a55 100644 --- a/src/components/utils/ListView.tsx +++ b/src/components/utils/ListView.tsx @@ -285,7 +285,10 @@ export function ListView(props: IListViewProps {displayChevron && ( {filters.map((filter, i) => ( -
  • - filter.toggle()}> - {filter.name} - -
    • + filter.toggle()}> + {filter.name} + ))} diff --git a/test/unit-tests/components/views/rooms/RoomListPanel/RoomListPrimaryFilters-test.tsx b/test/unit-tests/components/views/rooms/RoomListPanel/RoomListPrimaryFilters-test.tsx index 7128ecd1b8..d7d4e8314f 100644 --- a/test/unit-tests/components/views/rooms/RoomListPanel/RoomListPrimaryFilters-test.tsx +++ b/test/unit-tests/components/views/rooms/RoomListPanel/RoomListPrimaryFilters-test.tsx @@ -63,7 +63,7 @@ describe("", () => { render(); // 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); diff --git a/test/unit-tests/components/views/rooms/RoomListPanel/__snapshots__/RoomList-test.tsx.snap b/test/unit-tests/components/views/rooms/RoomListPanel/__snapshots__/RoomList-test.tsx.snap index fcbaa9110e..078950c360 100644 --- a/test/unit-tests/components/views/rooms/RoomListPanel/__snapshots__/RoomList-test.tsx.snap +++ b/test/unit-tests/components/views/rooms/RoomListPanel/__snapshots__/RoomList-test.tsx.snap @@ -9,6 +9,7 @@ exports[` should render a room list 1`] = ` data-virtuoso-scroller="true" role="listbox" style="height: 100%; outline: none; overflow-y: auto; position: relative;" + tabindex="0" >
    should renders all filters correctly 1`] = `
    -
      -
    • - -
      • -
    • + -
      • -
    • + -
      • -
    + Unreads + +
    `;