Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Refactor ContextMenu to use RovingTabIndex (more consistent keyboard navigation accessibility) #7353

Merged
merged 34 commits into from
Dec 17, 2021
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
7dfbe7a
Fix room list roving treeview
t3chguy Dec 10, 2021
e6e7885
Fix programmatic focus management in roving tab index not triggering …
t3chguy Dec 10, 2021
08b1fdb
Fix toolbar no longer handling left & right arrows
t3chguy Dec 10, 2021
7c226b9
Fix roving tab index focus tracking on interactive element like conte…
t3chguy Dec 10, 2021
4ddc97d
Fix thread list context menu roving
t3chguy Dec 10, 2021
459f155
add comment
t3chguy Dec 10, 2021
1784b44
fix comment
t3chguy Dec 10, 2021
64eef3a
Fix handling vertical arrows in the wrong direction
t3chguy Dec 13, 2021
6ce0798
iterate PR
t3chguy Dec 13, 2021
d82dbf6
Merge branch 'develop' of github.com:matrix-org/matrix-react-sdk into…
t3chguy Dec 13, 2021
2d0475d
delint
t3chguy Dec 14, 2021
bb1cbf7
Refactor ContextMenu to use RovingTabIndex
MadLittleMods Dec 14, 2021
1979f77
Restore some lost functionality for existing a ContextMenu
MadLittleMods Dec 14, 2021
283aece
Add to other Menu options
MadLittleMods Dec 14, 2021
ece5d7f
Refactor styled options to roving
MadLittleMods Dec 14, 2021
9c33bfd
Fix duplicate attributes
MadLittleMods Dec 14, 2021
4d635db
Make the UserMenu fully keyboard-navigable
MadLittleMods Dec 14, 2021
82527a5
Fix unmanaged context menus
MadLittleMods Dec 14, 2021
1099706
Explain UserMenu special navigation
MadLittleMods Dec 14, 2021
457606a
Fix lints
MadLittleMods Dec 14, 2021
b71e26d
Update jest snapshots
MadLittleMods Dec 14, 2021
c60bab7
Merge branch 'develop' into madlittlemods/roving-tab-index-context-menu
MadLittleMods Dec 14, 2021
e4cfa46
Remove extra changes from other PR
MadLittleMods Dec 14, 2021
5e803e1
Remove unnecessary syntax
MadLittleMods Dec 15, 2021
3af4eea
Use native :focus
MadLittleMods Dec 15, 2021
6acdd3e
Merge branch 'develop' into madlittlemods/roving-tab-index-context-menu
MadLittleMods Dec 15, 2021
9ea72d9
Force ContextMenu to re-render regardless if any rooms moved around f…
MadLittleMods Dec 15, 2021
904c0bd
Fix RoomSubList ContextMenu and generalize form interaction in Roving…
MadLittleMods Dec 15, 2021
ef101f3
Cleanup
MadLittleMods Dec 16, 2021
bd7891a
Remove dead code
MadLittleMods Dec 16, 2021
bae1f3f
Clarify the separation and delegation
MadLittleMods Dec 16, 2021
67c65a7
Input types can selected by CSS selectors too (simplify)
MadLittleMods Dec 16, 2021
a8899f9
Add comments
MadLittleMods Dec 16, 2021
ee05d76
Merge branch 'develop' into madlittlemods/roving-tab-index-context-menu
MadLittleMods Dec 16, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 37 additions & 2 deletions res/css/structures/_UserMenu.scss
Original file line number Diff line number Diff line change
Expand Up @@ -208,17 +208,52 @@ limitations under the License.
.mx_UserMenu_CustomStatusSection {
margin: 0 12px 8px;

.mx_UserMenu_CustomStatusSection_input {
.mx_UserMenu_CustomStatusSection_field {
position: relative;
display: flex;

> input {
&.mx_UserMenu_CustomStatusSection_field_focused,
MadLittleMods marked this conversation as resolved.
Show resolved Hide resolved
&.mx_UserMenu_CustomStatusSection_field_hasQuery {
.mx_UserMenu_CustomStatusSection_clear {
display: block;
}
}

> .mx_UserMenu_CustomStatusSection_input {
border: 1px solid $accent;
border-radius: 8px;
width: 100%;
}

> .mx_UserMenu_CustomStatusSection_clear {
display: none;

position: absolute;
top: 50%;
right: 0;
transform: translateY(-50%);

width: 16px;
height: 16px;
margin-right: 8px;
background-color: $quinary-content;
border-radius: 50%;

&::before {
content: "";
position: absolute;
width: inherit;
height: inherit;
mask-image: url('$(res)/img/feather-customised/x.svg');
mask-position: center;
mask-size: 12px;
mask-repeat: no-repeat;
background-color: $secondary-content;
}
}
t3chguy marked this conversation as resolved.
Show resolved Hide resolved
}


> p {
font-size: $font-12px;
line-height: $font-15px;
Expand Down
40 changes: 28 additions & 12 deletions src/accessibility/RovingTabIndex.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,8 @@ export const reducer = (state: IState, action: IAction) => {
}

case Type.SetFocus: {
// if the ref doesn't change just return the same object reference to skip a re-render
if (state.activeRef === action.payload.ref) return state;
// update active ref
state.activeRef = action.payload.ref;
return { ...state };
Expand Down Expand Up @@ -194,6 +196,7 @@ export const RovingTabIndexProvider: React.FC<IProps> = ({
}

let handled = false;
let focusRef: RefObject<HTMLElement>;
// Don't interfere with input default keydown behaviour
if (ev.target.tagName !== "INPUT" && ev.target.tagName !== "TEXTAREA") {
// check if we actually have any items
Expand All @@ -202,36 +205,38 @@ export const RovingTabIndexProvider: React.FC<IProps> = ({
if (handleHomeEnd) {
handled = true;
// move focus to first (visible) item
findSiblingElement(context.state.refs, 0)?.current?.focus();
focusRef = findSiblingElement(context.state.refs, 0);
}
break;

case Key.END:
if (handleHomeEnd) {
handled = true;
// move focus to last (visible) item
findSiblingElement(context.state.refs, context.state.refs.length - 1, true)?.current?.focus();
focusRef = findSiblingElement(context.state.refs, context.state.refs.length - 1, true);
}
break;

case Key.ARROW_UP:
case Key.ARROW_DOWN:
case Key.ARROW_RIGHT:
if ((ev.key === Key.ARROW_UP && handleUpDown) || (ev.key === Key.ARROW_RIGHT && handleLeftRight)) {
if ((ev.key === Key.ARROW_DOWN && handleUpDown) ||
(ev.key === Key.ARROW_RIGHT && handleLeftRight)
) {
handled = true;
if (context.state.refs.length > 0) {
const idx = context.state.refs.indexOf(context.state.activeRef);
findSiblingElement(context.state.refs, idx - 1)?.current?.focus();
focusRef = findSiblingElement(context.state.refs, idx + 1);
}
}
break;

case Key.ARROW_DOWN:
case Key.ARROW_UP:
case Key.ARROW_LEFT:
if ((ev.key === Key.ARROW_DOWN && handleUpDown) || (ev.key === Key.ARROW_LEFT && handleLeftRight)) {
if ((ev.key === Key.ARROW_UP && handleUpDown) || (ev.key === Key.ARROW_LEFT && handleLeftRight)) {
handled = true;
if (context.state.refs.length > 0) {
const idx = context.state.refs.indexOf(context.state.activeRef);
findSiblingElement(context.state.refs, idx + 1, true)?.current?.focus();
focusRef = findSiblingElement(context.state.refs, idx - 1, true);
}
}
break;
Expand All @@ -242,7 +247,18 @@ export const RovingTabIndexProvider: React.FC<IProps> = ({
ev.preventDefault();
ev.stopPropagation();
}
}, [context.state, onKeyDown, handleHomeEnd, handleUpDown, handleLeftRight]);

if (focusRef) {
focusRef.current?.focus();
// programmatic focus doesn't fire the onFocus handler, so we must do the do ourselves
dispatch({
type: Type.SetFocus,
payload: {
ref: focusRef,
},
});
}
}, [context, onKeyDown, handleHomeEnd, handleUpDown, handleLeftRight]);

return <RovingTabIndexContext.Provider value={context}>
{ children({ onKeyDownHandler }) }
Expand All @@ -254,9 +270,9 @@ export const RovingTabIndexProvider: React.FC<IProps> = ({
// onFocus should be called when the index gained focus in any manner
// isActive should be used to set tabIndex in a manner such as `tabIndex={isActive ? 0 : -1}`
// ref should be passed to a DOM node which will be used for DOM compareDocumentPosition
export const useRovingTabIndex = (inputRef?: Ref): [FocusHandler, boolean, Ref] => {
export const useRovingTabIndex = <T extends HTMLElement, >(inputRef?: RefObject<T>): [FocusHandler, boolean, RefObject<T>] => {
t3chguy marked this conversation as resolved.
Show resolved Hide resolved
const context = useContext(RovingTabIndexContext);
let ref = useRef<HTMLElement>(null);
let ref = useRef<T>(null);
MadLittleMods marked this conversation as resolved.
Show resolved Hide resolved

if (inputRef) {
// if we are given a ref, use it instead of ours
Expand All @@ -283,7 +299,7 @@ export const useRovingTabIndex = (inputRef?: Ref): [FocusHandler, boolean, Ref]
type: Type.SetFocus,
payload: { ref },
});
}, [ref, context]);
}, []); // eslint-disable-line react-hooks/exhaustive-deps

const isActive = context.state.activeRef === ref;
return [onFocus, isActive, ref];
Expand Down
2 changes: 1 addition & 1 deletion src/accessibility/Toolbar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ const Toolbar: React.FC<IProps> = ({ children, ...props }) => {
}
};

return <RovingTabIndexProvider handleHomeEnd={true} onKeyDown={onKeyDown}>
return <RovingTabIndexProvider handleHomeEnd handleLeftRight onKeyDown={onKeyDown}>
{ ({ onKeyDownHandler }) => <div {...props} onKeyDown={onKeyDownHandler} role="toolbar">
{ children }
</div> }
Expand Down
14 changes: 6 additions & 8 deletions src/accessibility/context_menu/MenuItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,9 @@ limitations under the License.

import React from "react";

import AccessibleButton from "../../components/views/elements/AccessibleButton";
import AccessibleTooltipButton from "../../components/views/elements/AccessibleTooltipButton";
import { RovingAccessibleButton, RovingAccessibleTooltipButton } from "../RovingTabIndex";

interface IProps extends React.ComponentProps<typeof AccessibleButton> {
interface IProps extends React.ComponentProps<typeof RovingAccessibleButton> {
label?: string;
tooltip?: string;
}
Expand All @@ -31,15 +30,14 @@ export const MenuItem: React.FC<IProps> = ({ children, label, tooltip, ...props
const ariaLabel = props["aria-label"] || label;

if (tooltip) {
return <AccessibleTooltipButton {...props} role="menuitem" tabIndex={-1} aria-label={ariaLabel} title={tooltip}>
return <RovingAccessibleTooltipButton {...props} role="menuitem" aria-label={ariaLabel} title={tooltip}>
{ children }
</AccessibleTooltipButton>;
</RovingAccessibleTooltipButton>;
}

return (
<AccessibleButton {...props} role="menuitem" tabIndex={-1} aria-label={ariaLabel}>
<RovingAccessibleButton {...props} role="menuitem" aria-label={ariaLabel}>
{ children }
</AccessibleButton>
</RovingAccessibleButton>
);
};

9 changes: 4 additions & 5 deletions src/accessibility/context_menu/MenuItemCheckbox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,26 +18,25 @@ limitations under the License.

import React from "react";

import AccessibleButton from "../../components/views/elements/AccessibleButton";
import { RovingAccessibleButton } from "../RovingTabIndex";

interface IProps extends React.ComponentProps<typeof AccessibleButton> {
interface IProps extends React.ComponentProps<typeof RovingAccessibleButton> {
label?: string;
active: boolean;
}

// Semantic component for representing a role=menuitemcheckbox
export const MenuItemCheckbox: React.FC<IProps> = ({ children, label, active, disabled, ...props }) => {
return (
<AccessibleButton
<RovingAccessibleButton
{...props}
role="menuitemcheckbox"
aria-checked={active}
aria-disabled={disabled}
disabled={disabled}
tabIndex={-1}
aria-label={label}
>
{ children }
</AccessibleButton>
</RovingAccessibleButton>
);
};
9 changes: 4 additions & 5 deletions src/accessibility/context_menu/MenuItemRadio.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,26 +18,25 @@ limitations under the License.

import React from "react";

import AccessibleButton from "../../components/views/elements/AccessibleButton";
import { RovingAccessibleButton } from "../RovingTabIndex";

interface IProps extends React.ComponentProps<typeof AccessibleButton> {
interface IProps extends React.ComponentProps<typeof RovingAccessibleButton> {
label?: string;
active: boolean;
}

// Semantic component for representing a role=menuitemradio
export const MenuItemRadio: React.FC<IProps> = ({ children, label, active, disabled, ...props }) => {
return (
<AccessibleButton
<RovingAccessibleButton
{...props}
role="menuitemradio"
aria-checked={active}
aria-disabled={disabled}
disabled={disabled}
tabIndex={-1}
aria-label={label}
>
{ children }
</AccessibleButton>
</RovingAccessibleButton>
);
};
7 changes: 6 additions & 1 deletion src/accessibility/context_menu/StyledMenuItemCheckbox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ limitations under the License.
import React from "react";

import { Key } from "../../Keyboard";
import { useRovingTabIndex } from "../RovingTabIndex";
import StyledCheckbox from "../../components/views/elements/StyledCheckbox";

interface IProps extends React.ComponentProps<typeof StyledCheckbox> {
Expand All @@ -29,6 +30,8 @@ interface IProps extends React.ComponentProps<typeof StyledCheckbox> {

// Semantic component for representing a styled role=menuitemcheckbox
export const StyledMenuItemCheckbox: React.FC<IProps> = ({ children, label, onChange, onClose, ...props }) => {
const [onFocus, isActive, ref] = useRovingTabIndex<HTMLInputElement>();

const onKeyDown = (e: React.KeyboardEvent) => {
if (e.key === Key.ENTER || e.key === Key.SPACE) {
e.stopPropagation();
Expand All @@ -52,11 +55,13 @@ export const StyledMenuItemCheckbox: React.FC<IProps> = ({ children, label, onCh
<StyledCheckbox
{...props}
role="menuitemcheckbox"
tabIndex={-1}
aria-label={label}
onChange={onChange}
onKeyDown={onKeyDown}
onKeyUp={onKeyUp}
onFocus={onFocus}
inputRef={ref}
tabIndex={isActive ? 0 : -1}
>
{ children }
</StyledCheckbox>
Expand Down
7 changes: 6 additions & 1 deletion src/accessibility/context_menu/StyledMenuItemRadio.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ limitations under the License.
import React from "react";

import { Key } from "../../Keyboard";
import { useRovingTabIndex } from "../RovingTabIndex";
import StyledRadioButton from "../../components/views/elements/StyledRadioButton";

interface IProps extends React.ComponentProps<typeof StyledRadioButton> {
Expand All @@ -29,6 +30,8 @@ interface IProps extends React.ComponentProps<typeof StyledRadioButton> {

// Semantic component for representing a styled role=menuitemradio
export const StyledMenuItemRadio: React.FC<IProps> = ({ children, label, onChange, onClose, ...props }) => {
const [onFocus, isActive, ref] = useRovingTabIndex<HTMLInputElement>();

const onKeyDown = (e: React.KeyboardEvent) => {
if (e.key === Key.ENTER || e.key === Key.SPACE) {
e.stopPropagation();
Expand All @@ -52,11 +55,13 @@ export const StyledMenuItemRadio: React.FC<IProps> = ({ children, label, onChang
<StyledRadioButton
{...props}
role="menuitemradio"
tabIndex={-1}
aria-label={label}
onChange={onChange}
onKeyDown={onKeyDown}
onKeyUp={onKeyUp}
onFocus={onFocus}
inputRef={ref}
tabIndex={isActive ? 0 : -1}
>
{ children }
</StyledRadioButton>
Expand Down
Loading