From 792a39a39b20a5ce2872cd020e1e7783120aff21 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 21 Apr 2023 10:48:48 +0100 Subject: [PATCH] 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 --- res/css/structures/_SpaceRoomView.pcss | 13 ++--- .../views/settings/_CrossSigningPanel.pcss | 7 ++- .../views/settings/_CryptographyPanel.pcss | 30 ++++++++-- .../views/settings/_SecureBackupPanel.pcss | 7 ++- src/components/structures/SpaceRoomView.tsx | 4 +- .../views/elements/EventTilePreview.tsx | 4 +- .../views/elements/LabelledToggleSwitch.tsx | 19 +++--- .../views/elements/ToggleSwitch.tsx | 4 +- src/components/views/rooms/EventTile.tsx | 19 ++++-- .../views/settings/CrossSigningPanel.tsx | 58 +++++++++---------- .../views/settings/CryptographyPanel.tsx | 30 +++++----- .../views/settings/SecureBackupPanel.tsx | 58 +++++++++---------- .../views/spaces/SpaceCreateMenu.tsx | 4 +- .../views/spaces/SpacePublicShare.tsx | 6 +- .../structures/SpaceHierarchy-test.tsx | 8 +-- .../views/location/LocationShareMenu-test.tsx | 5 ++ .../LocationShareMenu-test.tsx.snap | 8 ++- .../views/settings/Notifications-test.tsx | 12 ++++ .../__snapshots__/Notifications-test.tsx.snap | 11 +++- .../SpaceSettingsVisibilityTab-test.tsx | 17 ++++-- .../SpaceSettingsVisibilityTab-test.tsx.snap | 10 +++- 21 files changed, 197 insertions(+), 137 deletions(-) diff --git a/res/css/structures/_SpaceRoomView.pcss b/res/css/structures/_SpaceRoomView.pcss index 433febae48..6a71a75d95 100644 --- a/res/css/structures/_SpaceRoomView.pcss +++ b/res/css/structures/_SpaceRoomView.pcss @@ -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; } diff --git a/res/css/views/settings/_CrossSigningPanel.pcss b/res/css/views/settings/_CrossSigningPanel.pcss index 12a0e36835..1b5f7d1f74 100644 --- a/res/css/views/settings/_CrossSigningPanel.pcss +++ b/res/css/views/settings/_CrossSigningPanel.pcss @@ -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 { diff --git a/res/css/views/settings/_CryptographyPanel.pcss b/res/css/views/settings/_CryptographyPanel.pcss index 98dab47c59..855949d013 100644 --- a/res/css/views/settings/_CryptographyPanel.pcss +++ b/res/css/views/settings/_CryptographyPanel.pcss @@ -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 { diff --git a/res/css/views/settings/_SecureBackupPanel.pcss b/res/css/views/settings/_SecureBackupPanel.pcss index 86f7b2036d..6dcc8321fd 100644 --- a/res/css/views/settings/_SecureBackupPanel.pcss +++ b/res/css/views/settings/_SecureBackupPanel.pcss @@ -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 { diff --git a/src/components/structures/SpaceRoomView.tsx b/src/components/structures/SpaceRoomView.tsx index b8b020f039..8580691311 100644 --- a/src/components/structures/SpaceRoomView.tsx +++ b/src/components/structures/SpaceRoomView.tsx @@ -476,7 +476,7 @@ const SpaceSetupPrivateScope: React.FC<{ onFinished(false); }} > -

{_t("Just me")}

+ {_t("Just me")}
{_t("A private space to organise your rooms")}
-

{_t("Me and my teammates")}

+ {_t("Me and my teammates")}
{_t("A private space for you and your teammates")}
diff --git a/src/components/views/elements/EventTilePreview.tsx b/src/components/views/elements/EventTilePreview.tsx index eaa41903f7..2648aac6a7 100644 --- a/src/components/views/elements/EventTilePreview.tsx +++ b/src/components/views/elements/EventTilePreview.tsx @@ -128,8 +128,8 @@ export default class EventTilePreview extends React.Component { const event = this.fakeEvent(this.state); return ( -
- +
+
); } diff --git a/src/components/views/elements/LabelledToggleSwitch.tsx b/src/components/views/elements/LabelledToggleSwitch.tsx index 4455b16a9f..2a1540920b 100644 --- a/src/components/views/elements/LabelledToggleSwitch.tsx +++ b/src/components/views/elements/LabelledToggleSwitch.tsx @@ -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 { + 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 = ( - {label} - {caption && ( - <> -
- {caption} - - )} +
{label}
+ {caption && {caption}}
); let secondPart = ( @@ -62,15 +60,14 @@ export default class LabelledToggleSwitch extends React.PureComponent { 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, { diff --git a/src/components/views/elements/ToggleSwitch.tsx b/src/components/views/elements/ToggleSwitch.tsx index f29405ba8d..588374d17b 100644 --- a/src/components/views/elements/ToggleSwitch.tsx +++ b/src/components/views/elements/ToggleSwitch.tsx @@ -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} >
diff --git a/src/components/views/rooms/EventTile.tsx b/src/components/views/rooms/EventTile.tsx index ab3a4493da..fb44993553 100644 --- a/src/components/views/rooms/EventTile.tsx +++ b/src/components/views/rooms/EventTile.tsx @@ -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 } 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 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 = (
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 ); } - const linkedTimestamp = ( + const linkedTimestamp = !this.props.hideTimestamp ? ( > {timestamp} - ); + ) : null; const useIRCLayout = this.props.layout === Layout.IRC; const groupTimestamp = !useIRCLayout ? linkedTimestamp : null; diff --git a/src/components/views/settings/CrossSigningPanel.tsx b/src/components/views/settings/CrossSigningPanel.tsx index e3f62d4ba2..d3926d954f 100644 --- a/src/components/views/settings/CrossSigningPanel.tsx +++ b/src/components/views/settings/CrossSigningPanel.tsx @@ -243,36 +243,34 @@ export default class CrossSigningPanel extends React.PureComponent<{}, IState> {
{_t("Advanced")} - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + +
{_t("Cross-signing public keys:")}{crossSigningPublicKeysOnDevice ? _t("in memory") : _t("not found")}
{_t("Cross-signing private keys:")} - {crossSigningPrivateKeysInStorage - ? _t("in secret storage") - : _t("not found in storage")} -
{_t("Master private key:")}{masterPrivateKeyCached ? _t("cached locally") : _t("not found locally")}
{_t("Self signing private key:")}{selfSigningPrivateKeyCached ? _t("cached locally") : _t("not found locally")}
{_t("User signing private key:")}{userSigningPrivateKeyCached ? _t("cached locally") : _t("not found locally")}
{_t("Homeserver feature support:")}{homeserverSupportsCrossSigning ? _t("exists") : _t("not found")}
{_t("Cross-signing public keys:")}{crossSigningPublicKeysOnDevice ? _t("in memory") : _t("not found")}
{_t("Cross-signing private keys:")} + {crossSigningPrivateKeysInStorage + ? _t("in secret storage") + : _t("not found in storage")} +
{_t("Master private key:")}{masterPrivateKeyCached ? _t("cached locally") : _t("not found locally")}
{_t("Self signing private key:")}{selfSigningPrivateKeyCached ? _t("cached locally") : _t("not found locally")}
{_t("User signing private key:")}{userSigningPrivateKeyCached ? _t("cached locally") : _t("not found locally")}
{_t("Homeserver feature support:")}{homeserverSupportsCrossSigning ? _t("exists") : _t("not found")}
{errorSection} diff --git a/src/components/views/settings/CryptographyPanel.tsx b/src/components/views/settings/CryptographyPanel.tsx index 34b52e405e..79ddad2544 100644 --- a/src/components/views/settings/CryptographyPanel.tsx +++ b/src/components/views/settings/CryptographyPanel.tsx @@ -75,22 +75,20 @@ export default class CryptographyPanel extends React.Component {
{_t("Cryptography")} - - - - - - - - - - + + + + + + + +
{_t("Session ID:")} - {deviceId} -
{_t("Session key:")} - - {identityKey} - -
{_t("Session ID:")} + {deviceId} +
{_t("Session key:")} + + {identityKey} + +
{importExportButtons} {noSendUnverifiedSetting} diff --git a/src/components/views/settings/SecureBackupPanel.tsx b/src/components/views/settings/SecureBackupPanel.tsx index 747378684c..2b19a8af58 100644 --- a/src/components/views/settings/SecureBackupPanel.tsx +++ b/src/components/views/settings/SecureBackupPanel.tsx @@ -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 =
{_t("Unable to load key backup status")}
; @@ -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 = ( <> - {_t("Backup version:")} + {_t("Backup version:")} {backupInfo.version} - {_t("Algorithm:")} + {_t("Algorithm:")} {backupInfo.algorithm} @@ -460,7 +460,7 @@ export default class SecureBackupPanel extends React.PureComponent<{}, IState> { } } - let actionRow; + let actionRow: JSX.Element | undefined; if (actions.length) { actionRow =
{actions}
; } @@ -478,28 +478,26 @@ export default class SecureBackupPanel extends React.PureComponent<{}, IState> {
{_t("Advanced")} - - - - - - - - - - - - - - - - - - {extraDetailsTableRows} - + + + + + + + + + + + + + + + + + {extraDetailsTableRows}
{_t("Backup key stored:")}{backupKeyStored === true ? _t("in secret storage") : _t("not stored")}
{_t("Backup key cached:")} - {backupKeyCached ? _t("cached locally") : _t("not found locally")} - {backupKeyWellFormedText} -
{_t("Secret storage public key:")}{secretStorageKeyInAccount ? _t("in account data") : _t("not found")}
{_t("Secret storage:")}{secretStorageReady ? _t("ready") : _t("not ready")}
{_t("Backup key stored:")}{backupKeyStored === true ? _t("in secret storage") : _t("not stored")}
{_t("Backup key cached:")} + {backupKeyCached ? _t("cached locally") : _t("not found locally")} + {backupKeyWellFormedText} +
{_t("Secret storage public key:")}{secretStorageKeyInAccount ? _t("in account data") : _t("not found")}
{_t("Secret storage:")}{secretStorageReady ? _t("ready") : _t("not ready")}
{extraDetails}
diff --git a/src/components/views/spaces/SpaceCreateMenu.tsx b/src/components/views/spaces/SpaceCreateMenu.tsx index 64fc408b77..ded069778d 100644 --- a/src/components/views/spaces/SpaceCreateMenu.tsx +++ b/src/components/views/spaces/SpaceCreateMenu.tsx @@ -89,8 +89,8 @@ const SpaceCreateMenuType: React.FC<{ }> = ({ title, description, className, onClick }) => { return ( -

{title}

- {description} + {title} +
{description}
); }; diff --git a/src/components/views/spaces/SpacePublicShare.tsx b/src/components/views/spaces/SpacePublicShare.tsx index 85446ab251..68bf940831 100644 --- a/src/components/views/spaces/SpacePublicShare.tsx +++ b/src/components/views/spaces/SpacePublicShare.tsx @@ -52,7 +52,7 @@ const SpacePublicShare: React.FC = ({ space, onFinished }) => { } }} > -

{_t("Share invite link")}

+ {_t("Share invite link")} {copiedText} {space.canInvite(MatrixClientPeg.get()?.getUserId()) && shouldShowComponent(UIComponent.InviteUsers) ? ( @@ -63,8 +63,8 @@ const SpacePublicShare: React.FC = ({ space, onFinished }) => { showRoomInviteDialog(space.roomId); }} > -

{_t("Invite people")}

- {_t("Invite with email or username")} + {_t("Invite people")} +
{_t("Invite with email or username")}
) : null}
diff --git a/test/components/structures/SpaceHierarchy-test.tsx b/test/components/structures/SpaceHierarchy-test.tsx index 5f248140c8..b81a84faca 100644 --- a/test/components/structures/SpaceHierarchy-test.tsx +++ b/test/components/structures/SpaceHierarchy-test.tsx @@ -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", () => { diff --git a/test/components/views/location/LocationShareMenu-test.tsx b/test/components/views/location/LocationShareMenu-test.tsx index 9ee667f319..8ab7b46cbd 100644 --- a/test/components/views/location/LocationShareMenu-test.tsx +++ b/test/components/views/location/LocationShareMenu-test.tsx @@ -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("", () => { const userId = "@ernie:server.org"; const mockClient = getMockClientWithEventEmitter({ diff --git a/test/components/views/location/__snapshots__/LocationShareMenu-test.tsx.snap b/test/components/views/location/__snapshots__/LocationShareMenu-test.tsx.snap index cd492db7a2..e83d959d5c 100644 --- a/test/components/views/location/__snapshots__/LocationShareMenu-test.tsx.snap +++ b/test/components/views/location/__snapshots__/LocationShareMenu-test.tsx.snap @@ -25,12 +25,16 @@ exports[` with live location disabled goes to labs flag scr - Enable live location sharing +
+ Enable live location sharing +
({ + randomString: jest.fn(), +})); + const masterRule: IPushRule = { actions: [PushRuleActionName.DontNotify], conditions: [], @@ -271,6 +278,11 @@ describe("", () => { 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: [] }); diff --git a/test/components/views/settings/__snapshots__/Notifications-test.tsx.snap b/test/components/views/settings/__snapshots__/Notifications-test.tsx.snap index 23fec442b2..6d9c127b5f 100644 --- a/test/components/views/settings/__snapshots__/Notifications-test.tsx.snap +++ b/test/components/views/settings/__snapshots__/Notifications-test.tsx.snap @@ -12,18 +12,23 @@ exports[` main notification switches renders only enable notifi - Enable notifications for this account -
+
+ Enable notifications for this account +
Turn off to disable notifications on all your devices and sessions
({ + randomString: jest.fn(), +})); + jest.useFakeTimers(); describe("", () => { @@ -89,13 +95,16 @@ describe("", () => { 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); }); diff --git a/test/components/views/spaces/__snapshots__/SpaceSettingsVisibilityTab-test.tsx.snap b/test/components/views/spaces/__snapshots__/SpaceSettingsVisibilityTab-test.tsx.snap index 8de0ae2c15..a93fda9d6a 100644 --- a/test/components/views/spaces/__snapshots__/SpaceSettingsVisibilityTab-test.tsx.snap +++ b/test/components/views/spaces/__snapshots__/SpaceSettingsVisibilityTab-test.tsx.snap @@ -4,7 +4,7 @@ exports[` for a public space Access renders guest
renders container 1`] = ` - Preview Space +
+ Preview Space +