From 08393921830de13ce9f0f85e123b49963256f0e1 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Tue, 17 May 2022 23:20:09 -0500 Subject: [PATCH] rework roving tab index to use react-aria --- src/components/toolbar/Toolbar.test.tsx | 205 ++++++++++++++++++ src/components/toolbar/Toolbar.tsx | 66 ++---- src/components/toolbar/ToolbarButton.tsx | 47 ++++ src/components/toolbar/aria.ts | 76 +++++++ src/components/toolbar/state.ts | 29 +++ src/components/toolbar/types.ts | 9 + src/explorer/Explorer.tsx | 99 ++++----- src/toolbar/ActionButton.tsx | 21 +- src/toolbar/OpenFileButton.tsx | 23 +- src/toolbar/Toolbar.tsx | 36 ++- .../bluetooth/BluetoothButton.test.tsx | 4 +- .../buttons/bluetooth/BluetoothButton.tsx | 8 +- .../buttons/flash/FlashButton.test.tsx | 2 +- src/toolbar/buttons/flash/FlashButton.tsx | 6 +- src/toolbar/buttons/repl/ReplButton.test.tsx | 2 +- src/toolbar/buttons/repl/ReplButton.tsx | 6 +- src/toolbar/buttons/run/RunButton.test.tsx | 2 +- src/toolbar/buttons/run/RunButton.tsx | 6 +- src/toolbar/buttons/stop/StopButton.test.tsx | 2 +- src/toolbar/buttons/stop/StopButton.tsx | 6 +- src/utils/react.test.tsx | 119 ---------- src/utils/react.ts | 76 +------ 22 files changed, 495 insertions(+), 355 deletions(-) create mode 100644 src/components/toolbar/Toolbar.test.tsx create mode 100644 src/components/toolbar/ToolbarButton.tsx create mode 100644 src/components/toolbar/aria.ts create mode 100644 src/components/toolbar/state.ts create mode 100644 src/components/toolbar/types.ts delete mode 100644 src/utils/react.test.tsx diff --git a/src/components/toolbar/Toolbar.test.tsx b/src/components/toolbar/Toolbar.test.tsx new file mode 100644 index 00000000..703f65b2 --- /dev/null +++ b/src/components/toolbar/Toolbar.test.tsx @@ -0,0 +1,205 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2022 The Pybricks Authors + +import { RenderResult, cleanup } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import React from 'react'; +import { testRender } from '../../../test'; +import { Toolbar } from './Toolbar'; +import { ToolbarButton } from './ToolbarButton'; + +afterEach(() => { + cleanup(); +}); + +const TestToolbar: React.VoidFunctionComponent = () => { + return ( + + + + + + ); +}; + +function getButtons(toolbar: RenderResult): { + button1: HTMLElement; + button2: HTMLElement; + button3: HTMLElement; +} { + const button1 = toolbar.getByRole('button', { name: 'Button 1' }); + const button2 = toolbar.getByRole('button', { name: 'Button 2' }); + const button3 = toolbar.getByRole('button', { name: 'Button 3' }); + + return { button1, button2, button3 }; +} + +describe('Toolbar', () => { + it('should have toolbar role', () => { + const [toolbar] = testRender(); + + expect(toolbar.getByRole('toolbar', { name: 'Test Toolbar' })).toHaveClass( + 'test-class', + ); + }); + + it('should focus the first element by default', async () => { + const [toolbar] = testRender(); + + const { button1, button2, button3 } = getButtons(toolbar); + + expect(button1).not.toHaveAttribute('tabindex'); + expect(button2).toHaveAttribute('tabindex', '-1'); + expect(button3).toHaveAttribute('tabindex', '-1'); + + userEvent.tab(); + + expect(button1).toHaveFocus(); + expect(button1).not.toHaveAttribute('tabindex'); + expect(button2).toHaveAttribute('tabindex', '-1'); + expect(button3).toHaveAttribute('tabindex', '-1'); + }); + + it('should focus next with right arrow key', () => { + const [toolbar] = testRender(); + + const { button1, button2, button3 } = getButtons(toolbar); + + button1.focus(); + userEvent.keyboard('[ArrowRight]'); + + expect(button2).toHaveFocus(); + expect(button1).toHaveAttribute('tabindex', '-1'); + expect(button2).not.toHaveAttribute('tabindex'); + expect(button3).toHaveAttribute('tabindex', '-1'); + }); + + it('should focus previous with left arrow key', () => { + const [toolbar] = testRender(); + + const { button1, button2, button3 } = getButtons(toolbar); + + button3.focus(); + userEvent.keyboard('[ArrowLeft]'); + + expect(button2).toHaveFocus(); + expect(button1).toHaveAttribute('tabindex', '-1'); + expect(button2).not.toHaveAttribute('tabindex'); + expect(button3).toHaveAttribute('tabindex', '-1'); + }); + + it('should wrap focus next with right arrow key', () => { + const [toolbar] = testRender(); + + const { button1, button2, button3 } = getButtons(toolbar); + + button3.focus(); + userEvent.keyboard('[ArrowRight]'); + + expect(button1).toHaveFocus(); + expect(button1).not.toHaveAttribute('tabindex'); + expect(button2).toHaveAttribute('tabindex', '-1'); + expect(button3).toHaveAttribute('tabindex', '-1'); + }); + + it('should wrap focus previous with left arrow key', () => { + const [toolbar] = testRender(); + + const { button1, button2, button3 } = getButtons(toolbar); + + button1.focus(); + userEvent.keyboard('[ArrowLeft]'); + + expect(button3).toHaveFocus(); + expect(button1).toHaveAttribute('tabindex', '-1'); + expect(button2).toHaveAttribute('tabindex', '-1'); + expect(button3).not.toHaveAttribute('tabindex'); + }); + + it('should focus first with home key', () => { + const [toolbar] = testRender(); + + const { button1, button2, button3 } = getButtons(toolbar); + + button3.focus(); + userEvent.keyboard('[Home]'); + + expect(button1).toHaveFocus(); + expect(button1).not.toHaveAttribute('tabindex'); + expect(button2).toHaveAttribute('tabindex', '-1'); + expect(button3).toHaveAttribute('tabindex', '-1'); + }); + + it('should focus last with end key', () => { + const [toolbar] = testRender(); + + const { button1, button2, button3 } = getButtons(toolbar); + + button1.focus(); + userEvent.keyboard('[End]'); + + expect(button3).toHaveFocus(); + expect(button1).toHaveAttribute('tabindex', '-1'); + expect(button2).toHaveAttribute('tabindex', '-1'); + expect(button3).not.toHaveAttribute('tabindex'); + }); + + it('should not change focus with up arrow key', () => { + const [toolbar] = testRender(); + + const { button1, button2, button3 } = getButtons(toolbar); + + button2.focus(); + userEvent.keyboard('[ArrowUp]'); + + expect(button2).toHaveFocus(); + expect(button1).toHaveAttribute('tabindex', '-1'); + expect(button2).not.toHaveAttribute('tabindex'); + expect(button3).toHaveAttribute('tabindex', '-1'); + }); + + it('should not change focus with down arrow key', () => { + const [toolbar] = testRender(); + + const { button1, button2, button3 } = getButtons(toolbar); + + button2.focus(); + userEvent.keyboard('[ArrowDown]'); + + expect(button2).toHaveFocus(); + expect(button1).toHaveAttribute('tabindex', '-1'); + expect(button2).not.toHaveAttribute('tabindex'); + expect(button3).toHaveAttribute('tabindex', '-1'); + }); + + it('should not focus next item with tab key', () => { + const [toolbar] = testRender(); + + const { button1, button2, button3 } = getButtons(toolbar); + + button2.focus(); + userEvent.tab(); + + expect(document.body).toHaveFocus(); + expect(button1).toHaveAttribute('tabindex', '-1'); + expect(button2).not.toHaveAttribute('tabindex'); + expect(button3).toHaveAttribute('tabindex', '-1'); + }); + + it('should focus on click', () => { + const [toolbar] = testRender(); + + const { button1, button2, button3 } = getButtons(toolbar); + + userEvent.click(button2); + + expect(button2).toHaveFocus(); + expect(button1).toHaveAttribute('tabindex', '-1'); + expect(button2).not.toHaveAttribute('tabindex'); + expect(button3).toHaveAttribute('tabindex', '-1'); + }); +}); diff --git a/src/components/toolbar/Toolbar.tsx b/src/components/toolbar/Toolbar.tsx index 4d46753e..70a7fd5a 100644 --- a/src/components/toolbar/Toolbar.tsx +++ b/src/components/toolbar/Toolbar.tsx @@ -1,18 +1,19 @@ // SPDX-License-Identifier: MIT // Copyright (c) 2022 The Pybricks Authors -import React, { AriaAttributes, KeyboardEventHandler, useCallback } from 'react'; -import { FocusAction } from '../../utils/react'; +import React from 'react'; +import { FocusScope } from 'react-aria'; +import { useToolbar } from './aria'; +import { ToolbarStateContext, useToolbarState } from './state'; +import { AriaToolbarProps } from './types'; -type ToolbarProps = Pick & { +type ToolbarProps = Pick< + AriaToolbarProps, + 'aria-label' | 'aria-labelledby' | 'firstFocusableItemId' +> & { /** CSS class name for the tooltip element. */ className?: string; - /** Indicates that the toolbar has a vertical orientation. */ - vertical?: boolean; - /** Called when a keyboard event occurs. */ - onKeyboard?: (action: FocusAction) => void; }; - /** * An accessible toolbar component. * @@ -21,49 +22,16 @@ type ToolbarProps = Pick & { * * https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/toolbar_role */ -const Toolbar: React.FunctionComponent = ({ - children, - vertical, - onKeyboard, - ...divProps -}) => { - const handleKeyDown = useCallback>( - (e) => { - // ignore all key presses with modifiers - if (e.altKey || e.ctrlKey || e.shiftKey || e.metaKey) { - return; - } - - if (e.key === (vertical ? 'ArrowUp' : 'ArrowLeft')) { - onKeyboard?.(FocusAction.MovePrev); - } else if (e.key === (vertical ? 'ArrowDown' : 'ArrowRight')) { - onKeyboard?.(FocusAction.MoveNext); - } else if (e.key === 'Home') { - onKeyboard?.(FocusAction.MoveFirst); - } else if (e.key === 'End') { - onKeyboard?.(FocusAction.MoveLast); - } else { - // allow everything else to propagate. - return; - } - - // we consumed the key press - e.preventDefault(); - e.stopPropagation(); - }, - [vertical, onKeyboard], - ); +export const Toolbar: React.FunctionComponent = (props) => { + const { className, children } = props; + const state = useToolbarState(props); + const { toolbarProps } = useToolbar(props); return ( -
- {children} +
+ + {children} +
); }; - -export default Toolbar; diff --git a/src/components/toolbar/ToolbarButton.tsx b/src/components/toolbar/ToolbarButton.tsx new file mode 100644 index 00000000..33fc283a --- /dev/null +++ b/src/components/toolbar/ToolbarButton.tsx @@ -0,0 +1,47 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2022 The Pybricks Authors + +import { Classes } from '@blueprintjs/core'; +import { mergeProps } from '@react-aria/utils'; +import { AriaButtonProps } from '@react-types/button'; +import classNames from 'classnames'; +import React, { useRef } from 'react'; +import { FocusRing, useButton } from 'react-aria'; +import { useToolbarItemFocus } from './aria'; + +type ToolbarButtonProps = Pick< + AriaButtonProps, + 'aria-label' | 'aria-describedby' | 'id' | 'children' +> & { id: string; className?: string }; + +/** React component for toolbar item buttons. */ +export const ToolbarButton: React.FunctionComponent = (props) => { + const { className, children } = props; + const ref = useRef(null); + + const { toolbarItemFocusProps, excludeFromTabOrder } = useToolbarItemFocus(props); + + const { buttonProps, isPressed } = useButton( + mergeProps(props, { + excludeFromTabOrder, + }), + ref, + ); + + return ( + + + + ); +}; diff --git a/src/components/toolbar/aria.ts b/src/components/toolbar/aria.ts new file mode 100644 index 00000000..c34b2946 --- /dev/null +++ b/src/components/toolbar/aria.ts @@ -0,0 +1,76 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2022 The Pybricks Authors + +import { filterDOMProps } from '@react-aria/utils'; +import { + FocusEventHandler, + HTMLAttributes, + KeyboardEventHandler, + useCallback, + useContext, +} from 'react'; +import { FocusScope, mergeProps, useFocusManager } from 'react-aria'; +import { ToolbarStateContext } from './state'; +import { AriaToolbarProps } from './types'; + +// for doc comment link +FocusScope; + +type ToolbarAria = { + toolbarProps: HTMLAttributes; +}; + +/** React hook for creating toolbar element props. */ +export function useToolbar(props: AriaToolbarProps): ToolbarAria { + const domProps = filterDOMProps(props, { labelable: true }); + return { toolbarProps: mergeProps(domProps, { role: 'toolbar' }) }; +} + +type ToolbarItemFocusAria = { + toolbarItemFocusProps: HTMLAttributes; + excludeFromTabOrder: boolean; +}; + +/** + * React hook for creating toolbar item element props for focus management. + * + * Using this hook requires the current element to be inside of an + * {@link ToolbarStateContext} and to be inside of a {@link FocusScope}. + */ +export function useToolbarItemFocus(props: { id: string }): ToolbarItemFocusAria { + const { id } = props; + const state = useContext(ToolbarStateContext); + const focusManager = useFocusManager(); + + const onKeyDown = useCallback>( + (e) => { + switch (e.key) { + case 'ArrowLeft': + focusManager.focusPrevious({ wrap: true }); + break; + case 'ArrowRight': + focusManager.focusNext({ wrap: true }); + break; + case 'Home': + focusManager.focusFirst(); + break; + case 'End': + focusManager.focusLast(); + break; + default: + return; + } + }, + [focusManager], + ); + + const onFocus = useCallback>( + (e) => { + state.setLastFocusedItem(e.target.id); + }, + [state.setLastFocusedItem], + ); + const excludeFromTabOrder = id !== state.lastFocusedItem; + + return { toolbarItemFocusProps: { onKeyDown, onFocus }, excludeFromTabOrder }; +} diff --git a/src/components/toolbar/state.ts b/src/components/toolbar/state.ts new file mode 100644 index 00000000..4ec50b17 --- /dev/null +++ b/src/components/toolbar/state.ts @@ -0,0 +1,29 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2022 The Pybricks Authors + +import { createContext, useState } from 'react'; +import { ToolbarProps } from './types'; + +export type ToolbarState = Readonly<{ + /** The DOM id of the most recently focused item. */ + lastFocusedItem?: string; + /** + * Sets the most recently focused item. + * @param id The DOM id of the element. + */ + setLastFocusedItem: (id: string) => void; +}>; + +/** React hook for managing toolbar state. */ +export function useToolbarState(props: ToolbarProps): ToolbarState { + const { firstFocusableItemId } = props; + const [lastFocusedItem, setLastFocusedItem] = useState(firstFocusableItemId); + + return { lastFocusedItem, setLastFocusedItem }; +} + +/** React context for passing state from toolbar to toolbar items. */ +export const ToolbarStateContext = createContext({ + lastFocusedItem: 'default', + setLastFocusedItem: () => undefined, +}); diff --git a/src/components/toolbar/types.ts b/src/components/toolbar/types.ts new file mode 100644 index 00000000..6b20a727 --- /dev/null +++ b/src/components/toolbar/types.ts @@ -0,0 +1,9 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2022 The Pybricks Authors +import { AriaLabelingProps, DOMProps } from '@react-types/shared'; + +export type ToolbarProps = { + firstFocusableItemId: string; +}; + +export type AriaToolbarProps = ToolbarProps & DOMProps & AriaLabelingProps; diff --git a/src/explorer/Explorer.tsx b/src/explorer/Explorer.tsx index 0faf57c7..4f91332d 100644 --- a/src/explorer/Explorer.tsx +++ b/src/explorer/Explorer.tsx @@ -13,7 +13,8 @@ import { useHotkeys, } from '@blueprintjs/core'; import { I18n, useI18n } from '@shopify/react-i18n'; -import React, { RefObject, useCallback, useMemo, useRef, useState } from 'react'; +import React, { useCallback, useMemo, useState } from 'react'; +import { useId } from 'react-aria'; import { ControlledTreeEnvironment, LiveDescriptors, @@ -24,10 +25,10 @@ import { useTreeEnvironment, } from 'react-complex-tree'; import { useDispatch } from 'react-redux'; -import Toolbar from '../components/toolbar/Toolbar'; +import { Toolbar } from '../components/toolbar/Toolbar'; +import { useToolbarItemFocus } from '../components/toolbar/aria'; import { useSelector } from '../reducers'; import { isMacOS } from '../utils/os'; -import { useRovingTabIndex } from '../utils/react'; import { TreeItemContext, TreeItemData, renderers } from '../utils/tree-renderer'; import { explorerActivateFile, @@ -44,23 +45,20 @@ import { I18nId } from './i18n'; import NewFileWizard from './newFileWizard/NewFileWizard'; type ActionButtonProps = { + /** The DOM id for this instance. */ + id: string; /** The icon to use for the button. */ icon: IconName; /** The tooltip/title text. */ tooltip: string; - /** If false, prevent focus. Default is true. */ - focusable?: boolean; - /** Reference to the `