ARIA Accessibility improvements (#10675)

* Fix confusing tab indexes in EventTilePreview

* Stop using headings inside buttons

* Prefer labelledby and describedby over duplicated aria-labels

* Improve semantics of tables used in settings

* Fix types

* Update tests

* Fix timestamps
This commit is contained in:
Michael Telatynski 2023-04-21 10:48:48 +01:00 committed by GitHub
parent 259b5fe253
commit 792a39a39b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
21 changed files with 197 additions and 137 deletions

View File

@ -23,15 +23,14 @@ $SpaceRoomViewInnerWidth: 428px;
box-sizing: border-box;
border-radius: 8px;
border: 1px solid $input-border-color;
font-size: $font-15px;
font-size: $font-17px;
font-weight: $font-semi-bold;
margin: 20px 0;
> h3 {
font-weight: $font-semi-bold;
margin: 0 0 4px;
}
> span {
> div {
margin-top: 4px;
font-weight: normal;
font-size: $font-15px;
color: $secondary-content;
}

View File

@ -17,7 +17,12 @@ limitations under the License.
.mx_CrossSigningPanel_statusList {
border-spacing: 0;
td {
th {
text-align: start;
}
td,
th {
padding: 0;
&:first-of-type {

View File

@ -1,3 +1,19 @@
/*
Copyright 2023 The Matrix.org Foundation C.I.C.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
.mx_CryptographyPanel_sessionInfo {
padding: 0em;
border-spacing: 0px;
@ -5,13 +21,15 @@
.mx_CryptographyPanel_sessionInfo > tr {
vertical-align: baseline;
padding: 0em;
}
.mx_CryptographyPanel_sessionInfo > tr > td {
padding-bottom: 0em;
padding-left: 0em;
padding-right: 1em;
padding-top: 0em;
th {
text-align: start;
}
td,
th {
padding: 0 1em 0 0;
}
}
.mx_CryptographyPanel_importExportButtons .mx_AccessibleButton {

View File

@ -50,7 +50,12 @@ limitations under the License.
.mx_SecureBackupPanel_statusList {
border-spacing: 0;
td {
th {
text-align: start;
}
td,
th {
padding: 0;
&:first-of-type {

View File

@ -476,7 +476,7 @@ const SpaceSetupPrivateScope: React.FC<{
onFinished(false);
}}
>
<h3>{_t("Just me")}</h3>
{_t("Just me")}
<div>{_t("A private space to organise your rooms")}</div>
</AccessibleButton>
<AccessibleButton
@ -485,7 +485,7 @@ const SpaceSetupPrivateScope: React.FC<{
onFinished(true);
}}
>
<h3>{_t("Me and my teammates")}</h3>
{_t("Me and my teammates")}
<div>{_t("A private space for you and your teammates")}</div>
</AccessibleButton>
</div>

View File

@ -128,8 +128,8 @@ export default class EventTilePreview extends React.Component<IProps, IState> {
const event = this.fakeEvent(this.state);
return (
<div className={className}>
<EventTile mxEvent={event} layout={this.props.layout} as="div" />
<div className={className} role="presentation">
<EventTile mxEvent={event} layout={this.props.layout} as="div" hideTimestamp inhibitInteraction />
</div>
);
}

View File

@ -16,6 +16,7 @@ limitations under the License.
import React from "react";
import classNames from "classnames";
import { randomString } from "matrix-js-sdk/src/randomstring";
import ToggleSwitch from "./ToggleSwitch";
import { Caption } from "../typography/Caption";
@ -43,18 +44,15 @@ interface IProps {
}
export default class LabelledToggleSwitch extends React.PureComponent<IProps> {
private readonly id = `mx_LabelledToggleSwitch_${randomString(12)}`;
public render(): React.ReactNode {
// This is a minimal version of a SettingsFlag
const { label, caption } = this.props;
let firstPart = (
<span className="mx_SettingsFlag_label">
{label}
{caption && (
<>
<br />
<Caption>{caption}</Caption>
</>
)}
<div id={this.id}>{label}</div>
{caption && <Caption id={`${this.id}_caption`}>{caption}</Caption>}
</span>
);
let secondPart = (
@ -62,15 +60,14 @@ export default class LabelledToggleSwitch extends React.PureComponent<IProps> {
checked={this.props.value}
disabled={this.props.disabled}
onChange={this.props.onChange}
title={this.props.label}
tooltip={this.props.tooltip}
aria-labelledby={this.id}
aria-describedby={caption ? `${this.id}_caption` : undefined}
/>
);
if (this.props.toggleInFront) {
const temp = firstPart;
firstPart = secondPart;
secondPart = temp;
[firstPart, secondPart] = [secondPart, firstPart];
}
const classes = classNames("mx_SettingsFlag", this.props.className, {

View File

@ -41,7 +41,7 @@ interface IProps {
}
// Controlled Toggle Switch element, written with Accessibility in mind
export default ({ checked, disabled = false, title, tooltip, onChange, ...props }: IProps): JSX.Element => {
export default ({ checked, disabled = false, onChange, ...props }: IProps): JSX.Element => {
const _onClick = (): void => {
if (disabled) return;
onChange(!checked);
@ -61,8 +61,6 @@ export default ({ checked, disabled = false, title, tooltip, onChange, ...props
role="switch"
aria-checked={checked}
aria-disabled={disabled}
title={title}
tooltip={tooltip}
>
<div className="mx_ToggleSwitch_ball" />
</AccessibleTooltipButton>

View File

@ -218,6 +218,10 @@ export interface EventTileProps {
// displayed to the current user either because they're
// the author or they are a moderator
isSeeingThroughMessageHiddenForModeration?: boolean;
// The following properties are used by EventTilePreview to disable tab indexes within the event tile
hideTimestamp?: boolean;
inhibitInteraction?: boolean;
}
interface IState {
@ -1006,7 +1010,7 @@ export class UnwrappedEventTile extends React.Component<EventTileProps, IState>
}
if (this.props.mxEvent.sender && avatarSize) {
let member;
let member: RoomMember | null = null;
// set member to receiver (target) if it is a 3PID invite
// so that the correct avatar is shown as the text is
// `$target accepted the invitation for $email`
@ -1016,9 +1020,11 @@ export class UnwrappedEventTile extends React.Component<EventTileProps, IState>
member = this.props.mxEvent.sender;
}
// In the ThreadsList view we use the entire EventTile as a click target to open the thread instead
const viewUserOnClick = ![TimelineRenderingType.ThreadsList, TimelineRenderingType.Notification].includes(
this.context.timelineRenderingType,
);
const viewUserOnClick =
!this.props.inhibitInteraction &&
![TimelineRenderingType.ThreadsList, TimelineRenderingType.Notification].includes(
this.context.timelineRenderingType,
);
avatar = (
<div className="mx_EventTile_avatar">
<MemberAvatar
@ -1064,6 +1070,7 @@ export class UnwrappedEventTile extends React.Component<EventTileProps, IState>
const showTimestamp =
this.props.mxEvent.getTs() &&
!this.props.hideTimestamp &&
(this.props.alwaysShowTimestamps ||
this.props.last ||
this.state.hover ||
@ -1101,7 +1108,7 @@ export class UnwrappedEventTile extends React.Component<EventTileProps, IState>
);
}
const linkedTimestamp = (
const linkedTimestamp = !this.props.hideTimestamp ? (
<a
href={permalink}
onClick={this.onPermalinkClicked}
@ -1110,7 +1117,7 @@ export class UnwrappedEventTile extends React.Component<EventTileProps, IState>
>
{timestamp}
</a>
);
) : null;
const useIRCLayout = this.props.layout === Layout.IRC;
const groupTimestamp = !useIRCLayout ? linkedTimestamp : null;

View File

@ -243,36 +243,34 @@ export default class CrossSigningPanel extends React.PureComponent<{}, IState> {
<details>
<summary>{_t("Advanced")}</summary>
<table className="mx_CrossSigningPanel_statusList">
<tbody>
<tr>
<td>{_t("Cross-signing public keys:")}</td>
<td>{crossSigningPublicKeysOnDevice ? _t("in memory") : _t("not found")}</td>
</tr>
<tr>
<td>{_t("Cross-signing private keys:")}</td>
<td>
{crossSigningPrivateKeysInStorage
? _t("in secret storage")
: _t("not found in storage")}
</td>
</tr>
<tr>
<td>{_t("Master private key:")}</td>
<td>{masterPrivateKeyCached ? _t("cached locally") : _t("not found locally")}</td>
</tr>
<tr>
<td>{_t("Self signing private key:")}</td>
<td>{selfSigningPrivateKeyCached ? _t("cached locally") : _t("not found locally")}</td>
</tr>
<tr>
<td>{_t("User signing private key:")}</td>
<td>{userSigningPrivateKeyCached ? _t("cached locally") : _t("not found locally")}</td>
</tr>
<tr>
<td>{_t("Homeserver feature support:")}</td>
<td>{homeserverSupportsCrossSigning ? _t("exists") : _t("not found")}</td>
</tr>
</tbody>
<tr>
<th scope="row">{_t("Cross-signing public keys:")}</th>
<td>{crossSigningPublicKeysOnDevice ? _t("in memory") : _t("not found")}</td>
</tr>
<tr>
<th scope="row">{_t("Cross-signing private keys:")}</th>
<td>
{crossSigningPrivateKeysInStorage
? _t("in secret storage")
: _t("not found in storage")}
</td>
</tr>
<tr>
<th scope="row">{_t("Master private key:")}</th>
<td>{masterPrivateKeyCached ? _t("cached locally") : _t("not found locally")}</td>
</tr>
<tr>
<th scope="row">{_t("Self signing private key:")}</th>
<td>{selfSigningPrivateKeyCached ? _t("cached locally") : _t("not found locally")}</td>
</tr>
<tr>
<th scope="row">{_t("User signing private key:")}</th>
<td>{userSigningPrivateKeyCached ? _t("cached locally") : _t("not found locally")}</td>
</tr>
<tr>
<th scope="row">{_t("Homeserver feature support:")}</th>
<td>{homeserverSupportsCrossSigning ? _t("exists") : _t("not found")}</td>
</tr>
</table>
</details>
{errorSection}

View File

@ -75,22 +75,20 @@ export default class CryptographyPanel extends React.Component<IProps, IState> {
<div className="mx_SettingsTab_section mx_CryptographyPanel">
<span className="mx_SettingsTab_subheading">{_t("Cryptography")}</span>
<table className="mx_SettingsTab_subsectionText mx_CryptographyPanel_sessionInfo">
<tbody>
<tr>
<td>{_t("Session ID:")}</td>
<td>
<code>{deviceId}</code>
</td>
</tr>
<tr>
<td>{_t("Session key:")}</td>
<td>
<code>
<b>{identityKey}</b>
</code>
</td>
</tr>
</tbody>
<tr>
<th scope="row">{_t("Session ID:")}</th>
<td>
<code>{deviceId}</code>
</td>
</tr>
<tr>
<th scope="row">{_t("Session key:")}</th>
<td>
<code>
<b>{identityKey}</b>
</code>
</td>
</tr>
</table>
{importExportButtons}
{noSendUnverifiedSetting}

View File

@ -15,7 +15,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
import React from "react";
import React, { ReactNode } from "react";
import { IKeyBackupInfo } from "matrix-js-sdk/src/crypto/keybackup";
import { TrustInfo } from "matrix-js-sdk/src/crypto/backup";
import { CryptoEvent } from "matrix-js-sdk/src/crypto";
@ -231,9 +231,9 @@ export default class SecureBackupPanel extends React.PureComponent<{}, IState> {
sessionsRemaining,
} = this.state;
let statusDescription;
let extraDetailsTableRows;
let extraDetails;
let statusDescription: JSX.Element;
let extraDetailsTableRows: JSX.Element | undefined;
let extraDetails: JSX.Element | undefined;
const actions: JSX.Element[] = [];
if (error) {
statusDescription = <div className="error">{_t("Unable to load key backup status")}</div>;
@ -267,7 +267,7 @@ export default class SecureBackupPanel extends React.PureComponent<{}, IState> {
restoreButtonCaption = _t("Connect this session to Key Backup");
}
let uploadStatus;
let uploadStatus: ReactNode;
if (!MatrixClientPeg.get().getKeyBackupEnabled()) {
// No upload status to show when backup disabled.
uploadStatus = "";
@ -391,11 +391,11 @@ export default class SecureBackupPanel extends React.PureComponent<{}, IState> {
extraDetailsTableRows = (
<>
<tr>
<td>{_t("Backup version:")}</td>
<th scope="row">{_t("Backup version:")}</th>
<td>{backupInfo.version}</td>
</tr>
<tr>
<td>{_t("Algorithm:")}</td>
<th scope="row">{_t("Algorithm:")}</th>
<td>{backupInfo.algorithm}</td>
</tr>
</>
@ -460,7 +460,7 @@ export default class SecureBackupPanel extends React.PureComponent<{}, IState> {
}
}
let actionRow;
let actionRow: JSX.Element | undefined;
if (actions.length) {
actionRow = <div className="mx_SecureBackupPanel_buttonRow">{actions}</div>;
}
@ -478,28 +478,26 @@ export default class SecureBackupPanel extends React.PureComponent<{}, IState> {
<details>
<summary>{_t("Advanced")}</summary>
<table className="mx_SecureBackupPanel_statusList">
<tbody>
<tr>
<td>{_t("Backup key stored:")}</td>
<td>{backupKeyStored === true ? _t("in secret storage") : _t("not stored")}</td>
</tr>
<tr>
<td>{_t("Backup key cached:")}</td>
<td>
{backupKeyCached ? _t("cached locally") : _t("not found locally")}
{backupKeyWellFormedText}
</td>
</tr>
<tr>
<td>{_t("Secret storage public key:")}</td>
<td>{secretStorageKeyInAccount ? _t("in account data") : _t("not found")}</td>
</tr>
<tr>
<td>{_t("Secret storage:")}</td>
<td>{secretStorageReady ? _t("ready") : _t("not ready")}</td>
</tr>
{extraDetailsTableRows}
</tbody>
<tr>
<th scope="row">{_t("Backup key stored:")}</th>
<td>{backupKeyStored === true ? _t("in secret storage") : _t("not stored")}</td>
</tr>
<tr>
<th scope="row">{_t("Backup key cached:")}</th>
<td>
{backupKeyCached ? _t("cached locally") : _t("not found locally")}
{backupKeyWellFormedText}
</td>
</tr>
<tr>
<th scope="row">{_t("Secret storage public key:")}</th>
<td>{secretStorageKeyInAccount ? _t("in account data") : _t("not found")}</td>
</tr>
<tr>
<th scope="row">{_t("Secret storage:")}</th>
<td>{secretStorageReady ? _t("ready") : _t("not ready")}</td>
</tr>
{extraDetailsTableRows}
</table>
{extraDetails}
</details>

View File

@ -89,8 +89,8 @@ const SpaceCreateMenuType: React.FC<{
}> = ({ title, description, className, onClick }) => {
return (
<AccessibleButton className={classNames("mx_SpaceCreateMenuType", className)} onClick={onClick}>
<h3>{title}</h3>
<span>{description}</span>
{title}
<div>{description}</div>
</AccessibleButton>
);
};

View File

@ -52,7 +52,7 @@ const SpacePublicShare: React.FC<IProps> = ({ space, onFinished }) => {
}
}}
>
<h3>{_t("Share invite link")}</h3>
{_t("Share invite link")}
<span>{copiedText}</span>
</AccessibleButton>
{space.canInvite(MatrixClientPeg.get()?.getUserId()) && shouldShowComponent(UIComponent.InviteUsers) ? (
@ -63,8 +63,8 @@ const SpacePublicShare: React.FC<IProps> = ({ space, onFinished }) => {
showRoomInviteDialog(space.roomId);
}}
>
<h3>{_t("Invite people")}</h3>
<span>{_t("Invite with email or username")}</span>
{_t("Invite people")}
<div>{_t("Invite with email or username")}</div>
</AccessibleButton>
) : null}
</div>

View File

@ -32,11 +32,9 @@ import DMRoomMap from "../../../src/utils/DMRoomMap";
import SettingsStore from "../../../src/settings/SettingsStore";
// Fake random strings to give a predictable snapshot for checkbox IDs
jest.mock("matrix-js-sdk/src/randomstring", () => {
return {
randomString: () => "abdefghi",
};
});
jest.mock("matrix-js-sdk/src/randomstring", () => ({
randomString: () => "abdefghi",
}));
describe("SpaceHierarchy", () => {
describe("showRoom", () => {

View File

@ -72,6 +72,11 @@ jest.mock("../../../../src/Modal", () => ({
ModalManagerEvent: { Opened: "opened" },
}));
// Fake random strings to give a predictable snapshot for IDs
jest.mock("matrix-js-sdk/src/randomstring", () => ({
randomString: () => "abdefghi",
}));
describe("<LocationShareMenu />", () => {
const userId = "@ernie:server.org";
const mockClient = getMockClientWithEventEmitter({

View File

@ -25,12 +25,16 @@ exports[`<LocationShareMenu /> with live location disabled goes to labs flag scr
<span
class="mx_SettingsFlag_label"
>
Enable live location sharing
<div
id="mx_LabelledToggleSwitch_abdefghi"
>
Enable live location sharing
</div>
</span>
<div
aria-checked="false"
aria-disabled="false"
aria-label="Enable live location sharing"
aria-labelledby="mx_LabelledToggleSwitch_abdefghi"
class="mx_AccessibleButton mx_ToggleSwitch mx_ToggleSwitch_enabled"
role="switch"
tabindex="0"

View File

@ -27,8 +27,10 @@ import {
ConditionKind,
IPushRuleCondition,
} from "matrix-js-sdk/src/matrix";
import { randomString } from "matrix-js-sdk/src/randomstring";
import { IThreepid, ThreepidMedium } from "matrix-js-sdk/src/@types/threepids";
import { act, fireEvent, getByTestId, render, screen, waitFor, within } from "@testing-library/react";
import { mocked } from "jest-mock";
import Notifications from "../../../../src/components/views/settings/Notifications";
import SettingsStore from "../../../../src/settings/SettingsStore";
@ -41,6 +43,11 @@ jest.mock("matrix-js-sdk/src/logger");
// Avoid indirectly importing any eagerly created stores that would require extra setup
jest.mock("../../../../src/Notifier");
// Fake random strings to give a predictable snapshot for IDs
jest.mock("matrix-js-sdk/src/randomstring", () => ({
randomString: jest.fn(),
}));
const masterRule: IPushRule = {
actions: [PushRuleActionName.DontNotify],
conditions: [],
@ -271,6 +278,11 @@ describe("<Notifications />", () => {
mockClient.getPushRules.mockResolvedValue(pushRules);
beforeEach(() => {
let i = 0;
mocked(randomString).mockImplementation(() => {
return "testid_" + i++;
});
mockClient.getPushRules.mockClear().mockResolvedValue(pushRules);
mockClient.getPushers.mockClear().mockResolvedValue({ pushers: [] });
mockClient.getThreePids.mockClear().mockResolvedValue({ threepids: [] });

View File

@ -12,18 +12,23 @@ exports[`<Notifications /> main notification switches renders only enable notifi
<span
class="mx_SettingsFlag_label"
>
Enable notifications for this account
<br />
<div
id="mx_LabelledToggleSwitch_testid_0"
>
Enable notifications for this account
</div>
<span
class="mx_Caption"
id="mx_LabelledToggleSwitch_testid_0_caption"
>
Turn off to disable notifications on all your devices and sessions
</span>
</span>
<div
aria-checked="false"
aria-describedby="mx_LabelledToggleSwitch_testid_0_caption"
aria-disabled="false"
aria-label="Enable notifications for this account"
aria-labelledby="mx_LabelledToggleSwitch_testid_0"
class="mx_AccessibleButton mx_ToggleSwitch mx_ToggleSwitch_enabled"
role="switch"
tabindex="0"

View File

@ -16,6 +16,7 @@ limitations under the License.
import React from "react";
import { mocked } from "jest-mock";
import { randomString } from "matrix-js-sdk/src/randomstring";
import { act, fireEvent, render, RenderResult } from "@testing-library/react";
import { EventType, MatrixClient, Room } from "matrix-js-sdk/src/matrix";
import { GuestAccess, HistoryVisibility, JoinRule } from "matrix-js-sdk/src/@types/partials";
@ -27,6 +28,11 @@ import { MatrixClientPeg } from "../../../../src/MatrixClientPeg";
const SpaceSettingsVisibilityTab = wrapInMatrixClientContext(_SpaceSettingsVisibilityTab);
// Fake random strings to give a predictable snapshot for IDs
jest.mock("matrix-js-sdk/src/randomstring", () => ({
randomString: jest.fn(),
}));
jest.useFakeTimers();
describe("<SpaceSettingsVisibilityTab />", () => {
@ -89,13 +95,16 @@ describe("<SpaceSettingsVisibilityTab />", () => {
const toggleButton = getByTestId("toggle-guest-access-btn")!;
fireEvent.click(toggleButton);
};
const getGuestAccessToggle = ({ container }: RenderResult) =>
container.querySelector('[aria-label="Enable guest access"]');
const getHistoryVisibilityToggle = ({ container }: RenderResult) =>
container.querySelector('[aria-label="Preview Space"]');
const getGuestAccessToggle = ({ getByLabelText }: RenderResult) => getByLabelText("Enable guest access");
const getHistoryVisibilityToggle = ({ getByLabelText }: RenderResult) => getByLabelText("Preview Space");
const getErrorMessage = ({ getByTestId }: RenderResult) => getByTestId("space-settings-error")?.textContent;
beforeEach(() => {
let i = 0;
mocked(randomString).mockImplementation(() => {
return "testid_" + i++;
});
(mockMatrixClient.sendStateEvent as jest.Mock).mockClear().mockResolvedValue({});
MatrixClientPeg.get = jest.fn().mockReturnValue(mockMatrixClient);
});

View File

@ -4,7 +4,7 @@ exports[`<SpaceSettingsVisibilityTab /> for a public space Access renders guest
<div
aria-checked="true"
aria-disabled="false"
aria-label="Enable guest access"
aria-labelledby="mx_LabelledToggleSwitch_testid_1"
class="mx_AccessibleButton mx_ToggleSwitch mx_ToggleSwitch_on mx_ToggleSwitch_enabled"
role="switch"
tabindex="0"
@ -105,12 +105,16 @@ exports[`<SpaceSettingsVisibilityTab /> renders container 1`] = `
<span
class="mx_SettingsFlag_label"
>
Preview Space
<div
id="mx_LabelledToggleSwitch_testid_0"
>
Preview Space
</div>
</span>
<div
aria-checked="true"
aria-disabled="false"
aria-label="Preview Space"
aria-labelledby="mx_LabelledToggleSwitch_testid_0"
class="mx_AccessibleButton mx_ToggleSwitch mx_ToggleSwitch_on mx_ToggleSwitch_enabled"
role="switch"
tabindex="0"