Merge pull request #3019 from matrix-org/travis/sr/fix-timeline

Make the timeline less noisy for screen readers (mk II)
pull/21833/head
Travis Ralston 2019-05-22 19:18:40 -06:00 committed by GitHub
commit 9a1a9825b0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 77 additions and 11 deletions

View File

@ -67,8 +67,8 @@ export default function AccessibleButton(props) {
restProps.ref = restProps.inputRef;
delete restProps.inputRef;
restProps.tabIndex = restProps.tabIndex || "0";
restProps.role = "button";
restProps.tabIndex = restProps.tabIndex === undefined ? "0" : restProps.tabIndex;
restProps.role = restProps.role === undefined ? "button" : restProps.role;
restProps.className = (restProps.className ? restProps.className + " " : "") +
"mx_AccessibleButton";

View File

@ -45,12 +45,18 @@ class FlairAvatar extends React.Component {
const tooltip = this.props.groupProfile.name ?
`${this.props.groupProfile.name} (${this.props.groupProfile.groupId})`:
this.props.groupProfile.groupId;
// Note: we hide flair from screen readers but ideally we'd support
// reading something out on hover. There's no easy way to do this though,
// so instead we just hide it completely.
return <img
src={httpUrl}
width="16"
height="16"
onClick={this.onClick}
title={tooltip} />;
title={tooltip}
aria-hidden={true}
/>;
}
}

View File

