Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Accessibility: Improve Voiceover #12189

Closed
wants to merge 14 commits into from
2 changes: 2 additions & 0 deletions src/components/views/avatars/MemberAvatar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ function MemberAvatar(
return (
<BaseAvatar
{...props}
tabIndex={-1}
size={size}
name={name ?? ""}
title={hideTitle ? undefined : title}
Expand All @@ -102,6 +103,7 @@ function MemberAvatar(
}
: props.onClick
}
aria-label={name ? name + ". " : ""} // Full stop adds a pause between the name and the appended message.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment doesn't make much sense, and points out that not all consumers of MemberAvatar may have any appended message so maybe another solution is desired which is more respective of the plethora of places a MemberAvatar may be used

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tried something in 90ea6c4

Basically the aria-label for the component itself is just the name (without dot).
In EventTile, we override the accessible name of the DIV that contains the avatar to be name + dot.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That won't work because giving a div both a role and a negative tab index would be yet another accessibility violation. I've added a prop to conditionally add the . to the aria label.
I'm not entirely convinced that this is really useful. Testing with Orca, I don't really see a difference with/without the dot.

altText={_t("common|user_avatar")}
ref={ref}
/>
Expand Down
7 changes: 4 additions & 3 deletions src/components/views/messages/TextualBody.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -623,6 +623,7 @@ export default class TextualBody extends React.Component<IBodyProps, IState> {
}

let widgets;
const id = "mx_EventTile_content_" + mxEvent.getId();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getId() here will actually return the local id but will return the remote id from sync elsewhere:

image

Probably best to use mxEvent.getTxnId() ?? mxEvent.getId()

if (this.state.links.length && !this.state.widgetHidden && this.props.showUrlPreview) {
widgets = (
<LinkPreviewGroup
Expand All @@ -636,7 +637,7 @@ export default class TextualBody extends React.Component<IBodyProps, IState> {

if (isEmote) {
return (
<div className="mx_MEmoteBody mx_EventTile_content" onClick={this.onBodyLinkClick}>
<div className="mx_MEmoteBody mx_EventTile_content" id={id} onClick={this.onBodyLinkClick}>
*&nbsp;
<span className="mx_MEmoteBody_sender" onClick={this.onEmoteSenderClick}>
{mxEvent.sender ? mxEvent.sender.name : mxEvent.getSender()}
Expand All @@ -649,14 +650,14 @@ export default class TextualBody extends React.Component<IBodyProps, IState> {
}
if (isNotice) {
return (
<div className="mx_MNoticeBody mx_EventTile_content" onClick={this.onBodyLinkClick}>
<div className="mx_MNoticeBody mx_EventTile_content" id={id} onClick={this.onBodyLinkClick}>
{body}
{widgets}
</div>
);
}
return (
<div className="mx_MTextBody mx_EventTile_content" onClick={this.onBodyLinkClick}>
<div className="mx_MTextBody mx_EventTile_content" id={id} onClick={this.onBodyLinkClick}>
{body}
{widgets}
</div>
Expand Down
22 changes: 18 additions & 4 deletions src/components/views/rooms/EventTile.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1061,7 +1061,11 @@ export class UnwrappedEventTile extends React.Component<EventTileProps, IState>
this.context.timelineRenderingType,
);
avatar = (
<div className="mx_EventTile_avatar">
<div
className="mx_EventTile_avatar"
id={"mx_EventTile_avatar_" + this.props.mxEvent.getId()}
tabIndex={-1}
>
<MemberAvatar
member={member}
size={avatarSize}
Expand Down Expand Up @@ -1147,6 +1151,7 @@ export class UnwrappedEventTile extends React.Component<EventTileProps, IState>
<a
href={permalink}
onClick={this.onPermalinkClicked}
tabIndex={-1}
aria-label={formatTime(new Date(this.props.mxEvent.getTs()), this.props.isTwelveHour)}
onContextMenu={this.onTimestampContextMenu}
>
Expand Down Expand Up @@ -1209,7 +1214,9 @@ export class UnwrappedEventTile extends React.Component<EventTileProps, IState>
{
"ref": this.ref,
"className": classes,
"tabIndex": 0,
"aria-live": ariaLive,
"aria-labelledby": `mx_EventTile_avatar_${this.props.mxEvent.getId()} mx_EventTile_content_${this.props.mxEvent.getId()}`,
"aria-atomic": true,
"data-scroll-tokens": scrollToken,
"data-has-reply": !!replyChain,
Expand Down Expand Up @@ -1263,8 +1270,9 @@ export class UnwrappedEventTile extends React.Component<EventTileProps, IState>
{
"ref": this.ref,
"className": classes,
"tabIndex": -1,
"tabindex": 0,
"aria-live": ariaLive,
"aria-labelledby": `mx_EventTile_avatar_${this.props.mxEvent.getId()} mx_EventTile_content_${this.props.mxEvent.getId()}`,
"aria-atomic": "true",
"data-scroll-tokens": scrollToken,
"data-layout": this.props.layout,
Expand Down Expand Up @@ -1397,8 +1405,9 @@ export class UnwrappedEventTile extends React.Component<EventTileProps, IState>
{
"ref": this.ref,
"className": classes,
"tabIndex": -1,
"tabindex": 0,
"aria-live": ariaLive,
"aria-labelledby": `mx_EventTile_avatar_${this.props.mxEvent.getId()} mx_EventTile_content_${this.props.mxEvent.getId()}`,
"aria-atomic": "true",
"data-scroll-tokens": scrollToken,
"data-layout": this.props.layout,
Expand All @@ -1413,7 +1422,12 @@ export class UnwrappedEventTile extends React.Component<EventTileProps, IState>
{sender}
{ircPadlock}
{avatar}
<div className={lineClasses} key="mx_EventTile_line" onContextMenu={this.onContextMenu}>
<div
className={lineClasses}
key="mx_EventTile_line"
onContextMenu={this.onContextMenu}
aria-labelledby={"mx_EventTile_content_" + this.props.mxEvent.getId()}
>
{this.renderContextMenu()}
{groupTimestamp}
{groupPadlock}
Expand Down
Loading