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

Improve RovingTabIndex & Room List filtering performance #6987

Merged
merged 8 commits into from
Oct 26, 2021
Merged

Improve RovingTabIndex & Room List filtering performance #6987

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
f64898c
Attempt to improve RovingTabIndex performance
t3chguy Oct 19, 2021
d8232d7
Fix optimised algorithm
t3chguy Oct 20, 2021
29e3dcf
Fix RovingTabIndex Home/End handling with invisible elements
t3chguy Oct 20, 2021
b3e1a68
tidy
t3chguy Oct 20, 2021
e3baca6
delint
t3chguy Oct 20, 2021
7b80a36
Consolidate a lot of the roving focus traversal
t3chguy Oct 25, 2021
7bad338
add comment
t3chguy Oct 25, 2021
cca6580
delint
t3chguy Oct 25, 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
153 changes: 87 additions & 66 deletions src/accessibility/RovingTabIndex.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import React, {
useReducer,
Reducer,
Dispatch,
RefObject,
} from "react";

import { Key } from "../Keyboard";
Expand Down Expand Up @@ -63,7 +64,7 @@ const RovingTabIndexContext = createContext<IContext>({
});
RovingTabIndexContext.displayName = "RovingTabIndexContext";

enum Type {
export enum Type {
Register = "REGISTER",
Unregister = "UNREGISTER",
SetFocus = "SET_FOCUS",
Expand All @@ -76,73 +77,67 @@ interface IAction {
};
}

const reducer = (state: IState, action: IAction) => {
export const reducer = (state: IState, action: IAction) => {
switch (action.type) {
case Type.Register: {
if (state.refs.length === 0) {
// Our list of refs was empty, set activeRef to this first item
return {
...state,
activeRef: action.payload.ref,
refs: [action.payload.ref],
};
}

if (state.refs.includes(action.payload.ref)) {
return state; // already in refs, this should not happen
let left = 0;
let right = state.refs.length - 1;
let index = state.refs.length; // by default append to the end

// do a binary search to find the right slot
while (left <= right) {
index = Math.floor((left + right) / 2);
const ref = state.refs[index];

if (ref === action.payload.ref) {
return state; // already in refs, this should not happen
}

if (action.payload.ref.current.compareDocumentPosition(ref.current) & DOCUMENT_POSITION_PRECEDING) {
left = ++index;
} else {
right = index - 1;
}
}

// find the index of the first ref which is not preceding this one in DOM order
let newIndex = state.refs.findIndex(ref => {
return ref.current.compareDocumentPosition(action.payload.ref.current) & DOCUMENT_POSITION_PRECEDING;
});

if (newIndex < 0) {
newIndex = state.refs.length; // append to the end
if (!state.activeRef) {
// Our list of refs was empty, set activeRef to this first item
state.activeRef = action.payload.ref;
}

// update the refs list
return {
...state,
refs: [
...state.refs.slice(0, newIndex),
action.payload.ref,
...state.refs.slice(newIndex),
],
};
if (index < state.refs.length) {
state.refs.splice(index, 0, action.payload.ref);
} else {
state.refs.push(action.payload.ref);
}
return { ...state };
}

case Type.Unregister: {
// filter out the ref which we are removing
const refs = state.refs.filter(r => r !== action.payload.ref);
const oldIndex = state.refs.findIndex(r => r === action.payload.ref);

if (refs.length === state.refs.length) {
if (oldIndex === -1) {
return state; // already removed, this should not happen
}

if (state.activeRef === action.payload.ref) {
if (state.refs.splice(oldIndex, 1)[0] === state.activeRef) {
// we just removed the active ref, need to replace it
// pick the ref which is now in the index the old ref was in
const oldIndex = state.refs.findIndex(r => r === action.payload.ref);
return {
...state,
activeRef: oldIndex >= refs.length ? refs[refs.length - 1] : refs[oldIndex],
refs,
};
const len = state.refs.length;
state.activeRef = oldIndex >= len ? state.refs[len - 1] : state.refs[oldIndex];
}

// update the refs list
return {
...state,
refs,
};
return { ...state };
}

case Type.SetFocus: {
// update active ref
return {
...state,
activeRef: action.payload.ref,
};
state.activeRef = action.payload.ref;
return { ...state };
}

default:
return state;
}
Expand All @@ -151,13 +146,40 @@ const reducer = (state: IState, action: IAction) => {
interface IProps {
handleHomeEnd?: boolean;
handleUpDown?: boolean;
handleLeftRight?: boolean;
children(renderProps: {
onKeyDownHandler(ev: React.KeyboardEvent);
});
onKeyDown?(ev: React.KeyboardEvent, state: IState);
}

export const RovingTabIndexProvider: React.FC<IProps> = ({ children, handleHomeEnd, handleUpDown, onKeyDown }) => {
export const findSiblingElement = (
refs: RefObject<HTMLElement>[],
startIndex: number,
backwards = false,
): RefObject<HTMLElement> => {
if (backwards) {
for (let i = startIndex; i < refs.length && i >= 0; i--) {
if (refs[i].current.offsetParent !== null) {
return refs[i];
}
}
} else {
for (let i = startIndex; i < refs.length && i >= 0; i++) {
if (refs[i].current.offsetParent !== null) {
return refs[i];
}
}
}
};

export const RovingTabIndexProvider: React.FC<IProps> = ({
children,
handleHomeEnd,
handleUpDown,
handleLeftRight,
onKeyDown,
}) => {
const [state, dispatch] = useReducer<Reducer<IState, IAction>>(reducer, {
activeRef: null,
refs: [],
Expand All @@ -166,6 +188,13 @@ export const RovingTabIndexProvider: React.FC<IProps> = ({ children, handleHomeE
const context = useMemo<IContext>(() => ({ state, dispatch }), [state]);

const onKeyDownHandler = useCallback((ev) => {
if (onKeyDown) {
onKeyDown(ev, context.state);
if (ev.defaultPrevented) {
return;
}
}

let handled = false;
// Don't interfere with input default keydown behaviour
if (ev.target.tagName !== "INPUT" && ev.target.tagName !== "TEXTAREA") {
Expand All @@ -174,43 +203,37 @@ export const RovingTabIndexProvider: React.FC<IProps> = ({ children, handleHomeE
case Key.HOME:
if (handleHomeEnd) {
handled = true;
// move focus to first item
if (context.state.refs.length > 0) {
context.state.refs[0].current.focus();
}
// move focus to first (visible) item
findSiblingElement(context.state.refs, 0)?.current?.focus();
}
break;

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

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

case Key.ARROW_DOWN:
if (handleUpDown) {
case Key.ARROW_LEFT:
if ((ev.key === Key.ARROW_DOWN && handleUpDown) || (ev.key === Key.ARROW_LEFT && handleLeftRight)) {
handled = true;
if (context.state.refs.length > 0) {
const idx = context.state.refs.indexOf(context.state.activeRef);
if (idx < context.state.refs.length - 1) {
context.state.refs[idx + 1].current.focus();
}
findSiblingElement(context.state.refs, idx + 1, true)?.current?.focus();
}
}
break;
Expand All @@ -220,10 +243,8 @@ export const RovingTabIndexProvider: React.FC<IProps> = ({ children, handleHomeE
if (handled) {
ev.preventDefault();
ev.stopPropagation();
} else if (onKeyDown) {
return onKeyDown(ev, context.state);
}
}, [context.state, onKeyDown, handleHomeEnd, handleUpDown]);
}, [context.state, onKeyDown, handleHomeEnd, handleUpDown, handleLeftRight]);

return <RovingTabIndexContext.Provider value={context}>
{ children({ onKeyDownHandler }) }
Expand Down
13 changes: 2 additions & 11 deletions src/accessibility/Toolbar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ limitations under the License.

import React from "react";

import { IState, RovingTabIndexProvider } from "./RovingTabIndex";
import { RovingTabIndexProvider } from "./RovingTabIndex";
import { Key } from "../Keyboard";

interface IProps extends Omit<React.HTMLProps<HTMLDivElement>, "onKeyDown"> {
Expand All @@ -26,7 +26,7 @@ interface IProps extends Omit<React.HTMLProps<HTMLDivElement>, "onKeyDown"> {
// https://www.w3.org/TR/wai-aria-practices-1.1/#toolbar
// All buttons passed in children must use RovingTabIndex to set `onFocus`, `isActive`, `ref`
const Toolbar: React.FC<IProps> = ({ children, ...props }) => {
const onKeyDown = (ev: React.KeyboardEvent, state: IState) => {
const onKeyDown = (ev: React.KeyboardEvent) => {
const target = ev.target as HTMLElement;
// Don't interfere with input default keydown behaviour
if (target.tagName === "INPUT") return;
Expand All @@ -42,15 +42,6 @@ const Toolbar: React.FC<IProps> = ({ children, ...props }) => {
}
break;

case Key.ARROW_LEFT:
case Key.ARROW_RIGHT:
if (state.refs.length > 0) {
const i = state.refs.findIndex(r => r === state.activeRef);
const delta = ev.key === Key.ARROW_RIGHT ? 1 : -1;
state.refs.slice((i + delta) % state.refs.length)[0].current.focus();
}
break;

default:
handled = false;
}
Expand Down
2 changes: 2 additions & 0 deletions src/components/structures/ContextMenu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,8 @@ export class ContextMenu extends React.PureComponent<IProps, IState> {
let handled = true;

switch (ev.key) {
// XXX: this is imitating roving behaviour, it should really use the RovingTabIndex utils
// to inherit proper handling of unmount edge cases
case Key.TAB:
case Key.ESCAPE:
case Key.ARROW_LEFT: // close on left and right arrows too for when it is a context menu on a <Toolbar />
Expand Down
Loading