@ -23,12 +23,17 @@ export default class MessageTimestamp extends React.Component {
static propTypes = {
ts: PropTypes.number.isRequired,
showTwelveHour: PropTypes.bool,
ariaHidden: PropTypes.bool,
};
render() {
const date = new Date(this.props.ts);
return (
<span className="mx_MessageTimestamp" title={formatFullDate(date, this.props.showTwelveHour)}>
<span
className="mx_MessageTimestamp"
title={formatFullDate(date, this.props.showTwelveHour)}
aria-hidden={this.props.ariaHidden}
>
{ formatTime(date, this.props.showTwelveHour) }
</span>
);

View File

@ -32,6 +32,7 @@ import withMatrixClient from '../../../wrappers/withMatrixClient';
import dis from '../../../dispatcher';
import SettingsStore from "../../../settings/SettingsStore";
import {EventStatus} from 'matrix-js-sdk';
import MatrixClientPeg from "../../../MatrixClientPeg";
const ObjectUtils = require('../../../ObjectUtils');
@ -545,6 +546,50 @@ module.exports = withMatrixClient(React.createClass({
const isRedacted = isMessageEvent(this.props.mxEvent) && this.props.isRedacted;
const isEncryptionFailure = this.props.mxEvent.isDecryptionFailure();
// TLDR: Screen readers are complicated and can watch for new DOM elements, but not
// changes to DOM elements. As such, we hack a bunch of conditions together.
//
// Screen readers do not react well to aria attributes changing dynamically after
// parsing them. Although readers watch the DOM, they cannot react to aria-hidden
// going from true to false. To work around that, we check to see if the eventSendStatus
// is something worthwhile for us to read out. We specifically don't want to read
// out pending/queued messages because they'll be read out again when they are sent.
//
// There's a small annoyance with doing this though: if we can't change the aria attrs,
// we need to track the entry state for when the component mounts. As it stands, the
// EventTile is unmounted/mounted when going pending->sent, and then a simple properties
// change is made to mxEvent for sent->null (the final state). We abuse this cycle to
// mute the pending state and react on the sent state.
//
// However there's then a bug where readers don't read messages from other people (they
// enter the component as eventSendStatus of null) - to counteract this, we look for a
// transaction_id under the unsigned object of the event. According to the spec, we can
// use this to determine if an event was sent by us (as it's bound to the access token
// which sent the event). This allows us to do a few checks on whether to speak:
// * If the event was sent by our user ID and the eventSendStatus is 'sent', then speak.
// We cannot check the transaction_id at this point because it is undefined. We can
// make the assumption that 'sent' means this exact device is handling it though.
// * If the event was sent by our user ID and the eventSendStatus is falsey (null), then
// only speak if the event was not sent by us (no transaction_id).
// * If the event was not sent by our user ID then speak.
//
// Note: although NVDA (a screen reader) does react to aria-hidden changing, it does so
// in a horrible way. Because multiple properties and DOM elements are changing, it reads
// the message twice when we limit the 'should speak' checks to just 'if eventSendStatus
// is null'. This is part of the reason for the complexity above.
//
// Hopefully all of that leads to us not reading out messages in duplicate or triplicate.
const sentByMyUserId = this.props.mxEvent.getSender() === MatrixClientPeg.get().getUserId();
const sentByThisDevice = !!this.props.mxEvent.getUnsigned()["transaction_id"];
let screenReaderShouldSpeak = false;
if (!isSending) {
if (this.props.eventSendStatus === 'sent') {
screenReaderShouldSpeak = sentByMyUserId;
} else if (!this.props.eventSendStatus) {
screenReaderShouldSpeak = !sentByMyUserId || !sentByThisDevice;
}
}
const classes = classNames({
mx_EventTile: true,
mx_EventTile_isEditing: this.props.isEditing,
@ -601,9 +646,13 @@ module.exports = withMatrixClient(React.createClass({
if (this.props.mxEvent.sender && avatarSize) {
avatar = (
<div className="mx_EventTile_avatar">
<MemberAvatar member={this.props.mxEvent.sender}
<MemberAvatar
member={this.props.mxEvent.sender}
width={avatarSize} height={avatarSize}
viewUserOnClick={true}
aria-hidden={true} /* silence screen readers */
buttonRole={null} /* trick screen readers into thinking this is not a button */
tabIndex={null} /* trick screen readers into thinking this is not a button */
/>
</div>
);
@ -634,8 +683,12 @@ module.exports = withMatrixClient(React.createClass({
onFocusChange={this.onActionBarFocusChange}
/> : undefined;
const timestamp = this.props.mxEvent.getTs() ?
<MessageTimestamp showTwelveHour={this.props.isTwelveHour} ts={this.props.mxEvent.getTs()} /> : null;
const timestamp = this.props.mxEvent.getTs()
? <MessageTimestamp
showTwelveHour={this.props.isTwelveHour}
ts={this.props.mxEvent.getTs()}
ariaHidden={!screenReaderShouldSpeak}
/> : null;
const keyRequestHelpText =
<div className="mx_EventTile_keyRequestInfo_tooltip_contents">
@ -773,13 +826,13 @@ module.exports = withMatrixClient(React.createClass({
'replyThread',
);
return (
<div className={classes}>
<div className={classes} aria-hidden={!screenReaderShouldSpeak}>
<div className="mx_EventTile_msgOption">
{ readAvatars }
</div>
{ sender }
<div className="mx_EventTile_line">
<a href={permalink} onClick={this.onPermalinkClicked}>
<a href={permalink} onClick={this.onPermalinkClicked} aria-hidden={true}>
{ timestamp }
</a>
{ this._renderE2EPadlock() }
@ -797,7 +850,7 @@ module.exports = withMatrixClient(React.createClass({
{ actionBar }
</div>
{
// The avatar goes after the event tile as it's absolutly positioned to be over the
// The avatar goes after the event tile as it's absolutely positioned to be over the
// event tile line, so needs to be later in the DOM so it appears on top (this avoids
// the need for further z-indexing chaos)
}

View File

@ -211,11 +211,13 @@ module.exports = React.createClass({
<MemberAvatar
member={this.props.member}
fallbackUserId={this.props.fallbackUserId}
aria-hidden="true"
width={14} height={14} resizeMethod="crop"
style={style}
title={title}
onClick={this.props.onClick}
aria-hidden={true} /* silence screen readers */
buttonRole={null} /* trick screen readers into thinking this is not a button */
tabIndex={null} /* trick screen readers into thinking this is not a button */
/>
</Velociraptor>
);