From f67eedf8439591531273ad3165d513cdeed0206a Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Sun, 15 Dec 2019 14:24:56 +0000 Subject: [PATCH 1/4] Fix keyboard handling including scroll into view, add aria roles --- src/components/views/elements/Dropdown.js | 95 ++++++++++++++++------- 1 file changed, 68 insertions(+), 27 deletions(-) diff --git a/src/components/views/elements/Dropdown.js b/src/components/views/elements/Dropdown.js index 4c5e14b3ba..65be56f1f5 100644 --- a/src/components/views/elements/Dropdown.js +++ b/src/components/views/elements/Dropdown.js @@ -20,6 +20,7 @@ import PropTypes from 'prop-types'; import classnames from 'classnames'; import AccessibleButton from './AccessibleButton'; import { _t } from '../../../languageHandler'; +import {Key} from "../../../Keyboard"; class MenuOption extends React.Component { constructor(props) { @@ -48,9 +49,13 @@ class MenuOption extends React.Component { mx_Dropdown_option_highlight: this.props.highlighted, }); - return
{ this.props.children }
; @@ -66,6 +71,7 @@ MenuOption.propTypes = { dropdownKey: PropTypes.string, onClick: PropTypes.func.isRequired, onMouseEnter: PropTypes.func.isRequired, + inputRef: PropTypes.any, }; /* @@ -177,32 +183,43 @@ export default class Dropdown extends React.Component { } _onInputKeyPress(e) { - // This needs to be on the keypress event because otherwise - // it can't cancel the form submission - if (e.key == 'Enter') { - this.setState({ - expanded: false, - }); - this.props.onOptionChange(this.state.highlightedOption); + // This needs to be on the keypress event because otherwise it can't cancel the form submission + if (e.key === Key.ENTER) { e.preventDefault(); } } _onInputKeyUp(e) { - // These keys don't generate keypress events and so needs to - // be on keyup - if (e.key == 'Escape') { - this.setState({ - expanded: false, - }); - } else if (e.key == 'ArrowDown') { - this.setState({ - highlightedOption: this._nextOption(this.state.highlightedOption), - }); - } else if (e.key == 'ArrowUp') { - this.setState({ - highlightedOption: this._prevOption(this.state.highlightedOption), - }); + // These keys don't generate keypress events and so needs to be on keyup + switch (e.key) { + case Key.ENTER: + this.props.onOptionChange(this.state.highlightedOption); + // fallthrough + case Key.ESCAPE: + this.setState({ + expanded: false, + }); + break; + case Key.ARROW_DOWN: + this.setState({ + highlightedOption: this._nextOption(this.state.highlightedOption), + }); + break; + case Key.ARROW_UP: + this.setState({ + highlightedOption: this._prevOption(this.state.highlightedOption), + }); + break; + case Key.HOME: + this.setState({ + highlightedOption: this._firstOption(), + }); + break; + case Key.END: + this.setState({ + highlightedOption: this._lastOption(), + }); + break; } } @@ -250,13 +267,36 @@ export default class Dropdown extends React.Component { return keys[(index - 1) % keys.length]; } + _firstOption() { + const keys = Object.keys(this.childrenByKey); + return keys[0]; + } + + _lastOption() { + const keys = Object.keys(this.childrenByKey); + return keys[keys.length - 1]; + } + + _scrollIntoView(node) { + if (node) { + node.scrollIntoView({ + block: "nearest", + behavior: "auto", + }); + } + } + _getMenuOptions() { const options = React.Children.map(this.props.children, (child) => { + const highlighted = this.state.highlightedOption === child.key; return ( - { child } @@ -280,13 +320,14 @@ export default class Dropdown extends React.Component { if (this.state.expanded) { if (this.props.searchEnabled) { currentValue = ; } - menu =
+ menu =
{ this._getMenuOptions() }
; } @@ -313,7 +354,7 @@ export default class Dropdown extends React.Component { return
{ currentValue } - + { menu }
; From cecf581e04af046f4547f4d2d002a452ed127c5e Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Sun, 15 Dec 2019 15:04:57 +0000 Subject: [PATCH 2/4] Make Combobox dropdown keyboard and screen reader accessible --- src/components/views/auth/CountryDropdown.js | 16 +++-- src/components/views/elements/Dropdown.js | 64 +++++++++++++++---- .../views/elements/LanguageDropdown.js | 12 +++- src/i18n/strings/en_EN.json | 2 + 4 files changed, 73 insertions(+), 21 deletions(-) diff --git a/src/components/views/auth/CountryDropdown.js b/src/components/views/auth/CountryDropdown.js index d8aa88c798..567bcf59ef 100644 --- a/src/components/views/auth/CountryDropdown.js +++ b/src/components/views/auth/CountryDropdown.js @@ -21,6 +21,7 @@ import sdk from '../../../index'; import { COUNTRIES } from '../../../phonenumber'; import SdkConfig from "../../../SdkConfig"; +import { _t } from "../../../languageHandler"; const COUNTRIES_BY_ISO2 = {}; for (const c of COUNTRIES) { @@ -130,10 +131,17 @@ export default class CountryDropdown extends React.Component { // values between mounting and the initial value propgating const value = this.props.value || this.state.defaultCountry.iso2; - return { options } ; diff --git a/src/components/views/elements/Dropdown.js b/src/components/views/elements/Dropdown.js index 65be56f1f5..72e6cf19b3 100644 --- a/src/components/views/elements/Dropdown.js +++ b/src/components/views/elements/Dropdown.js @@ -1,6 +1,7 @@ /* Copyright 2017 Vector Creations Ltd Copyright 2019 Michael Telatynski <7t3chguy@gmail.com> +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. @@ -15,7 +16,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import React from 'react'; +import React, {createRef} from 'react'; import PropTypes from 'prop-types'; import classnames from 'classnames'; import AccessibleButton from './AccessibleButton'; @@ -50,6 +51,7 @@ class MenuOption extends React.Component { }); return
+ return [
{ _t("No results") }
]; } @@ -319,24 +331,36 @@ export default class Dropdown extends React.Component { let menu; if (this.state.expanded) { if (this.props.searchEnabled) { - currentValue = ; + currentValue = ( + + ); } - menu =
- { this._getMenuOptions() } -
; + menu = ( +
+ { this._getMenuOptions() } +
+ ); } if (!currentValue) { const selectedChild = this.props.getShortOption ? this.props.getShortOption(this.props.value) : this.childrenByKey[this.props.value]; - currentValue =
+ currentValue =
{ selectedChild }
; } @@ -352,7 +376,16 @@ export default class Dropdown extends React.Component { // Note the menu sits inside the AccessibleButton div so it's anchored // to the input, but overflows below it. The root contains both. return
- + { currentValue } { menu } @@ -362,6 +395,7 @@ export default class Dropdown extends React.Component { } Dropdown.propTypes = { + id: PropTypes.string.isRequired, // The width that the dropdown should be. If specified, // the dropped-down part of the menu will be set to this // width. @@ -381,4 +415,6 @@ Dropdown.propTypes = { value: PropTypes.string, // negative for consistency with HTML disabled: PropTypes.bool, + // ARIA label + label: PropTypes.string.isRequired, }; diff --git a/src/components/views/elements/LanguageDropdown.js b/src/components/views/elements/LanguageDropdown.js index 451c97d958..ebe26cfad8 100644 --- a/src/components/views/elements/LanguageDropdown.js +++ b/src/components/views/elements/LanguageDropdown.js @@ -21,6 +21,7 @@ import PropTypes from 'prop-types'; import sdk from '../../../index'; import * as languageHandler from '../../../languageHandler'; import SettingsStore from "../../../settings/SettingsStore"; +import { _t } from "../../../languageHandler"; function languageMatchesSearchQuery(query, language) { if (language.label.toUpperCase().indexOf(query.toUpperCase()) == 0) return true; @@ -105,9 +106,14 @@ export default class LanguageDropdown extends React.Component { value = this.props.value || language; } - return { options } ; diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 80604e9090..17d589d00e 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -1262,6 +1262,7 @@ "Rotate Right": "Rotate Right", "Rotate clockwise": "Rotate clockwise", "Download this file": "Download this file", + "Language Dropdown": "Language Dropdown", "Manage Integrations": "Manage Integrations", "%(nameList)s %(transitionList)s": "%(nameList)s %(transitionList)s", "%(severalUsers)sjoined %(count)s times|other": "%(severalUsers)sjoined %(count)s times", @@ -1627,6 +1628,7 @@ "User Status": "User Status", "powered by Matrix": "powered by Matrix", "This homeserver would like to make sure you are not a robot.": "This homeserver would like to make sure you are not a robot.", + "Country Dropdown": "Country Dropdown", "Custom Server Options": "Custom Server Options", "You can use the custom server options to sign into other Matrix servers by specifying a different homeserver URL. This allows you to use this app with an existing Matrix account on a different homeserver.": "You can use the custom server options to sign into other Matrix servers by specifying a different homeserver URL. This allows you to use this app with an existing Matrix account on a different homeserver.", "To continue, please enter your password.": "To continue, please enter your password.", From 224ee05b62c2b3e818b7bcb19f4b7f13b7357162 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Sun, 15 Dec 2019 15:07:49 +0000 Subject: [PATCH 3/4] this is a combobox, HOME/END should pertain to the input, not selection --- src/components/views/elements/Dropdown.js | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/src/components/views/elements/Dropdown.js b/src/components/views/elements/Dropdown.js index 72e6cf19b3..9b4181fe68 100644 --- a/src/components/views/elements/Dropdown.js +++ b/src/components/views/elements/Dropdown.js @@ -221,16 +221,6 @@ export default class Dropdown extends React.Component { highlightedOption: this._prevOption(this.state.highlightedOption), }); break; - case Key.HOME: - this.setState({ - highlightedOption: this._firstOption(), - }); - break; - case Key.END: - this.setState({ - highlightedOption: this._lastOption(), - }); - break; } } @@ -278,16 +268,6 @@ export default class Dropdown extends React.Component { return keys[(index - 1) % keys.length]; } - _firstOption() { - const keys = Object.keys(this.childrenByKey); - return keys[0]; - } - - _lastOption() { - const keys = Object.keys(this.childrenByKey); - return keys[keys.length - 1]; - } - _scrollIntoView(node) { if (node) { node.scrollIntoView({ From 9c4eb1d649f9887133651ea83643f7496994d202 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 16 Dec 2019 10:03:40 +0000 Subject: [PATCH 4/4] clean up new code --- src/components/views/elements/Dropdown.js | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/components/views/elements/Dropdown.js b/src/components/views/elements/Dropdown.js index 9b4181fe68..67a8663b3e 100644 --- a/src/components/views/elements/Dropdown.js +++ b/src/components/views/elements/Dropdown.js @@ -178,7 +178,7 @@ export default class Dropdown extends React.Component { } } - _onMenuOptionClick(dropdownKey) { + _close() { this.setState({ expanded: false, }); @@ -186,6 +186,10 @@ export default class Dropdown extends React.Component { if (this._button.current) { this._button.current.focus(); } + } + + _onMenuOptionClick(dropdownKey) { + this._close(); this.props.onOptionChange(dropdownKey); } @@ -203,13 +207,7 @@ export default class Dropdown extends React.Component { this.props.onOptionChange(this.state.highlightedOption); // fallthrough case Key.ESCAPE: - this.setState({ - expanded: false, - }); - // their focus was on the input, its getting unmounted, move it to the button - if (this._button.current) { - this._button.current.focus(); - } + this._close(); break; case Key.ARROW_DOWN: this.setState({