From cb5c9f2c5ae34e019d34b121e1ee32ce4ab7c6c0 Mon Sep 17 00:00:00 2001 From: Stefan Parviainen Date: Wed, 29 Nov 2017 21:13:48 +0100 Subject: [PATCH 01/19] Make Dialogs more accessible Signed-off-by: Stefan Parviainen --- src/components/views/dialogs/BaseDialog.js | 8 ++++++-- src/components/views/dialogs/QuestionDialog.js | 1 + 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/components/views/dialogs/BaseDialog.js b/src/components/views/dialogs/BaseDialog.js index 295bb21ea1..aec9af4e98 100644 --- a/src/components/views/dialogs/BaseDialog.js +++ b/src/components/views/dialogs/BaseDialog.js @@ -45,6 +45,10 @@ export default React.createClass({ // children should be the content of the dialog children: React.PropTypes.node, + + // Id of content element + // If provided, this is used to add a aria-describedby attribute + contentId: React.PropTypes.string }, _onKeyDown: function(e) { @@ -69,13 +73,13 @@ export default React.createClass({ const TintableSvg = sdk.getComponent("elements.TintableSvg"); return ( -
+
-
+
{ this.props.title }
{ this.props.children } diff --git a/src/components/views/dialogs/QuestionDialog.js b/src/components/views/dialogs/QuestionDialog.js index 339b284e2f..db20fd00ed 100644 --- a/src/components/views/dialogs/QuestionDialog.js +++ b/src/components/views/dialogs/QuestionDialog.js @@ -66,6 +66,7 @@ export default React.createClass({
{ this.props.description } From 437a440bdfa17553e83e5d7e762b7494bb803ddb Mon Sep 17 00:00:00 2001 From: Stefan Parviainen Date: Thu, 30 Nov 2017 08:32:18 +0100 Subject: [PATCH 02/19] Add missing id Signed-off-by: Stefan Parviainen --- src/components/views/dialogs/QuestionDialog.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/dialogs/QuestionDialog.js b/src/components/views/dialogs/QuestionDialog.js index db20fd00ed..92051a0df7 100644 --- a/src/components/views/dialogs/QuestionDialog.js +++ b/src/components/views/dialogs/QuestionDialog.js @@ -68,7 +68,7 @@ export default React.createClass({ title={this.props.title} contentId='mx_Dialog_content' > -
+
{ this.props.description }
From 5ccbcf02e200aa8348a7415dda40a0c73e35937d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20V=C3=A1gner?= Date: Sun, 3 Dec 2017 21:38:21 +0100 Subject: [PATCH 03/19] Several changes improving accessibility of the dialogs - Wrapped all the modals inside a react-focus-trap component disabling keyboard navigation outside the modal dialogs - Disabled our custom key handling at dialog level. Cancelling on esc key is now handled via FocusTrap component. - Removed onEnter prop from the BaseDialog component. Dialogs that submit data all now embed a form with onSubmit handler. And since keyboard focus is now managed better via FocusTrap it no longer makes sense for the other dialog types. Fixes https://github.com/vector-im/riot-web/issues/5736 - Set aria-hidden on the matrixChat outer node when showing dialogs to disable navigating outside the modals by using screen reader specific features. --- package.json | 1 + src/Modal.js | 6 +- src/components/views/dialogs/BaseDialog.js | 26 ++++----- .../views/dialogs/ConfirmUserActionDialog.js | 4 +- .../views/dialogs/CreateGroupDialog.js | 1 - .../views/dialogs/CreateRoomDialog.js | 57 +++++++++---------- .../views/dialogs/QuestionDialog.js | 1 - .../views/dialogs/TextInputDialog.js | 29 +++++----- 8 files changed, 60 insertions(+), 65 deletions(-) diff --git a/package.json b/package.json index b443b4c72a..46517230a7 100644 --- a/package.json +++ b/package.json @@ -78,6 +78,7 @@ "react": "^15.4.0", "react-addons-css-transition-group": "15.3.2", "react-dom": "^15.4.0", + "react-focus-trap": "^2.5.0", "react-gemini-scrollbar": "matrix-org/react-gemini-scrollbar#5e97aef", "sanitize-html": "^1.14.1", "text-encoding-utf-8": "^1.0.1", diff --git a/src/Modal.js b/src/Modal.js index 68d75d1ff1..daf66a37de 100644 --- a/src/Modal.js +++ b/src/Modal.js @@ -19,6 +19,7 @@ limitations under the License. const React = require('react'); const ReactDOM = require('react-dom'); +import FocusTrap from 'react-focus-trap'; import Analytics from './Analytics'; import sdk from './index'; @@ -164,6 +165,7 @@ class ModalManager { ); modal.onFinished = props ? props.onFinished : null; modal.className = className; + modal.closeDialog = closeDialog; this._modals.unshift(modal); @@ -194,9 +196,9 @@ class ModalManager { const modal = this._modals[0]; const dialog = (
-
+ { modal.elem } -
+
); diff --git a/src/components/views/dialogs/BaseDialog.js b/src/components/views/dialogs/BaseDialog.js index aec9af4e98..1f29f2d1f4 100644 --- a/src/components/views/dialogs/BaseDialog.js +++ b/src/components/views/dialogs/BaseDialog.js @@ -33,9 +33,6 @@ export default React.createClass({ // onFinished callback to call when Escape is pressed onFinished: React.PropTypes.func.isRequired, - // callback to call when Enter is pressed - onEnterPressed: React.PropTypes.func, - // CSS class to apply to dialog div className: React.PropTypes.string, @@ -51,17 +48,16 @@ export default React.createClass({ contentId: React.PropTypes.string }, - _onKeyDown: function(e) { - if (e.keyCode === KeyCode.ESCAPE) { - e.stopPropagation(); - e.preventDefault(); - this.props.onFinished(); - } else if (e.keyCode === KeyCode.ENTER) { - if (this.props.onEnterPressed) { - e.stopPropagation(); - e.preventDefault(); - this.props.onEnterPressed(e); - } + componentDidMount: function() { + this.applicationNode = document.getElementById('matrixchat'); + if (this.applicationNode) { + this.applicationNode.setAttribute('aria-hidden', 'true'); + } + }, + + componentWillUnmount: function() { + if (this.applicationNode) { + this.applicationNode.setAttribute('aria-hidden', 'false'); } }, @@ -73,7 +69,7 @@ export default React.createClass({ const TintableSvg = sdk.getComponent("elements.TintableSvg"); return ( -
+
diff --git a/src/components/views/dialogs/ConfirmUserActionDialog.js b/src/components/views/dialogs/ConfirmUserActionDialog.js index 78d084b709..1c246a580b 100644 --- a/src/components/views/dialogs/ConfirmUserActionDialog.js +++ b/src/components/views/dialogs/ConfirmUserActionDialog.js @@ -116,10 +116,10 @@ export default React.createClass({ return ( -
+
{ avatar }
diff --git a/src/components/views/dialogs/CreateGroupDialog.js b/src/components/views/dialogs/CreateGroupDialog.js index 168fe75947..8e262a6e51 100644 --- a/src/components/views/dialogs/CreateGroupDialog.js +++ b/src/components/views/dialogs/CreateGroupDialog.js @@ -116,7 +116,6 @@ export default React.createClass({ return (
diff --git a/src/components/views/dialogs/CreateRoomDialog.js b/src/components/views/dialogs/CreateRoomDialog.js index f7be47b3eb..f5a5f87b72 100644 --- a/src/components/views/dialogs/CreateRoomDialog.js +++ b/src/components/views/dialogs/CreateRoomDialog.js @@ -43,38 +43,37 @@ export default React.createClass({ const BaseDialog = sdk.getComponent('views.dialogs.BaseDialog'); return ( -
-
- -
-
- -
-
- -
- { _t('Advanced options') } -
- - + +
+
+
-
-
-
- - -
+
+ +
+
+ +
+ { _t('Advanced options') } +
+ + +
+
+
+
+ + +
+ ); }, diff --git a/src/components/views/dialogs/QuestionDialog.js b/src/components/views/dialogs/QuestionDialog.js index 92051a0df7..41733470a1 100644 --- a/src/components/views/dialogs/QuestionDialog.js +++ b/src/components/views/dialogs/QuestionDialog.js @@ -64,7 +64,6 @@ export default React.createClass({ }); return ( diff --git a/src/components/views/dialogs/TextInputDialog.js b/src/components/views/dialogs/TextInputDialog.js index 5ea4191e5e..907848b3b8 100644 --- a/src/components/views/dialogs/TextInputDialog.js +++ b/src/components/views/dialogs/TextInputDialog.js @@ -60,25 +60,24 @@ export default React.createClass({ const BaseDialog = sdk.getComponent('views.dialogs.BaseDialog'); return ( -
-
- +
+
+
+ +
+
+ +
-
- +
+ +
-
-
- - -
+
); }, From 4f83f6cf25480776c04329a0463d8f6fe82f48ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20V=C3=A1gner?= Date: Tue, 5 Dec 2017 08:50:40 +0100 Subject: [PATCH 04/19] Move keyboard focus management back to the BaseDialog rather than leaving it in the Modal manager. We are using Modal manager to load other components not just BaseDialog and its subclasses and they might require different keyboard handling. Also depend on focus-trap-react rather than react-focus-trap for locking keyboard focus inside the dialog. The experience is much nicer and even the FocusTrap element it-self no longer gains the focus. On a side note using the FocusTrap element outside the dialog (on its parent) stops it from working properly. --- package.json | 2 +- src/Modal.js | 6 ++---- src/components/views/dialogs/BaseDialog.js | 13 +++++++++++-- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/package.json b/package.json index 46517230a7..ee089daf29 100644 --- a/package.json +++ b/package.json @@ -65,6 +65,7 @@ "file-saver": "^1.3.3", "filesize": "3.5.6", "flux": "2.1.1", + "focus-trap-react": "^3.0.5", "fuse.js": "^2.2.0", "glob": "^5.0.14", "highlight.js": "^8.9.1", @@ -78,7 +79,6 @@ "react": "^15.4.0", "react-addons-css-transition-group": "15.3.2", "react-dom": "^15.4.0", - "react-focus-trap": "^2.5.0", "react-gemini-scrollbar": "matrix-org/react-gemini-scrollbar#5e97aef", "sanitize-html": "^1.14.1", "text-encoding-utf-8": "^1.0.1", diff --git a/src/Modal.js b/src/Modal.js index daf66a37de..68d75d1ff1 100644 --- a/src/Modal.js +++ b/src/Modal.js @@ -19,7 +19,6 @@ limitations under the License. const React = require('react'); const ReactDOM = require('react-dom'); -import FocusTrap from 'react-focus-trap'; import Analytics from './Analytics'; import sdk from './index'; @@ -165,7 +164,6 @@ class ModalManager { ); modal.onFinished = props ? props.onFinished : null; modal.className = className; - modal.closeDialog = closeDialog; this._modals.unshift(modal); @@ -196,9 +194,9 @@ class ModalManager { const modal = this._modals[0]; const dialog = (
- +
{ modal.elem } - +
); diff --git a/src/components/views/dialogs/BaseDialog.js b/src/components/views/dialogs/BaseDialog.js index 1f29f2d1f4..25909a18fa 100644 --- a/src/components/views/dialogs/BaseDialog.js +++ b/src/components/views/dialogs/BaseDialog.js @@ -15,6 +15,7 @@ limitations under the License. */ import React from 'react'; +import FocusTrap from 'focus-trap-react'; import * as KeyCode from '../../../KeyCode'; import AccessibleButton from '../elements/AccessibleButton'; @@ -61,6 +62,14 @@ export default React.createClass({ } }, + _onKeyDown: function(e) { + if (e.keyCode === KeyCode.ESCAPE) { + e.stopPropagation(); + e.preventDefault(); + this.props.onFinished(); + } + }, + _onCancelClick: function(e) { this.props.onFinished(); }, @@ -69,7 +78,7 @@ export default React.createClass({ const TintableSvg = sdk.getComponent("elements.TintableSvg"); return ( -
+ @@ -79,7 +88,7 @@ export default React.createClass({ { this.props.title }
{ this.props.children } -
+ ); }, }); From a31af39ca8af8da06462d6d2cc3dd8d20093ca64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20V=C3=A1gner?= Date: Tue, 5 Dec 2017 13:52:20 +0100 Subject: [PATCH 05/19] Applied aria-describedby to all other dialogs that are using BaseDialog. Also added initial focus where it has not been set. --- .../views/dialogs/ChatCreateOrReuseDialog.js | 11 +++++----- src/components/views/dialogs/ErrorDialog.js | 14 +++++-------- .../views/dialogs/InteractiveAuthDialog.js | 8 +++++--- .../views/dialogs/KeyShareDialog.js | 7 ++++--- .../dialogs/SessionRestoreErrorDialog.js | 16 +++++++++++---- .../views/dialogs/SetEmailDialog.js | 5 ++++- src/components/views/dialogs/SetMxIdDialog.js | 7 ++++--- .../views/dialogs/UnknownDeviceDialog.js | 5 +++-- .../login/InteractiveAuthEntryComponents.js | 20 +++++++++++++------ 9 files changed, 57 insertions(+), 36 deletions(-) diff --git a/src/components/views/dialogs/ChatCreateOrReuseDialog.js b/src/components/views/dialogs/ChatCreateOrReuseDialog.js index e0578f3b53..0623177e1a 100644 --- a/src/components/views/dialogs/ChatCreateOrReuseDialog.js +++ b/src/components/views/dialogs/ChatCreateOrReuseDialog.js @@ -127,7 +127,7 @@ export default class ChatCreateOrReuseDialog extends React.Component {
{ _t("Start new chat") }
; - content =
+ content =
{ _t('You already have existing direct chats with this user:') }
{ this.state.tiles } @@ -144,7 +144,7 @@ export default class ChatCreateOrReuseDialog extends React.Component { if (this.state.busyProfile) { profile = ; } else if (this.state.profileError) { - profile =
+ profile =
Unable to load profile information for { this.props.userId }
; } else { @@ -160,14 +160,14 @@ export default class ChatCreateOrReuseDialog extends React.Component {
; } content =
-
+

{ _t('Click on the button below to start chatting!') }

{ profile }
-
@@ -179,6 +179,7 @@ export default class ChatCreateOrReuseDialog extends React.Component { { content } @@ -186,7 +187,7 @@ export default class ChatCreateOrReuseDialog extends React.Component { } } -ChatCreateOrReuseDialog.propTyps = { +ChatCreateOrReuseDialog.propTypes = { userId: React.PropTypes.string.isRequired, // Called when clicking outside of the dialog onFinished: React.PropTypes.func.isRequired, diff --git a/src/components/views/dialogs/ErrorDialog.js b/src/components/views/dialogs/ErrorDialog.js index 97ed47e10f..0910264cef 100644 --- a/src/components/views/dialogs/ErrorDialog.js +++ b/src/components/views/dialogs/ErrorDialog.js @@ -51,22 +51,18 @@ export default React.createClass({ }; }, - componentDidMount: function() { - if (this.props.focus) { - this.refs.button.focus(); - } - }, - render: function() { const BaseDialog = sdk.getComponent('views.dialogs.BaseDialog'); return ( -
+ title={this.props.title || _t('Error')} + contentId='mx_Dialog_content' + > +
{ this.props.description || _t('An error has occurred.') }
-
diff --git a/src/components/views/dialogs/InteractiveAuthDialog.js b/src/components/views/dialogs/InteractiveAuthDialog.js index 59de7c7f59..d1273849ae 100644 --- a/src/components/views/dialogs/InteractiveAuthDialog.js +++ b/src/components/views/dialogs/InteractiveAuthDialog.js @@ -72,11 +72,12 @@ export default React.createClass({ let content; if (this.state.authError) { content = ( -
-
{ this.state.authError.message || this.state.authError.toString() }
+
+
{ this.state.authError.message || this.state.authError.toString() }

{ _t("Dismiss") } @@ -84,7 +85,7 @@ export default React.createClass({ ); } else { content = ( -
+
{ content } diff --git a/src/components/views/dialogs/KeyShareDialog.js b/src/components/views/dialogs/KeyShareDialog.js index 9c8be27c89..821939ff0d 100644 --- a/src/components/views/dialogs/KeyShareDialog.js +++ b/src/components/views/dialogs/KeyShareDialog.js @@ -125,11 +125,11 @@ export default React.createClass({ text = _t(text, {displayName: displayName}); return ( -
+

{ text }

- diff --git a/src/components/views/dialogs/SetEmailDialog.js b/src/components/views/dialogs/SetEmailDialog.js index 2dd996953d..5a9569c1b5 100644 --- a/src/components/views/dialogs/SetEmailDialog.js +++ b/src/components/views/dialogs/SetEmailDialog.js @@ -41,6 +41,7 @@ export default React.createClass({ }, componentDidMount: function() { + this.refs.emailInputField.focus(); }, onEmailAddressChanged: function(value) { @@ -130,6 +131,7 @@ export default React.createClass({ const emailInput = this.state.emailBusy ? :
-

+

{ _t('This will allow you to reset your password and receive notifications.') }

{ emailInput } diff --git a/src/components/views/dialogs/SetMxIdDialog.js b/src/components/views/dialogs/SetMxIdDialog.js index 6fc1d77682..5ef04f5be1 100644 --- a/src/components/views/dialogs/SetMxIdDialog.js +++ b/src/components/views/dialogs/SetMxIdDialog.js @@ -234,14 +234,14 @@ export default React.createClass({ "error": Boolean(this.state.usernameError), "success": usernameAvailable, }); - usernameIndicator =
+ usernameIndicator =
{ usernameAvailable ? _t('Username available') : this.state.usernameError }
; } let authErrorIndicator = null; if (this.state.authError) { - authErrorIndicator =
+ authErrorIndicator =
{ this.state.authError }
; } @@ -253,8 +253,9 @@ export default React.createClass({ -
+
- +

{ _t('"%(RoomName)s" contains devices that you haven\'t seen before.', {RoomName: this.props.room.name}) }

@@ -161,7 +162,7 @@ export default React.createClass({ }}> { _t("Send anyway") } - - +
diff --git a/src/components/views/dialogs/SessionRestoreErrorDialog.js b/src/components/views/dialogs/SessionRestoreErrorDialog.js index e68a097e80..f018a83301 100644 --- a/src/components/views/dialogs/SessionRestoreErrorDialog.js +++ b/src/components/views/dialogs/SessionRestoreErrorDialog.js @@ -54,7 +54,8 @@ export default React.createClass({ { _t( "Otherwise, click here to send a bug report.", {}, - { 'a': (sub) => { sub } }, + { 'a': (sub) => { sub } }, ) }

); diff --git a/src/components/views/dialogs/TextInputDialog.js b/src/components/views/dialogs/TextInputDialog.js index 907848b3b8..4d73752641 100644 --- a/src/components/views/dialogs/TextInputDialog.js +++ b/src/components/views/dialogs/TextInputDialog.js @@ -75,7 +75,7 @@ export default React.createClass({ - +
From ab0ff9b7814240f91ba0ad03351adf26cf4a63e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20V=C3=A1gner?= Date: Tue, 12 Dec 2017 18:55:57 +0100 Subject: [PATCH 12/19] BaseDialog: split a very long line --- src/components/views/dialogs/BaseDialog.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/components/views/dialogs/BaseDialog.js b/src/components/views/dialogs/BaseDialog.js index fbeb35c808..db8e530fa0 100644 --- a/src/components/views/dialogs/BaseDialog.js +++ b/src/components/views/dialogs/BaseDialog.js @@ -83,7 +83,12 @@ export default React.createClass({ const TintableSvg = sdk.getComponent("elements.TintableSvg"); return ( - + From 642675c96da9fb31d191539f884872d2b7717c27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20V=C3=A1gner?= Date: Thu, 14 Dec 2017 10:31:28 +0100 Subject: [PATCH 13/19] Address review request comments --- .../views/dialogs/InteractiveAuthDialog.js | 2 +- .../dialogs/SessionRestoreErrorDialog.js | 5 +- .../login/InteractiveAuthEntryComponents.js | 51 ++++++++++++++----- 3 files changed, 43 insertions(+), 15 deletions(-) diff --git a/src/components/views/dialogs/InteractiveAuthDialog.js b/src/components/views/dialogs/InteractiveAuthDialog.js index d1273849ae..5b0e34df2c 100644 --- a/src/components/views/dialogs/InteractiveAuthDialog.js +++ b/src/components/views/dialogs/InteractiveAuthDialog.js @@ -73,7 +73,7 @@ export default React.createClass({ if (this.state.authError) { content = (
-
{ this.state.authError.message || this.state.authError.toString() }
+
{ this.state.authError.message || this.state.authError.toString() }

); } + const shouldFocusContinueButton =!(bugreport==true); return ( -
-
diff --git a/src/components/views/login/InteractiveAuthEntryComponents.js b/src/components/views/login/InteractiveAuthEntryComponents.js index 81b84684fc..a0bb1065f2 100644 --- a/src/components/views/login/InteractiveAuthEntryComponents.js +++ b/src/components/views/login/InteractiveAuthEntryComponents.js @@ -127,6 +127,15 @@ export const PasswordAuthEntry = React.createClass({ ); } + let errorSection; + if (this.props.errorText) { + errorSection = ( +
+ { this.props.errorText } +
+ ); + } + return (

{ _t("To continue, please enter your password.") }

@@ -143,9 +152,7 @@ export const PasswordAuthEntry = React.createClass({ { submitButtonOrSpinner }
-
- { this.props.errorText } -
+ { errorSection }
); }, @@ -180,14 +187,22 @@ export const RecaptchaAuthEntry = React.createClass({ const CaptchaForm = sdk.getComponent("views.login.CaptchaForm"); const sitePublicKey = this.props.stageParams.public_key; + + let errorSection; + if (this.props.errorText) { + errorSection = ( +
+ { this.props.errorText } +
+ ); + } + return (
-
- { this.props.errorText } -
+ { errorSection }
); }, @@ -372,6 +387,14 @@ export const MsisdnAuthEntry = React.createClass({ mx_InteractiveAuthEntryComponents_msisdnSubmit: true, mx_UserSettings_button: true, // XXX button classes }); + let errorSection; + if (this.state.errorText) { + errorSection = ( +
+ { this.state.errorText } +
+ ); + } return (

{ _t("A text message has been sent to %(msisdn)s", @@ -393,9 +416,7 @@ export const MsisdnAuthEntry = React.createClass({ disabled={!enableSubmit} /> -

- { this.state.errorText } -
+ {errorSection}
); @@ -452,12 +473,18 @@ export const FallbackAuthEntry = React.createClass({ }, render: function() { + let errorSection; + if (this.props.errorText) { + errorSection = ( +
+ { this.props.errorText } +
+ ); + } return (
{ _t("Start authentication") } -
- { this.props.errorText } -
+ {errorSection}
); }, From 20c485d85e4deaae0a8090496241133902a76486 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20V=C3=A1gner?= Date: Wed, 20 Dec 2017 10:09:26 +0100 Subject: [PATCH 14/19] Move aria-hidden management from the BaseDialog component to the Modal --- src/Modal.js | 14 ++++++++++++++ src/components/views/dialogs/BaseDialog.js | 18 ------------------ 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/src/Modal.js b/src/Modal.js index 68d75d1ff1..69ff806045 100644 --- a/src/Modal.js +++ b/src/Modal.js @@ -186,11 +186,25 @@ class ModalManager { } _reRender() { + // Retrieve the root node of the Riot application outside the modal + let applicationNode = document.getElementById('matrixchat'); if (this._modals.length == 0) { + if (applicationNode) { + // If there is no modal to render, make all of Riot available + // to screen reader users again + applicationNode.setAttribute('aria-hidden', 'false'); + } ReactDOM.unmountComponentAtNode(this.getOrCreateContainer()); return; } + if (applicationNode) { + // Hide the content outside the modal to screen reader users + // so they won't be able to navigate into it and act on it using + // screen reader specific features + applicationNode.setAttribute('aria-hidden', 'true'); + } + const modal = this._modals[0]; const dialog = (
diff --git a/src/components/views/dialogs/BaseDialog.js b/src/components/views/dialogs/BaseDialog.js index db8e530fa0..7fb642b560 100644 --- a/src/components/views/dialogs/BaseDialog.js +++ b/src/components/views/dialogs/BaseDialog.js @@ -49,24 +49,6 @@ export default React.createClass({ contentId: React.PropTypes.string, }, - componentDidMount: function() { - // Retrieve the root node of the Riot application outside the dialog - this.applicationNode = document.getElementById('matrixchat'); - if (this.applicationNode) { - // Hide the content outside the dialog to screen reader users - // so they won't be able to navigate into it and act on it using - // screen reader specific features - this.applicationNode.setAttribute('aria-hidden', 'true'); - } - }, - - componentWillUnmount: function() { - if (this.applicationNode) { - // When dismissing the dialog, make all of Riot available to screen reader users again - this.applicationNode.setAttribute('aria-hidden', 'false'); - } - }, - _onKeyDown: function(e) { if (e.keyCode === KeyCode.ESCAPE) { e.stopPropagation(); From f2ca02eaf8aa7b4cd997526bc23396bddf25b8ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20V=C3=A1gner?= Date: Wed, 20 Dec 2017 10:13:37 +0100 Subject: [PATCH 15/19] SetEmailDialog: use autoFocus prop on the EditableText rather than using its ref inside onComponentDidMount function. This is shorter better and has been requested. --- src/components/views/dialogs/SetEmailDialog.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/components/views/dialogs/SetEmailDialog.js b/src/components/views/dialogs/SetEmailDialog.js index 5a9569c1b5..ba054b0c27 100644 --- a/src/components/views/dialogs/SetEmailDialog.js +++ b/src/components/views/dialogs/SetEmailDialog.js @@ -40,10 +40,6 @@ export default React.createClass({ }; }, - componentDidMount: function() { - this.refs.emailInputField.focus(); - }, - onEmailAddressChanged: function(value) { this.setState({ emailAddress: value, @@ -131,7 +127,7 @@ export default React.createClass({ const emailInput = this.state.emailBusy ? : Date: Fri, 5 Jan 2018 11:45:45 +0100 Subject: [PATCH 16/19] Consume all combinations of space / enter, keyDown / keyUp presses and try to explain this key handling inconsistency with some additional comments as per the review discussion. --- src/components/views/elements/AccessibleButton.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/components/views/elements/AccessibleButton.js b/src/components/views/elements/AccessibleButton.js index f5254de490..ee8fd20d6f 100644 --- a/src/components/views/elements/AccessibleButton.js +++ b/src/components/views/elements/AccessibleButton.js @@ -32,12 +32,20 @@ export default function AccessibleButton(props) { // We need to consume enter onKeyDown and space onKeyUp // otherwise we are risking also activating other keyboard focusable elements // that might receive focus as a result of the AccessibleButtonClick action + // It's because we are using html buttons at a few places e.g. inside dialogs + // And divs which we report as role button to assistive technologies. + // Browsers handle space and enter keypresses differently and we are only adjusting to the + // inconsistencies here restProps.onKeyDown = function(e) { if (e.keyCode === KeyCode.ENTER) { e.stopPropagation(); e.preventDefault(); return onClick(e); } + if (e.keyCode === KeyCode.SPACE) { + e.stopPropagation(); + e.preventDefault(); + } }; restProps.onKeyUp = function(e) { if (e.keyCode === KeyCode.SPACE) { @@ -45,6 +53,10 @@ export default function AccessibleButton(props) { e.preventDefault(); return onClick(e); } + if (e.keyCode === KeyCode.ENTER) { + e.stopPropagation(); + e.preventDefault(); + } }; restProps.tabIndex = restProps.tabIndex || "0"; restProps.role = "button"; From 8f97e9479d3be395d9855f17fb093629b1cd4ad6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20V=C3=A1gner?= Date: Thu, 8 Feb 2018 21:16:57 +0100 Subject: [PATCH 17/19] Ooops, restore a bit of RoomCreateDialog content I have accidentally removed while trying to solve merge conflicts (thx @dbkr) --- src/components/views/dialogs/CreateRoomDialog.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/components/views/dialogs/CreateRoomDialog.js b/src/components/views/dialogs/CreateRoomDialog.js index 7a8852bc00..51693a19c9 100644 --- a/src/components/views/dialogs/CreateRoomDialog.js +++ b/src/components/views/dialogs/CreateRoomDialog.js @@ -55,6 +55,19 @@ export default React.createClass({
+
+ +
+ { _t('Advanced options') } +
+ + +
+
Date: Thu, 8 Feb 2018 22:51:07 +0100 Subject: [PATCH 18/19] Reimplement setting aria-hidden on the main app node by dispatching actions rather than assuming we can find and manipulate the node directly --- src/Modal.js | 25 +++++++++++------------ src/components/structures/LoggedInView.js | 2 +- src/components/structures/MatrixChat.js | 14 +++++++++++++ 3 files changed, 27 insertions(+), 14 deletions(-) diff --git a/src/Modal.js b/src/Modal.js index 8e3b394f87..2565d5c73b 100644 --- a/src/Modal.js +++ b/src/Modal.js @@ -22,6 +22,7 @@ const ReactDOM = require('react-dom'); import PropTypes from 'prop-types'; import Analytics from './Analytics'; import sdk from './index'; +import dis from './dispatcher'; const DIALOG_CONTAINER_ID = "mx_Dialog_Container"; @@ -187,24 +188,22 @@ class ModalManager { } _reRender() { - // Retrieve the root node of the Riot application outside the modal - let applicationNode = document.getElementById('matrixchat'); if (this._modals.length == 0) { - if (applicationNode) { - // If there is no modal to render, make all of Riot available - // to screen reader users again - applicationNode.setAttribute('aria-hidden', 'false'); - } + // If there is no modal to render, make all of Riot available + // to screen reader users again + dis.dispatch({ + action: 'aria_unhide_main_app', + }); ReactDOM.unmountComponentAtNode(this.getOrCreateContainer()); return; } - if (applicationNode) { - // Hide the content outside the modal to screen reader users - // so they won't be able to navigate into it and act on it using - // screen reader specific features - applicationNode.setAttribute('aria-hidden', 'true'); - } + // Hide the content outside the modal to screen reader users + // so they won't be able to navigate into it and act on it using + // screen reader specific features + dis.dispatch({ + action: 'aria_hide_main_app', + }); const modal = this._modals[0]; const dialog = ( diff --git a/src/components/structures/LoggedInView.js b/src/components/structures/LoggedInView.js index e97d9dd0a1..211340f5a8 100644 --- a/src/components/structures/LoggedInView.js +++ b/src/components/structures/LoggedInView.js @@ -327,7 +327,7 @@ const LoggedInView = React.createClass({ } return ( -
+
{ topBar }
{ SettingsStore.isFeatureEnabled("feature_tag_panel") ? :
} diff --git a/src/components/structures/MatrixChat.js b/src/components/structures/MatrixChat.js index d6d0b00c84..380cc697e1 100644 --- a/src/components/structures/MatrixChat.js +++ b/src/components/structures/MatrixChat.js @@ -171,6 +171,10 @@ export default React.createClass({ register_hs_url: null, register_is_url: null, register_id_sid: null, + + // When showing Modal dialogs we need to set aria-hidden on the root app element + // and disable it when there are no dialogs + hideToSRUsers: false, }; return s; }, @@ -608,6 +612,16 @@ export default React.createClass({ case 'send_event': this.onSendEvent(payload.room_id, payload.event); break; + case 'aria_hide_main_app': + this.setState({ + hideToSRUsers: true, + }); + break; + case 'aria_unhide_main_app': + this.setState({ + hideToSRUsers: false, + }); + break; } }, From 5e9368e794eac546569d93ae2738bd24566cdc74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20V=C3=A1gner?= Date: Mon, 12 Feb 2018 21:13:53 +0100 Subject: [PATCH 19/19] Add comments explaining our non standard usage of aria-described-by --- src/components/views/dialogs/BaseDialog.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/components/views/dialogs/BaseDialog.js b/src/components/views/dialogs/BaseDialog.js index 08fd972621..053aef66c3 100644 --- a/src/components/views/dialogs/BaseDialog.js +++ b/src/components/views/dialogs/BaseDialog.js @@ -76,6 +76,12 @@ export default React.createClass({ className={this.props.className} role="dialog" aria-labelledby='mx_BaseDialog_title' + // This should point to a node describing the dialog. + // If we were about to completelly follow this recommendation we'd need to + // make all the components relying on BaseDialog to be aware of it. + // So instead we will use the whole content as the description. + // Description comes first and if the content contains more text, + // AT users can skip its presentation. aria-describedby={this.props.contentId} >