From 171874ae30ca36513c01a5ac74596dba33bc6826 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 2 Jul 2021 16:31:37 +0100 Subject: [PATCH 1/4] Use FocusLock around ContextMenus to simplify focus management --- src/components/structures/ContextMenu.tsx | 27 +++++++------------ .../views/spaces/SpaceCreateMenu.tsx | 23 +++++++--------- 2 files changed, 20 insertions(+), 30 deletions(-) diff --git a/src/components/structures/ContextMenu.tsx b/src/components/structures/ContextMenu.tsx index 407dc6f04c..9bc9c0a8b2 100644 --- a/src/components/structures/ContextMenu.tsx +++ b/src/components/structures/ContextMenu.tsx @@ -19,6 +19,7 @@ limitations under the License. import React, { CSSProperties, RefObject, useRef, useState } from "react"; import ReactDOM from "react-dom"; import classNames from "classnames"; +import FocusLock from "react-focus-lock"; import { Key } from "../../Keyboard"; import { Writeable } from "../../@types/common"; @@ -44,6 +45,7 @@ function getOrCreateContainer(): HTMLDivElement { } const ARIA_MENU_ITEM_ROLES = new Set(["menuitem", "menuitemcheckbox", "menuitemradio"]); +const ARIA_MENU_ITEM_SELECTOR = '[role^="menuitem"], [role^="menuitemcheckbox"], [role^="menuitemradio"]'; interface IPosition { top?: number; @@ -95,8 +97,6 @@ interface IState { // this will allow the ContextMenu to manage its own focus using arrow keys as per the ARIA guidelines. @replaceableComponent("structures.ContextMenu") export class ContextMenu extends React.PureComponent { - private initialFocus: HTMLElement; - static defaultProps = { hasBackground: true, managed: true, @@ -107,24 +107,15 @@ export class ContextMenu extends React.PureComponent { this.state = { contextMenuElem: null, }; - - // persist what had focus when we got initialized so we can return it after - this.initialFocus = document.activeElement as HTMLElement; } - componentWillUnmount() { - // return focus to the thing which had it before us - this.initialFocus.focus(); - } - - private collectContextMenuRect = (element) => { + private collectContextMenuRect = (element: HTMLDivElement) => { // We don't need to clean up when unmounting, so ignore if (!element) return; - let first = element.querySelector('[role^="menuitem"]'); - if (!first) { - first = element.querySelector('[tab-index]'); - } + const first = element.querySelector(ARIA_MENU_ITEM_SELECTOR) + || element.querySelector('[tab-index]'); + if (first) { first.focus(); } @@ -381,8 +372,10 @@ export class ContextMenu extends React.PureComponent { ref={this.collectContextMenuRect} role={this.props.managed ? "menu" : undefined} > - { chevron } - { props.children } + + { chevron } + { props.children } + { background } diff --git a/src/components/views/spaces/SpaceCreateMenu.tsx b/src/components/views/spaces/SpaceCreateMenu.tsx index 4bb61d7ccb..11f68698ee 100644 --- a/src/components/views/spaces/SpaceCreateMenu.tsx +++ b/src/components/views/spaces/SpaceCreateMenu.tsx @@ -17,7 +17,8 @@ limitations under the License. import React, { useContext, useRef, useState } from "react"; import classNames from "classnames"; import { EventType, RoomType, RoomCreateTypeField } from "matrix-js-sdk/src/@types/event"; -import FocusLock from "react-focus-lock"; +import { Preset } from "matrix-js-sdk/src/@types/partials"; +import { ICreateRoomStateEvent } from "matrix-js-sdk/src/@types/requests"; import { _t } from "../../../languageHandler"; import AccessibleTooltipButton from "../elements/AccessibleTooltipButton"; @@ -33,8 +34,6 @@ import { UserTab } from "../dialogs/UserSettingsDialog"; import Field from "../elements/Field"; import withValidation from "../elements/Validation"; import { SpaceFeedbackPrompt } from "../../structures/SpaceRoomView"; -import { Preset } from "matrix-js-sdk/src/@types/partials"; -import { ICreateRoomStateEvent } from "matrix-js-sdk/src/@types/requests"; import RoomAliasField from "../elements/RoomAliasField"; const SpaceCreateMenuType = ({ title, description, className, onClick }) => { @@ -250,16 +249,14 @@ const SpaceCreateMenu = ({ onFinished }) => { wrapperClassName="mx_SpaceCreateMenu_wrapper" managed={false} > - - { - onFinished(); - defaultDispatcher.dispatch({ - action: Action.ViewUserSettings, - initialTabId: UserTab.Labs, - }); - }} /> - { body } - + { + onFinished(); + defaultDispatcher.dispatch({ + action: Action.ViewUserSettings, + initialTabId: UserTab.Labs, + }); + }} /> + { body } ; }; From b373b98d487a2ca3305113f62026ea6e52dd425b Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Wed, 6 Oct 2021 16:49:53 +0100 Subject: [PATCH 2/4] Simplify aria menu item roles/selectors --- src/components/structures/ContextMenu.tsx | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/components/structures/ContextMenu.tsx b/src/components/structures/ContextMenu.tsx index e2ec5d232c..dc6cbb59cd 100644 --- a/src/components/structures/ContextMenu.tsx +++ b/src/components/structures/ContextMenu.tsx @@ -44,9 +44,6 @@ function getOrCreateContainer(): HTMLDivElement { return container; } -const ARIA_MENU_ITEM_ROLES = new Set(["menuitem", "menuitemcheckbox", "menuitemradio"]); -const ARIA_MENU_ITEM_SELECTOR = '[role^="menuitem"], [role^="menuitemcheckbox"], [role^="menuitemradio"]'; - export interface IPosition { top?: number; bottom?: number; @@ -117,7 +114,7 @@ export class ContextMenu extends React.PureComponent { // We don't need to clean up when unmounting, so ignore if (!element) return; - const first = element.querySelector(ARIA_MENU_ITEM_SELECTOR) + const first = element.querySelector('[role^="menuitem"]') || element.querySelector('[tab-index]'); if (first) { @@ -196,7 +193,7 @@ export class ContextMenu extends React.PureComponent { descending = true; } } - } while (element && !ARIA_MENU_ITEM_ROLES.has(element.getAttribute("role"))); + } while (element && !element.getAttribute("role")?.startsWith("menuitem")); if (element) { (element as HTMLElement).focus(); From 047f182cd8f42d82424e960ba3d4ea768f484f3e Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 7 Oct 2021 11:04:10 +0100 Subject: [PATCH 3/4] focusLock only specific context menus --- src/components/structures/ContextMenu.tsx | 31 ++++++++++++++++--- .../views/directory/NetworkDropdown.tsx | 2 +- 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/components/structures/ContextMenu.tsx b/src/components/structures/ContextMenu.tsx index dc6cbb59cd..90d16a2eff 100644 --- a/src/components/structures/ContextMenu.tsx +++ b/src/components/structures/ContextMenu.tsx @@ -83,6 +83,10 @@ export interface IProps extends IPosition { // it will be mounted to a container at the root of the DOM. mountAsChild?: boolean; + // If specified, contents will be wrapped in a FocusLock, this is only needed if the context menu is being rendered + // within an existing FocusLock e.g inside a modal. + focusLock?: boolean; + // Function to be called on menu close onFinished(); // on resize callback @@ -98,6 +102,8 @@ interface IState { // this will allow the ContextMenu to manage its own focus using arrow keys as per the ARIA guidelines. @replaceableComponent("structures.ContextMenu") export class ContextMenu extends React.PureComponent { + private readonly initialFocus: HTMLElement; + static defaultProps = { hasBackground: true, managed: true, @@ -105,9 +111,18 @@ export class ContextMenu extends React.PureComponent { constructor(props, context) { super(props, context); + this.state = { contextMenuElem: null, }; + + // persist what had focus when we got initialized so we can return it after + this.initialFocus = document.activeElement as HTMLElement; + } + + componentWillUnmount() { + // return focus to the thing which had it before us after the unmount + this.initialFocus.focus(); } private collectContextMenuRect = (element: HTMLDivElement) => { @@ -371,6 +386,17 @@ export class ContextMenu extends React.PureComponent { ); } + let body = <> + { chevron } + { props.children } + ; + + if (props.focusLock) { + body = + { body } + ; + } + return (
{ ref={this.collectContextMenuRect} role={this.props.managed ? "menu" : undefined} > - - { chevron } - { props.children } - + { body }
{ background } diff --git a/src/components/views/directory/NetworkDropdown.tsx b/src/components/views/directory/NetworkDropdown.tsx index dbad2ca024..9a999625d7 100644 --- a/src/components/views/directory/NetworkDropdown.tsx +++ b/src/components/views/directory/NetworkDropdown.tsx @@ -268,7 +268,7 @@ const NetworkDropdown = ({ onOptionChange, protocols = {}, selectedServerName, s }; const buttonRect = handle.current.getBoundingClientRect(); - content = + content =
{ options } From 1c8bcce0ede25ac8c7a07a17aadb88ed3adeb4f1 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 7 Oct 2021 11:13:13 +0100 Subject: [PATCH 4/4] comment --- src/components/structures/ContextMenu.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/structures/ContextMenu.tsx b/src/components/structures/ContextMenu.tsx index 90d16a2eff..4250b5925b 100644 --- a/src/components/structures/ContextMenu.tsx +++ b/src/components/structures/ContextMenu.tsx @@ -121,7 +121,7 @@ export class ContextMenu extends React.PureComponent { } componentWillUnmount() { - // return focus to the thing which had it before us after the unmount + // return focus to the thing which had it before us this.initialFocus.focus(); }