Improve accessibility of the `<AvatarSetting> component (#30907)

* Always use an accessible button with base avatar rendered inside it

* Rename avatarAltText to accessibleName

* Improve accessibility

* Fix tests
This commit is contained in:
R Midhun Suresh
2025-09-30 23:23:37 +05:30
committed by GitHub
parent b6046d2120
commit 2f8e2be09d
5 changed files with 40 additions and 42 deletions

View File

@@ -262,7 +262,7 @@ export default class RoomProfileSettings extends React.Component<IProps, IState>
? undefined
: (this.state.avatarFile ?? this.state.originalAvatarUrl ?? undefined)
}
avatarAltText={_t("room_settings|general|avatar_field_label")}
avatarAccessibleName={_t("room_settings|general|avatar_field_label")}
disabled={!this.state.canSetAvatar}
onChange={this.onAvatarChanged}
removeAvatar={canRemove ? this.removeAvatar : undefined}

View File

@@ -6,7 +6,7 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Com
Please see LICENSE files in the repository root for full details.
*/
import React, { type JSX, type ReactNode, createRef, useCallback, useEffect, useState, useId } from "react";
import React, { type JSX, type ReactNode, createRef, useCallback, useEffect, useState } from "react";
import EditIcon from "@vector-im/compound-design-tokens/assets/web/icons/edit";
import UploadIcon from "@vector-im/compound-design-tokens/assets/web/icons/share";
import DeleteIcon from "@vector-im/compound-design-tokens/assets/web/icons/delete";
@@ -89,9 +89,9 @@ interface IProps {
removeAvatar?: () => void;
/**
* The alt text for the avatar
* The accessible name for the avatar, eg: "Foo's Profile Picture"
*/
avatarAltText: string;
avatarAccessibleName: string;
/**
* String to use for computing the colour of the placeholder avatar if no avatar is set
@@ -121,7 +121,7 @@ export function getFileChanged(e: React.ChangeEvent<HTMLInputElement>): File | n
*/
const AvatarSetting: React.FC<IProps> = ({
avatar,
avatarAltText,
avatarAccessibleName,
onChange,
removeAvatar,
disabled,
@@ -147,9 +147,6 @@ const AvatarSetting: React.FC<IProps> = ({
}
}, [avatar]);
// Prevents ID collisions when this component is used more than once on the same page.
const a11yId = useId();
const onFileChanged = useCallback(
(e: React.ChangeEvent<HTMLInputElement>) => {
const file = getFileChanged(e);
@@ -170,33 +167,26 @@ const AvatarSetting: React.FC<IProps> = ({
setMenuOpen(newOpen);
}, []);
let avatarElement = (
const avatarElement = (
<AccessibleButton
element="div"
onClick={uploadAvatar}
/**
* This button will open a menu. That is done by passing this element as trigger
* to the menu component, hence the empty onClick.
*/
onClick={() => {}}
className="mx_AvatarSetting_avatarPlaceholder mx_AvatarSetting_avatarDisplay"
aria-labelledby={disabled ? undefined : a11yId}
// Inhibit tab stop as we have explicit upload/remove buttons
tabIndex={-1}
disabled={disabled}
>
<BaseAvatar idName={placeholderId} name={placeholderName} size="90px" />
<BaseAvatar
idName={placeholderId}
name={placeholderName}
size="90px"
url={avatarURL}
altText={avatarAccessibleName}
/>
</AccessibleButton>
);
if (avatarURL) {
avatarElement = (
<AccessibleButton
element="img"
className="mx_AvatarSetting_avatarDisplay"
src={avatarURL}
alt={avatarAltText}
onClick={uploadAvatar}
// Inhibit tab stop as we have explicit upload/remove buttons
tabIndex={-1}
disabled={disabled}
/>
);
}
let uploadAvatarBtn: JSX.Element | undefined;
if (!disabled) {
@@ -204,14 +194,20 @@ const AvatarSetting: React.FC<IProps> = ({
mx_AvatarSetting_uploadButton_active: menuOpen,
});
uploadAvatarBtn = (
<div className={uploadButtonClasses}>
<EditIcon width="20px" height="20px" />
<div
className={uploadButtonClasses}
role="button"
aria-label={_t("settings|general|avatar_open_menu")}
tabIndex={0}
aria-haspopup="menu"
>
<EditIcon aria-hidden={true} width="20px" height="20px" />
</div>
);
}
const content = (
<div className="mx_AvatarSetting_avatar" role="group" aria-label={avatarAltText}>
<div className="mx_AvatarSetting_avatar" role="group" aria-label={avatarAccessibleName}>
{avatarElement}
{uploadAvatarBtn}
</div>

View File

@@ -203,7 +203,7 @@ const UserProfileSettings: React.FC<UserProfileSettingsProps> = ({
<div className="mx_UserProfileSettings_profile">
<AvatarSetting
avatar={avatarURL ?? undefined}
avatarAltText={_t("common|user_avatar")}
avatarAccessibleName={_t("common|user_avatar")}
onChange={onAvatarChange}
removeAvatar={avatarURL ? onAvatarRemove : undefined}
placeholderName={displayName}

View File

@@ -2664,6 +2664,7 @@
"allow_spellcheck": "Allow spell check",
"application_language": "Application language",
"application_language_reload_hint": "The app will reload after selecting another language",
"avatar_open_menu": "Open avatar menu",
"avatar_remove_progress": "Removing image...",
"avatar_save_progress": "Uploading image...",
"avatar_upload_error_text": "The file format is not supported or the image is larger than %(size)s.",

View File

@@ -25,18 +25,18 @@ describe("<AvatarSetting />", () => {
stubClient();
});
it("renders avatar with specified alt text", async () => {
const { queryByAltText } = render(
it("renders avatar with specified accessible name", async () => {
const { getByRole } = render(
<AvatarSetting
placeholderId="blee"
placeholderName="boo"
avatarAltText="Avatar of Peter Fox"
avatarAccessibleName="Avatar of Peter Fox"
avatar="mxc://example.org/my-avatar"
/>,
);
const imgElement = queryByAltText("Avatar of Peter Fox");
expect(imgElement).toBeInTheDocument();
const avatarButton = getByRole("button", { name: "Avatar of Peter Fox" });
expect(avatarButton).toBeInTheDocument();
});
it("renders a file as the avatar when supplied", async () => {
@@ -44,12 +44,13 @@ describe("<AvatarSetting />", () => {
<AvatarSetting
placeholderId="blee"
placeholderName="boo"
avatarAltText="Avatar of Peter Fox"
avatarAccessibleName="Avatar of Peter Fox"
avatar={AVATAR_FILE}
/>,
);
const imgElement = await screen.findByRole("button", { name: "Avatar of Peter Fox" });
const avatarButton = await screen.findByRole("button", { name: "Avatar of Peter Fox" });
const imgElement = avatarButton.querySelector("img");
expect(imgElement).toBeInTheDocument();
expect(imgElement).toHaveAttribute("src", "data:image/gif;base64," + BASE64_GIF);
});
@@ -63,7 +64,7 @@ describe("<AvatarSetting />", () => {
placeholderId="blee"
placeholderName="boo"
avatar="mxc://example.org/my-avatar"
avatarAltText="Avatar of Peter Fox"
avatarAccessibleName="Avatar of Peter Fox"
onChange={onChange}
/>,
);
@@ -82,7 +83,7 @@ describe("<AvatarSetting />", () => {
placeholderId="blee"
placeholderName="boo"
avatar="mxc://example.org/my-avatar"
avatarAltText="Avatar of Peter Fox"
avatarAccessibleName="Avatar of Peter Fox"
onChange={onChange}
/>,
);
@@ -102,7 +103,7 @@ describe("<AvatarSetting />", () => {
placeholderId="blee"
placeholderName="boo"
avatar="mxc://example.org/my-avatar"
avatarAltText="Avatar of Peter Fox"
avatarAccessibleName="Avatar of Peter Fox"
onChange={onChange}
/>,
);