Skip to content

Add aria-describedBy to room list menus #30126

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
+176 −9
Closed

Add aria-describedBy to room list menus #30126

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a26703c
Add aria-describedBy to room list menus
dbkr Jun 11, 2025
3eb33b0
Update snapshot
dbkr Jun 11, 2025
e0a73a0
Add snapshot
dbkr Jun 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 31 additions & 7 deletions src/components/views/rooms/RoomListPanel/RoomListItemMenuView.tsx
View file
Original file line number Diff line number Diff line change
@@ -38,18 +38,28 @@ interface RoomListItemMenuViewProps {
* @param isOpen
*/
setMenuOpen: (isOpen: boolean) => void;
/**
* If set, the ID of a DOM element that describes the menu buttons in this view.
* Note that this will be the same for each menu button: it is suggested that it is the
* ID of the room list item that this menu is for, ie. containing the room name as text.
*/
describedById?: string;
}

/**
* A view for the room list item menu.
*/
export function RoomListItemMenuView({ room, setMenuOpen }: RoomListItemMenuViewProps): JSX.Element {
export function RoomListItemMenuView({ room, setMenuOpen, describedById }: RoomListItemMenuViewProps): JSX.Element {
const vm = useRoomListItemMenuViewModel(room);

return (
<Flex className="mx_RoomListItemMenuView" align="center" gap="var(--cpd-space-1x)">
{vm.showMoreOptionsMenu && <MoreOptionsMenu setMenuOpen={setMenuOpen} vm={vm} />}
{vm.showNotificationMenu && <NotificationMenu setMenuOpen={setMenuOpen} vm={vm} />}
{vm.showMoreOptionsMenu && (
<MoreOptionsMenu setMenuOpen={setMenuOpen} vm={vm} describedById={describedById} />
)}
{vm.showNotificationMenu && (
<NotificationMenu setMenuOpen={setMenuOpen} vm={vm} describedById={describedById} />
)}
</Flex>
);
}
@@ -64,12 +74,18 @@ interface MoreOptionsMenuProps {
* @param isOpen
*/
setMenuOpen: (isOpen: boolean) => void;
/**
* If set, the ID of a DOM element that describes the menu buttons in this view.
* Note that this will be the same for each menu button: it is suggested that it is the
* ID of the room list item that this menu is for, ie. containing the room name as text.
*/
describedById?: string;
}

/**
* The more options menu for the room list item.
*/
function MoreOptionsMenu({ vm, setMenuOpen }: MoreOptionsMenuProps): JSX.Element {
function MoreOptionsMenu({ vm, setMenuOpen, describedById }: MoreOptionsMenuProps): JSX.Element {
const [open, setOpen] = useState(false);

return (
@@ -82,7 +98,7 @@ function MoreOptionsMenu({ vm, setMenuOpen }: MoreOptionsMenuProps): JSX.Element
title={_t("room_list|room|more_options")}
showTitle={false}
align="start"
trigger={<MoreOptionsButton size="24px" />}
trigger={<MoreOptionsButton size="24px" aria-describedby={describedById} />}
>
{vm.canMarkAsRead && (
<MenuItem
@@ -174,9 +190,15 @@ interface NotificationMenuProps {
* @param isOpen
*/
setMenuOpen: (isOpen: boolean) => void;
/**
* If set, the ID of a DOM element that describes the menu buttons in this view.
* Note that this will be the same for each menu button: it is suggested that it is the
* ID of the room list item that this menu is for, ie. containing the room name as text.
*/
describedById?: string;
}

function NotificationMenu({ vm, setMenuOpen }: NotificationMenuProps): JSX.Element {
function NotificationMenu({ vm, setMenuOpen, describedById }: NotificationMenuProps): JSX.Element {
const [open, setOpen] = useState(false);

const checkComponent = <CheckIcon width="24px" height="24px" color="var(--cpd-color-icon-primary)" />;
@@ -191,7 +213,9 @@ function NotificationMenu({ vm, setMenuOpen }: NotificationMenuProps): JSX.Eleme
title={_t("room_list|notification_options")}
showTitle={false}
align="start"
trigger={<NotificationButton isRoomMuted={vm.isNotificationMute} size="24px" />}
trigger={
<NotificationButton isRoomMuted={vm.isNotificationMute} size="24px" aria-describedby={describedById} />
}
>
<MenuItem
aria-selected={vm.isNotificationAllMessage}
6 changes: 4 additions & 2 deletions src/components/views/rooms/RoomListPanel/RoomListItemView.tsx
View file
Original file line number Diff line number Diff line change
@@ -5,7 +5,7 @@
* Please see LICENSE files in the repository root for full details.
*/

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

@@ -46,6 +46,7 @@ export const RoomListItemView = memo(function RoomListItemView({
// Using display: none; and then display:flex when hovered in CSS causes the menu to be misaligned
const showHoverDecoration = isMenuOpen || isHover;
const showHoverMenu = showHoverDecoration && vm.showHoverMenu;
const roomNameId = useId();

return (
<button
@@ -84,14 +85,15 @@ export const RoomListItemView = memo(function RoomListItemView({
>
{/* We truncate the room name when too long. Title here is to show the full name on hover */}
<div className="mx_RoomListItemView_text">
<div className="mx_RoomListItemView_roomName" title={vm.name}>
<div className="mx_RoomListItemView_roomName" title={vm.name} id={roomNameId}>
{vm.name}
</div>
<div className="mx_RoomListItemView_messagePreview">{vm.messagePreview}</div>
</div>
{showHoverMenu ? (
<RoomListItemMenuView
room={room}
describedById={roomNameId}
setMenuOpen={(isOpen) => {
if (isOpen) {
setIsMenuOpen(isOpen);
4 changes: 4 additions & 0 deletions test/unit-tests/components/views/rooms/RoomListPanel/RoomListItemView-test.tsx
View file
Original file line number Diff line number Diff line change
@@ -89,6 +89,10 @@ describe("<RoomListItemView />", () => {

await user.hover(listItem);
await waitFor(() => expect(screen.getByRole("button", { name: "More Options" })).toBeInTheDocument());

// also make another snapshot in the hover state (mostly to test the aria-describedBy on the buttons
// as there's no easy way to get the effective ARIA description in react testing library, surprisingly)
expect(listItem).toMatchSnapshot();
});

test("should hover decoration if focused", async () => {
10 changes: 10 additions & 0 deletions test/unit-tests/components/views/rooms/RoomListPanel/__snapshots__/RoomList-test.tsx.snap
View file
Original file line number Diff line number Diff line change
@@ -63,6 +63,7 @@ exports[`<RoomList /> should render a room list 1`] = `
>
<div
class="mx_RoomListItemView_roomName"
id="«r0»"
title="room0"
>
room0
@@ -115,6 +116,7 @@ exports[`<RoomList /> should render a room list 1`] = `
>
<div
class="mx_RoomListItemView_roomName"
id="«r1»"
title="room1"
>
room1
@@ -167,6 +169,7 @@ exports[`<RoomList /> should render a room list 1`] = `
>
<div
class="mx_RoomListItemView_roomName"
id="«r2»"
title="room2"
>
room2
@@ -219,6 +222,7 @@ exports[`<RoomList /> should render a room list 1`] = `
>
<div
class="mx_RoomListItemView_roomName"
id="«r3»"
title="room3"
>
room3
@@ -271,6 +275,7 @@ exports[`<RoomList /> should render a room list 1`] = `
>
<div
class="mx_RoomListItemView_roomName"
id="«r4»"
title="room4"
>
room4
@@ -323,6 +328,7 @@ exports[`<RoomList /> should render a room list 1`] = `
>
<div
class="mx_RoomListItemView_roomName"
id="«r5»"
title="room5"
>
room5
@@ -375,6 +381,7 @@ exports[`<RoomList /> should render a room list 1`] = `
>
<div
class="mx_RoomListItemView_roomName"
id="«r6»"
title="room6"
>
room6
@@ -427,6 +434,7 @@ exports[`<RoomList /> should render a room list 1`] = `
>
<div
class="mx_RoomListItemView_roomName"
id="«r7»"
title="room7"
>
room7
@@ -479,6 +487,7 @@ exports[`<RoomList /> should render a room list 1`] = `
>
<div
class="mx_RoomListItemView_roomName"
id="«r8»"
title="room8"
>
room8
@@ -531,6 +540,7 @@ exports[`<RoomList /> should render a room list 1`] = `
>
<div
class="mx_RoomListItemView_roomName"
id="«r9»"
title="room9"
>
room9
127 changes: 127 additions & 0 deletions ...t-tests/components/views/rooms/RoomListPanel/__snapshots__/RoomListItemView-test.tsx.snap
View file
Original file line number Diff line number Diff line change
@@ -41,6 +41,7 @@ exports[`<RoomListItemView /> should be selected if isSelected=true 1`] = `
>
<div
class="mx_RoomListItemView_roomName"
id="«rl»"
title="room1"
>
room1
@@ -96,6 +97,7 @@ exports[`<RoomListItemView /> should display notification decoration 1`] = `
>
<div
class="mx_RoomListItemView_roomName"
id="«rm»"
title="room1"
>
room1
@@ -122,6 +124,129 @@ exports[`<RoomListItemView /> should display notification decoration 1`] = `
</DocumentFragment>
`;

exports[`<RoomListItemView /> should hover decoration if hovered 1`] = `
<button
aria-label="Open room room1"
aria-selected="false"
class="mx_RoomListItemView mx_RoomListItemView_hover mx_RoomListItemView_menu_open"
tabindex="-1"
type="button"
>
<div
class="mx_Flex mx_RoomListItemView_container"
style="--mx-flex-display: flex; --mx-flex-direction: row; --mx-flex-align: center; --mx-flex-justify: start; --mx-flex-gap: var(--cpd-space-3x); --mx-flex-wrap: nowrap;"
>
<span
aria-label="Avatar"
class="_avatar_1qbcf_8 mx_BaseAvatar"
data-color="3"
data-testid="avatar-img"
data-type="round"
style="--cpd-avatar-size: 32px;"
>
<img
alt=""
class="_image_1qbcf_41"
data-type="round"
height="32px"
loading="lazy"
referrerpolicy="no-referrer"
src="http://this.is.a.url/avatar.url/room.png"
width="32px"
/>
</span>
<div
class="mx_Flex mx_RoomListItemView_content"
style="--mx-flex-display: flex; --mx-flex-direction: row; --mx-flex-align: center; --mx-flex-justify: space-between; --mx-flex-gap: var(--cpd-space-2x); --mx-flex-wrap: nowrap;"
>
<div
class="mx_RoomListItemView_text"
>
<div
class="mx_RoomListItemView_roomName"
id="«r3»"
title="room1"
>
room1
</div>
<div
class="mx_RoomListItemView_messagePreview"
/>
</div>
<div
class="mx_Flex mx_RoomListItemMenuView"
style="--mx-flex-display: flex; --mx-flex-direction: row; --mx-flex-align: center; --mx-flex-justify: start; --mx-flex-gap: var(--cpd-space-1x); --mx-flex-wrap: nowrap;"
>
<button
aria-describedby="«r3»"
aria-disabled="false"
aria-expanded="false"
aria-haspopup="menu"
aria-label="More Options"
aria-labelledby="«r6»"
class="_icon-button_m2erp_8"
data-state="closed"
id="radix-«r4»"
role="button"
style="--cpd-icon-button-size: 24px;"
tabindex="0"
type="button"
>
<div
class="_indicator-icon_zr2a0_17"
style="--cpd-icon-button-size: 100%;"
>
<svg
fill="currentColor"
height="1em"
viewBox="0 0 24 24"
width="1em"
xmlns="http://www.w3.org/2000/svg"
>
<path
d="M6 14q-.824 0-1.412-.588A1.93 1.93 0 0 1 4 12q0-.825.588-1.412A1.93 1.93 0 0 1 6 10q.824 0 1.412.588Q8 11.175 8 12t-.588 1.412A1.93 1.93 0 0 1 6 14m6 0q-.825 0-1.412-.588A1.93 1.93 0 0 1 10 12q0-.825.588-1.412A1.93 1.93 0 0 1 12 10q.825 0 1.412.588Q14 11.175 14 12t-.588 1.412A1.93 1.93 0 0 1 12 14m6 0q-.824 0-1.413-.588A1.93 1.93 0 0 1 16 12q0-.825.587-1.412A1.93 1.93 0 0 1 18 10q.824 0 1.413.588Q20 11.175 20 12t-.587 1.412A1.93 1.93 0 0 1 18 14"
/>
</svg>
</div>
</button>
<button
aria-describedby="«r3»"
aria-disabled="false"
aria-expanded="false"
aria-haspopup="menu"
aria-label="Notification options"
aria-labelledby="«rd»"
class="_icon-button_m2erp_8"
data-state="closed"
id="radix-«rb»"
role="button"
style="--cpd-icon-button-size: 24px;"
tabindex="0"
type="button"
>
<div
class="_indicator-icon_zr2a0_17"
style="--cpd-icon-button-size: 100%;"
>
<svg
fill="currentColor"
height="1em"
viewBox="0 0 24 24"
width="1em"
xmlns="http://www.w3.org/2000/svg"
>
<path
d="M20.293 17.293c.63.63.184 1.707-.707 1.707H4.414c-.89 0-1.337-1.077-.707-1.707L5 16v-6s0-7 7-7 7 7 7 7v6zM12 22a2 2 0 0 1-2-2h4a2 2 0 0 1-2 2"
/>
</svg>
</div>
</button>
</div>
</div>
</div>
</button>
`;

exports[`<RoomListItemView /> should render a room item 1`] = `
<DocumentFragment>
<button
@@ -163,6 +288,7 @@ exports[`<RoomListItemView /> should render a room item 1`] = `
>
<div
class="mx_RoomListItemView_roomName"
id="«r0»"
title="room1"
>
room1
@@ -218,6 +344,7 @@ exports[`<RoomListItemView /> should render a room item with a message preview 1
>
<div
class="mx_RoomListItemView_roomName"
id="«r1»"
title="room1"
>
room1

Unchanged files with check annotations Beta

View file
playwright/e2e/crypto/device-verification.spec.ts
/*
Copyright 2024 New Vector Ltd.
Copyright 2023 The Matrix.org Foundation C.I.C.
await page.route("**/_matrix/client/v3/sendToDevice/m.secret.request/**", async (route) => {
await route.fulfill({ json: {} });
await new Promise((f) => setTimeout(f, 1000));
await route.fetch();

Check failure on line 97 in playwright/e2e/crypto/device-verification.spec.ts

GitHub Actions / Run Tests [Chrome] 1/6

[Chrome] › playwright/e2e/crypto/device-verification.spec.ts:122:9 › Device verification › Verify device with QR code during login @no-webkit 

  2) [Chrome] › playwright/e2e/crypto/device-verification.spec.ts:122:9 › Device verification › Verify device with QR code during login @no-webkit 
    Error: "route.fetch: Test ended." while running route callback.
    Consider awaiting `await page.unrouteAll({ behavior: 'ignoreErrors' })`
    before the end of the test to ignore remaining routes in flight.

       95 |             await route.fulfill({ json: {} });
       96 |             await new Promise((f) => setTimeout(f, 1000));
    >  97 |             await route.fetch();
          |                         ^
       98 |         });
       99 |
      100 |         await logIntoElement(page, credentials);
        at /home/runner/work/element-web/element-web/playwright/e2e/crypto/device-verification.spec.ts:97:25
});
await logIntoElement(page, credentials);
View file
playwright/e2e/audio-player/audio-player.spec.ts
await expect(async () => {
await tile.hover();
await tile.getByRole("button", { name: "Reply", exact: true }).click();
}).toPass();

Check failure on line 21 in playwright/e2e/audio-player/audio-player.spec.ts

GitHub Actions / Run Tests [Chrome] 1/6

[Chrome] › playwright/e2e/audio-player/audio-player.spec.ts:250:9 › Audio player › should support creating a reply chain with multiple audio files @no-firefox @no-webkit @screenshot 

  1) [Chrome] › playwright/e2e/audio-player/audio-player.spec.ts:250:9 › Audio player › should support creating a reply chain with multiple audio files @no-firefox @no-webkit @screenshot 
    Error: Test timeout of 30000ms exceeded

      19 |         await tile.hover();
      20 |         await tile.getByRole("button", { name: "Reply", exact: true }).click();
    > 21 |     }).toPass();
         |        ^
      22 | };
      23 |
      24 | test.describe("Audio player", { tag: ["@no-firefox", "@no-webkit"] }, () => {
        at clickButtonReply (/home/runner/work/element-web/element-web/playwright/e2e/audio-player/audio-player.spec.ts:21:8)
        at /home/runner/work/element-web/element-web/playwright/e2e/audio-player/audio-player.spec.ts:274:19
};
test.describe("Audio player", { tag: ["@no-firefox", "@no-webkit"] }, () => {
View file
playwright/e2e/timeline/timeline.spec.ts
// wait for the date separator to appear to have a stable screenshot
await expect(page.locator(".mx_TimelineSeparator")).toHaveText("today");
await expect(page.locator(".mx_MainSplit")).toMatchScreenshot("configured-room-irc-layout.png");

Check failure on line 158 in playwright/e2e/timeline/timeline.spec.ts

GitHub Actions / Run Tests [Chrome] 5/6

[Chrome] › playwright/e2e/timeline/timeline.spec.ts:142:13 › Timeline › configure room › should create and configure a room on IRC layout @screenshot 

  1) [Chrome] › playwright/e2e/timeline/timeline.spec.ts:142:13 › Timeline › configure room › should create and configure a room on IRC layout @screenshot 
    Error: expect(locator).toHaveScreenshot(expected)

      40 pixels (ratio 0.01 of all image pixels) are different.

    Expected: /home/runner/work/element-web/element-web/playwright/snapshots/timeline/timeline.spec.ts/configured-room-irc-layout-linux.png
    Received: /home/runner/work/element-web/element-web/playwright/test-results/timeline-timeline-Timeline-43bff-figure-a-room-on-IRC-layout-Chrome/configured-room-irc-layout-actual.png
        Diff: /home/runner/work/element-web/element-web/playwright/test-results/timeline-timeline-Timeline-43bff-figure-a-room-on-IRC-layout-Chrome/configured-room-irc-layout-diff.png

    Call log:
      - expect.toHaveScreenshot(configured-room-irc-layout.png) with timeout 5000ms
        - verifying given screenshot expectation
      - waiting for locator('.mx_MainSplit')
        - locator resolved to <div class="mx_MainSplit">…</div>
      - taking element screenshot
        - disabled all CSS animations
      - waiting for fonts to load...
      - fonts loaded
      - attempting scroll into view action
        - waiting for element to be stable
      - 40 pixels (ratio 0.01 of all image pixels) are different.
      - waiting 100ms before taking screenshot
      - waiting for locator('.mx_MainSplit')
        - locator resolved to <div class="mx_MainSplit">…</div>
      - taking element screenshot
        - disabled all CSS animations
      - waiting for fonts to load...
      - fonts loaded
      - attempting scroll into view action
        - waiting for element to be stable
      - captured a stable screenshot
      - 40 pixels (ratio 0.01 of all image pixels) are different.


      156 |                 await expect(page.locator(".mx_TimelineSeparator")).toHaveText("today");
      157 |
    > 158 |                 await expect(page.locator(".mx_MainSplit")).toMatchScreenshot("configured-room-irc-layout.png");
          |                                                             ^
      159 |             },
      160 |         );
      161 |
        at /home/runner/work/element-web/element-web/playwright/e2e/timeline/timeline.spec.ts:158:61
},
);
}
// Take a snapshot on modern layout
await expect(page.locator(".mx_EventTile_last")).toMatchScreenshot(

Check failure on line 1107 in playwright/e2e/timeline/timeline.spec.ts

GitHub Actions / Run Tests [Chrome] 5/6

[Chrome] › playwright/e2e/timeline/timeline.spec.ts:1024:13 › Timeline › message sending › should display a reply chain @no-firefox @no-webkit @screenshot 

  2) [Chrome] › playwright/e2e/timeline/timeline.spec.ts:1024:13 › Timeline › message sending › should display a reply chain @no-firefox @no-webkit @screenshot 
    Error: expect(locator).toHaveScreenshot(expected)

      41 pixels (ratio 0.01 of all image pixels) are different.

    Expected: /home/runner/work/element-web/element-web/playwright/snapshots/timeline/timeline.spec.ts/event-tile-reply-chains-irc-modern-linux.png
    Received: /home/runner/work/element-web/element-web/playwright/test-results/timeline-timeline-Timeline-801c1-hould-display-a-reply-chain-Chrome/event-tile-reply-chains-irc-modern-actual.png
        Diff: /home/runner/work/element-web/element-web/playwright/test-results/timeline-timeline-Timeline-801c1-hould-display-a-reply-chain-Chrome/event-tile-reply-chains-irc-modern-diff.png

    Call log:
      - expect.toHaveScreenshot(event-tile-reply-chains-irc-modern.png) with timeout 5000ms
        - verifying given screenshot expectation
      - waiting for locator('.mx_EventTile_last')
        - locator resolved to <li tabindex="-1" aria-live="off" data-self="true" aria-atomic="true" data-layout="group" data-has-reply="true" data-event-id="$l33BUViJ2Elvu4RXsx4ZWDWYymF9L15udiyC4_z_4og" data-scroll-tokens="$l33BUViJ2Elvu4RXsx4ZWDWYymF9L15udiyC4_z_4og" class="mx_EventTile mx_EventTile_continuation mx_EventTile_last mx_EventTile_lastInSection">…</li>
      - taking element screenshot
        - disabled all CSS animations
      - waiting for fonts to load...
      - fonts loaded
      - attempting scroll into view action
        - waiting for element to be stable
      - 29 pixels (ratio 0.01 of all image pixels) are different.
      - waiting 100ms before taking screenshot
      - waiting for locator('.mx_EventTile_last')
        - locator resolved to <li tabindex="-1" aria-live="off" data-self="true" aria-atomic="true" data-layout="group" data-has-reply="true" data-event-id="$l33BUViJ2Elvu4RXsx4ZWDWYymF9L15udiyC4_z_4og" data-scroll-tokens="$l33BUViJ2Elvu4RXsx4ZWDWYymF9L15udiyC4_z_4og" class="mx_EventTile mx_EventTile_continuation mx_EventTile_last mx_EventTile_lastInSection">…</li>
      - taking element screenshot
        - disabled all CSS animations
      - waiting for fonts to load...
      - fonts loaded
      - attempting scroll into view action
        - waiting for element to be stable
      - 12 pixels (ratio 0.01 of all image pixels) are different.
      - waiting 250ms before taking screenshot
      - waiting for locator('.mx_EventTile_last')
        - locator resolved to <li tabindex="-1" aria-live="off" data-self="true" aria-atomic="true" data-layout="group" data-has-reply="true" data-event-id="$l33BUViJ2Elvu4RXsx4ZWDWYymF9L15udiyC4_z_4og" data-scroll-tokens="$l33BUViJ2Elvu4RXsx4ZWDWYymF9L15udiyC4_z_4og" class="mx_EventTile mx_EventTile_continuation mx_EventTile_last mx_EventTile_lastInSection">…</li>
      - taking element screenshot
        - disabled all CSS animations
      - waiting for fonts to load...
      - fonts loaded
      - attempting scroll into view action
        - waiting for element to be stable
      - captured a stable screenshot
      - 41 pixels (ratio 0.01 of all image pixels) are different.


      1105 |
      1106 |             // Take a snapshot on modern layout
    > 1107 |             await expect(page.locator(".mx_EventTile_last")).toMatchScreenshot(
           |                                                              ^
      1108 |                 "event-tile-reply-chains-irc-modern.png",
      1109 |                 screenshotOptions,
      1110 |             );
        at /home/runner/work/element-web/element-web/playwright/e2e/timeline/timeline.spec.ts:1107:62
"event-tile-reply-chains-irc-modern.png",
screenshotOptions,
);