From 47add752780fd462eefdfb2601923487dc421d6d Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 17 May 2019 15:25:17 -0600 Subject: [PATCH 1/8] Add class to hide focus highlight We use tabIndex to make elements selectable and therefore focused by screen readers. Doing this draws a blue border (in chrome at least) around the element - in some cases, we don't want this. --- res/css/_common.scss | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/res/css/_common.scss b/res/css/_common.scss index d46f38bddb..ba239b8ca8 100644 --- a/res/css/_common.scss +++ b/res/css/_common.scss @@ -110,6 +110,10 @@ textarea { color: $primary-fg-color; } +.mx_HiddenFocusable { + outline: none; +} + // .mx_textinput is a container for a text input // + some other controls like buttons, ... // it has the appearance of a text box so the controls From f1aa2875e1e9bf6c7a8d87f93b8f8f050568b0d6 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 17 May 2019 15:25:59 -0600 Subject: [PATCH 2/8] Hide avatars from screen readers by default To avoid having them read out the user's ID --- src/components/views/avatars/BaseAvatar.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/avatars/BaseAvatar.js b/src/components/views/avatars/BaseAvatar.js index 47de7c9dc4..af84b7fd81 100644 --- a/src/components/views/avatars/BaseAvatar.js +++ b/src/components/views/avatars/BaseAvatar.js @@ -189,7 +189,7 @@ module.exports = React.createClass({ const imgNode = ( + width={width} height={height} aria-hidden="true" /> ); if (onClick != null) { return ( From c5757d8303bda840ac417c8bdaedfab65d3347e6 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 17 May 2019 15:28:12 -0600 Subject: [PATCH 3/8] Support CTRL+I for opening TopLeftMenu --- src/components/structures/LoggedInView.js | 8 +++++ .../structures/TopLeftMenuButton.js | 36 ++++++++++++++++--- .../views/elements/AccessibleButton.js | 5 +++ src/i18n/strings/en_EN.json | 1 + 4 files changed, 45 insertions(+), 5 deletions(-) diff --git a/src/components/structures/LoggedInView.js b/src/components/structures/LoggedInView.js index 4771c6f487..130a3bf055 100644 --- a/src/components/structures/LoggedInView.js +++ b/src/components/structures/LoggedInView.js @@ -322,6 +322,14 @@ const LoggedInView = React.createClass({ handled = true; } break; + case KeyCode.KEY_I: + if (ctrlCmdOnly) { + dis.dispatch({ + action: 'toggle_top_left_menu', + }); + handled = true; + } + break; } if (handled) { diff --git a/src/components/structures/TopLeftMenuButton.js b/src/components/structures/TopLeftMenuButton.js index b68d3a95a0..36cd359b8a 100644 --- a/src/components/structures/TopLeftMenuButton.js +++ b/src/components/structures/TopLeftMenuButton.js @@ -23,6 +23,7 @@ import BaseAvatar from '../views/avatars/BaseAvatar'; import MatrixClientPeg from '../../MatrixClientPeg'; import Avatar from '../../Avatar'; import { _t } from '../../languageHandler'; +import dis from "../../dispatcher"; const AVATAR_SIZE = 28; @@ -37,6 +38,7 @@ export default class TopLeftMenuButton extends React.Component { super(); this.state = { menuDisplayed: false, + menuFunctions: null, // should be { close: fn } profileInfo: null, }; @@ -59,6 +61,8 @@ export default class TopLeftMenuButton extends React.Component { } async componentDidMount() { + this._dispatcherRef = dis.register(this.onAction); + try { const profileInfo = await this._getProfileInfo(); this.setState({profileInfo}); @@ -68,6 +72,17 @@ export default class TopLeftMenuButton extends React.Component { } } + componentWillUnmount() { + dis.unregister(this._dispatcherRef); + } + + onAction = (payload) => { + // For accessibility + if (payload.action === "toggle_top_left_menu") { + if (this._buttonRef) this._buttonRef.click(); + } + }; + _getDisplayName() { if (MatrixClientPeg.get().isGuest()) { return _t("Guest"); @@ -88,7 +103,13 @@ export default class TopLeftMenuButton extends React.Component { } return ( - + this._buttonRef = r} + aria-label={_t("Your profile")} + > { nameElement } - + ); } @@ -107,20 +128,25 @@ export default class TopLeftMenuButton extends React.Component { e.preventDefault(); e.stopPropagation(); + if (this.state.menuDisplayed && this.state.menuFunctions) { + this.state.menuFunctions.close(); + return; + } + const elementRect = e.currentTarget.getBoundingClientRect(); const x = elementRect.left; const y = elementRect.top + elementRect.height; - ContextualMenu.createMenu(TopLeftMenu, { + const menuFunctions = ContextualMenu.createMenu(TopLeftMenu, { chevronFace: "none", left: x, top: y, userId: MatrixClientPeg.get().getUserId(), displayName: this._getDisplayName(), onFinished: () => { - this.setState({ menuDisplayed: false }); + this.setState({ menuDisplayed: false, menuFunctions: null }); }, }); - this.setState({ menuDisplayed: true }); + this.setState({ menuDisplayed: true, menuFunctions }); } } diff --git a/src/components/views/elements/AccessibleButton.js b/src/components/views/elements/AccessibleButton.js index 1c39ba4f49..06c440c54e 100644 --- a/src/components/views/elements/AccessibleButton.js +++ b/src/components/views/elements/AccessibleButton.js @@ -63,6 +63,10 @@ export default function AccessibleButton(props) { }; } + // Pass through the ref - used for keyboard shortcut access to some buttons + restProps.ref = restProps.inputRef; + delete restProps.inputRef; + restProps.tabIndex = restProps.tabIndex || "0"; restProps.role = "button"; restProps.className = (restProps.className ? restProps.className + " " : "") + @@ -89,6 +93,7 @@ export default function AccessibleButton(props) { */ AccessibleButton.propTypes = { children: PropTypes.node, + inputRef: PropTypes.func, element: PropTypes.string, onClick: PropTypes.func.isRequired, diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 86131645cf..2af0af2795 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -1493,6 +1493,7 @@ "Tried to load a specific point in this room's timeline, but was unable to find it.": "Tried to load a specific point in this room's timeline, but was unable to find it.", "Failed to load timeline position": "Failed to load timeline position", "Guest": "Guest", + "Your profile": "Your profile", "Uploading %(filename)s and %(count)s others|other": "Uploading %(filename)s and %(count)s others", "Uploading %(filename)s and %(count)s others|zero": "Uploading %(filename)s", "Uploading %(filename)s and %(count)s others|one": "Uploading %(filename)s and %(count)s other", From 2a187810fd66d7d45bcccf0f8fb64691fd0578c6 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 17 May 2019 15:32:03 -0600 Subject: [PATCH 4/8] Restructure TopLeftMenu for accessibility and autofocus it We use a trick with refs to automatically focus the element, also making use of mx_HiddenFocusable to hide the unnecessary outline. The menu itself has been restructured to hide some elements from screen readers (reduce noise) and to have a single unordered list. Screen readers mention when the user "enters" a list, and each item was previously saying "enter list " when it should have just been "". By focusing automatically, the keyboard can be used to go up/down the menu as may be expected by keyboard users. --- .../views/context_menus/TopLeftMenu.js | 43 +++++++++++-------- src/utils/Accessibility.js | 19 ++++++++ 2 files changed, 43 insertions(+), 19 deletions(-) create mode 100644 src/utils/Accessibility.js diff --git a/src/components/views/context_menus/TopLeftMenu.js b/src/components/views/context_menus/TopLeftMenu.js index 278c879404..14e93044f4 100644 --- a/src/components/views/context_menus/TopLeftMenu.js +++ b/src/components/views/context_menus/TopLeftMenu.js @@ -23,6 +23,7 @@ import Modal from "../../../Modal"; import SdkConfig from '../../../SdkConfig'; import { getHostingLink } from '../../../utils/HostingLink'; import MatrixClientPeg from '../../../MatrixClientPeg'; +import {focusCapturedRef} from "../../../utils/Accessibility"; export class TopLeftMenu extends React.Component { static propTypes = { @@ -61,44 +62,48 @@ export class TopLeftMenu extends React.Component { {_t( "Upgrade to your own domain", {}, { - a: sub => {sub}, + a: sub => {sub}, }, )} - + ; } - let homePageSection = null; + let homePageItem = null; if (this.hasHomePage()) { - homePageSection =
    -
  • {_t("Home")}
    • -
; + homePageItem =
  • + {_t("Home")} +
    • ; } - let signInOutSection; + let signInOutItem; if (isGuest) { - signInOutSection =
      -
    • {_t("Sign in")}
      • -
    ; + signInOutItem =
  • + {_t("Sign in")} +
    • ; } else { - signInOutSection =
      -
    • {_t("Sign out")}
      • -
    ; + signInOutItem =
  • + {_t("Sign out")} +
    • ; } - return
    -
    + const settingsItem =
  • + {_t("Settings")} +
    • ; + + return
    +
    {this.props.displayName}
    -
    {this.props.userId}
    +
    {this.props.userId}
    {hostingSignup}
    - {homePageSection}
      -
    • {_t("Settings")}
      • + {homePageItem} + {settingsItem} + {signInOutItem}
    - {signInOutSection}
    ; } diff --git a/src/utils/Accessibility.js b/src/utils/Accessibility.js new file mode 100644 index 0000000000..dbdfeec7df --- /dev/null +++ b/src/utils/Accessibility.js @@ -0,0 +1,19 @@ +/* +Copyright 2019 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +export function focusCapturedRef(ref) { + if (ref) ref.focus(); +} \ No newline at end of file From 332f716ce4433c5b11abdf83495abb1bc4b89751 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 17 May 2019 15:36:28 -0600 Subject: [PATCH 5/8] The linter will be the death of me --- src/utils/Accessibility.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/Accessibility.js b/src/utils/Accessibility.js index dbdfeec7df..8c6090671d 100644 --- a/src/utils/Accessibility.js +++ b/src/utils/Accessibility.js @@ -16,4 +16,4 @@ limitations under the License. export function focusCapturedRef(ref) { if (ref) ref.focus(); -} \ No newline at end of file +} From 52b0f285c6eef7482b34a2bea69369a91c1e28c1 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 20 May 2019 20:41:48 -0600 Subject: [PATCH 6/8] Add some clarifying comments --- res/css/_common.scss | 4 ++++ src/components/structures/LoggedInView.js | 4 ++++ src/utils/Accessibility.js | 11 ++++++++++- 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/res/css/_common.scss b/res/css/_common.scss index ba239b8ca8..5c64fc4c1a 100644 --- a/res/css/_common.scss +++ b/res/css/_common.scss @@ -110,6 +110,10 @@ textarea { color: $primary-fg-color; } +// This is used to hide the standard outline added by browsers for +// accessible (focusable) components. Not intended for buttons, but +// should be used on things like focusable containers where the outline +// is usually not helping anyone. .mx_HiddenFocusable { outline: none; } diff --git a/src/components/structures/LoggedInView.js b/src/components/structures/LoggedInView.js index 130a3bf055..0ad2f72cfc 100644 --- a/src/components/structures/LoggedInView.js +++ b/src/components/structures/LoggedInView.js @@ -323,6 +323,10 @@ const LoggedInView = React.createClass({ } break; case KeyCode.KEY_I: + // Ideally this would be CTRL+P for "Profile", but that's + // taken by the print dialog. CTRL+I for "Information" + // will have to do. + if (ctrlCmdOnly) { dis.dispatch({ action: 'toggle_top_left_menu', diff --git a/src/utils/Accessibility.js b/src/utils/Accessibility.js index 8c6090671d..c04d6e0d9b 100644 --- a/src/utils/Accessibility.js +++ b/src/utils/Accessibility.js @@ -14,6 +14,15 @@ See the License for the specific language governing permissions and limitations under the License. */ +/** + * Automatically focuses the captured reference when receiving a non-null + * object. Useful in scenarios where componentDidMount does not have a + * useful reference to an element, but one needs to focus the element on + * first render. Example usage: ref={focusCapturedRef} + * @param ref The React reference to focus on, if not null + */ export function focusCapturedRef(ref) { - if (ref) ref.focus(); + if (ref) { + ref.focus(); + } } From aac87c4635eacb13e44bcf50e2e192c34f126d15 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 20 May 2019 21:07:55 -0600 Subject: [PATCH 7/8] Move focusing to the context menu create call --- src/components/structures/TopLeftMenuButton.js | 3 +++ src/components/views/context_menus/TopLeftMenu.js | 8 ++++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/components/structures/TopLeftMenuButton.js b/src/components/structures/TopLeftMenuButton.js index 36cd359b8a..f745a7f7bc 100644 --- a/src/components/structures/TopLeftMenuButton.js +++ b/src/components/structures/TopLeftMenuButton.js @@ -1,5 +1,6 @@ /* Copyright 2018 New Vector Ltd +Copyright 2019 The Matrix.org Foundation C.I.C. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -24,6 +25,7 @@ import MatrixClientPeg from '../../MatrixClientPeg'; import Avatar from '../../Avatar'; import { _t } from '../../languageHandler'; import dis from "../../dispatcher"; +import {focusCapturedRef} from "../../utils/Accessibility"; const AVATAR_SIZE = 28; @@ -143,6 +145,7 @@ export default class TopLeftMenuButton extends React.Component { top: y, userId: MatrixClientPeg.get().getUserId(), displayName: this._getDisplayName(), + containerRef: focusCapturedRef, // Focus the TopLeftMenu on first render onFinished: () => { this.setState({ menuDisplayed: false, menuFunctions: null }); }, diff --git a/src/components/views/context_menus/TopLeftMenu.js b/src/components/views/context_menus/TopLeftMenu.js index 14e93044f4..09e9142201 100644 --- a/src/components/views/context_menus/TopLeftMenu.js +++ b/src/components/views/context_menus/TopLeftMenu.js @@ -1,5 +1,6 @@ /* Copyright 2018, 2019 New Vector Ltd +Copyright 2019 The Matrix.org Foundation C.I.C. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -23,13 +24,16 @@ import Modal from "../../../Modal"; import SdkConfig from '../../../SdkConfig'; import { getHostingLink } from '../../../utils/HostingLink'; import MatrixClientPeg from '../../../MatrixClientPeg'; -import {focusCapturedRef} from "../../../utils/Accessibility"; export class TopLeftMenu extends React.Component { static propTypes = { displayName: PropTypes.string.isRequired, userId: PropTypes.string.isRequired, onFinished: PropTypes.func, + + // Optional function to collect a reference to the container + // of this component directly. + containerRef: PropTypes.func, }; constructor() { @@ -93,7 +97,7 @@ export class TopLeftMenu extends React.Component { {_t("Settings")} ; - return
    + return
    {this.props.displayName}
    {this.props.userId}
    From bf289935097d453b2606cd9ed8c63f84c869e926 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 21 May 2019 09:33:52 -0600 Subject: [PATCH 8/8] appease the js-doc linter hopefully --- src/utils/Accessibility.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/Accessibility.js b/src/utils/Accessibility.js index c04d6e0d9b..f4909f971b 100644 --- a/src/utils/Accessibility.js +++ b/src/utils/Accessibility.js @@ -19,7 +19,7 @@ limitations under the License. * object. Useful in scenarios where componentDidMount does not have a * useful reference to an element, but one needs to focus the element on * first render. Example usage: ref={focusCapturedRef} - * @param ref The React reference to focus on, if not null + * @param {function} ref The React reference to focus on, if not null */ export function focusCapturedRef(ref) { if (ref) {