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

Commit

Permalink
Merge pull request #3033 from matrix-org/jryans/revert-3019
Browse files Browse the repository at this point in the history
Revert "Make the timeline less noisy for screen readers (mk II) #3019"
  • Loading branch information
jryans authored May 28, 2019
2 parents e1d1c8f + f0d67e0 commit 9914bac
Show file tree
Hide file tree
Showing 5 changed files with 10 additions and 76 deletions.
4 changes: 2 additions & 2 deletions src/components/views/elements/AccessibleButton.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ export default function AccessibleButton(props) {
restProps.ref = restProps.inputRef;
delete restProps.inputRef;

restProps.tabIndex = restProps.tabIndex === undefined ? "0" : restProps.tabIndex;
restProps.role = restProps.role === undefined ? "button" : restProps.role;
restProps.tabIndex = restProps.tabIndex || "0";
restProps.role = "button";
restProps.className = (restProps.className ? restProps.className + " " : "") +
"mx_AccessibleButton";

Expand Down
8 changes: 1 addition & 7 deletions src/components/views/elements/Flair.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,18 +45,12 @@ 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}
aria-hidden={true}
/>;
title={tooltip} />;
}
}

Expand Down
7 changes: 1 addition & 6 deletions src/components/views/messages/MessageTimestamp.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,12 @@ 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)}
aria-hidden={this.props.ariaHidden}
>
<span className="mx_MessageTimestamp" title={formatFullDate(date, this.props.showTwelveHour)}>
{ formatTime(date, this.props.showTwelveHour) }
</span>
);
Expand Down
63 changes: 5 additions & 58 deletions src/components/views/rooms/EventTile.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ 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');

Expand Down Expand Up @@ -546,50 +545,6 @@ 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,
Expand Down Expand Up @@ -646,13 +601,9 @@ 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>
);
Expand Down Expand Up @@ -683,12 +634,8 @@ 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()}
ariaHidden={!screenReaderShouldSpeak}
/> : null;
const timestamp = this.props.mxEvent.getTs() ?
<MessageTimestamp showTwelveHour={this.props.isTwelveHour} ts={this.props.mxEvent.getTs()} /> : null;

const keyRequestHelpText =
<div className="mx_EventTile_keyRequestInfo_tooltip_contents">
Expand Down Expand Up @@ -826,13 +773,13 @@ module.exports = withMatrixClient(React.createClass({
'replyThread',
);
return (
<div className={classes} aria-hidden={!screenReaderShouldSpeak}>
<div className={classes}>
<div className="mx_EventTile_msgOption">
{ readAvatars }
</div>
{ sender }
<div className="mx_EventTile_line">
<a href={permalink} onClick={this.onPermalinkClicked} aria-hidden={true}>
<a href={permalink} onClick={this.onPermalinkClicked}>
{ timestamp }
</a>
{ this._renderE2EPadlock() }
Expand Down
4 changes: 1 addition & 3 deletions src/components/views/rooms/ReadReceiptMarker.js
Original file line number Diff line number Diff line change
Expand Up @@ -211,13 +211,11 @@ 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>
);
Expand Down

0 comments on commit 9914bac

Please sign in to comment.