From f0d67e0454f5b10834f598335e5859df83b87dff Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Tue, 28 May 2019 12:19:35 +0100 Subject: [PATCH] Revert "Merge pull request #3019 from matrix-org/travis/sr/fix-timeline" This reverts commit 9a1a9825b0d2a2c8065253b42e9d4ccd919122ef, reversing changes made to 62dc83310a83d42a56d95206b9461e883f3e03e2. --- .../views/elements/AccessibleButton.js | 4 +- src/components/views/elements/Flair.js | 8 +-- .../views/messages/MessageTimestamp.js | 7 +-- src/components/views/rooms/EventTile.js | 63 ++----------------- .../views/rooms/ReadReceiptMarker.js | 4 +- 5 files changed, 10 insertions(+), 76 deletions(-) diff --git a/src/components/views/elements/AccessibleButton.js b/src/components/views/elements/AccessibleButton.js index 19150682f0..06c440c54e 100644 --- a/src/components/views/elements/AccessibleButton.js +++ b/src/components/views/elements/AccessibleButton.js @@ -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"; diff --git a/src/components/views/elements/Flair.js b/src/components/views/elements/Flair.js index 7d3d298804..aa629794ba 100644 --- a/src/components/views/elements/Flair.js +++ b/src/components/views/elements/Flair.js @@ -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 ; + title={tooltip} />; } } diff --git a/src/components/views/messages/MessageTimestamp.js b/src/components/views/messages/MessageTimestamp.js index 5bfdc1bc26..0bbb3f631e 100644 --- a/src/components/views/messages/MessageTimestamp.js +++ b/src/components/views/messages/MessageTimestamp.js @@ -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 ( - + { formatTime(date, this.props.showTwelveHour) } ); diff --git a/src/components/views/rooms/EventTile.js b/src/components/views/rooms/EventTile.js index 3c7f3deec9..78e98a8133 100644 --- a/src/components/views/rooms/EventTile.js +++ b/src/components/views/rooms/EventTile.js @@ -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'); @@ -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, @@ -646,13 +601,9 @@ module.exports = withMatrixClient(React.createClass({ if (this.props.mxEvent.sender && avatarSize) { avatar = (
-
); @@ -683,12 +634,8 @@ module.exports = withMatrixClient(React.createClass({ onFocusChange={this.onActionBarFocusChange} /> : undefined; - const timestamp = this.props.mxEvent.getTs() - ? : null; + const timestamp = this.props.mxEvent.getTs() ? + : null; const keyRequestHelpText =
@@ -826,13 +773,13 @@ module.exports = withMatrixClient(React.createClass({ 'replyThread', ); return ( -
+
{ readAvatars }
{ sender }
- + { timestamp } { this._renderE2EPadlock() } diff --git a/src/components/views/rooms/ReadReceiptMarker.js b/src/components/views/rooms/ReadReceiptMarker.js index 4025a36831..2f7a599d95 100644 --- a/src/components/views/rooms/ReadReceiptMarker.js +++ b/src/components/views/rooms/ReadReceiptMarker.js @@ -211,13 +211,11 @@ module.exports = React.createClass({