From a556121c66a12daadc9a4e6c8df48c4e191072ad Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Mon, 30 May 2022 16:04:42 +0200 Subject: [PATCH 1/5] Replace typeahead with mnemonics --- src/ActionMenu.tsx | 4 +- src/__tests__/hooks/useMnemonics.test.tsx | 132 ++++++++++++++++++++ src/hooks/index.ts | 1 + src/hooks/useMnemonics.ts | 65 ++++++++++ src/stories/ActionMenu/fixtures.stories.tsx | 4 +- 5 files changed, 202 insertions(+), 4 deletions(-) create mode 100644 src/__tests__/hooks/useMnemonics.test.tsx create mode 100644 src/hooks/useMnemonics.ts diff --git a/src/ActionMenu.tsx b/src/ActionMenu.tsx index fa07408c99e..b471530132c 100644 --- a/src/ActionMenu.tsx +++ b/src/ActionMenu.tsx @@ -3,7 +3,7 @@ import {useSSRSafeId} from '@react-aria/ssr' import {TriangleDownIcon} from '@primer/octicons-react' import {AnchoredOverlay, AnchoredOverlayProps} from './AnchoredOverlay' import {OverlayProps} from './Overlay' -import {useProvidedRefOrCreate, useProvidedStateOrCreate, useMenuInitialFocus, useTypeaheadFocus} from './hooks' +import {useProvidedRefOrCreate, useProvidedStateOrCreate, useMenuInitialFocus, useMnemonics} from './hooks' import {Divider} from './ActionList/Divider' import {ActionListContainerContext} from './ActionList/ActionListContainerContext' import {Button, ButtonProps} from './Button' @@ -112,7 +112,7 @@ const Overlay: React.FC = ({children, align = 'start', ...over const containerRef = React.createRef() const {openWithFocus} = useMenuInitialFocus(open, onOpen, containerRef) - useTypeaheadFocus(open, containerRef) + useMnemonics(open, containerRef) return ( null, + hasInput = false, + refNotAttached = false +}: { + onSelect?: (event: React.KeyboardEvent) => void + hasInput?: boolean + refNotAttached?: boolean +}) => { + const containerRef = React.createRef() + useMnemonics(true, containerRef) // hard coding open=true for test + + return ( + <> +
+ {/* eslint-disable-next-line jsx-a11y/no-autofocus */} + {hasInput && } + + + + + + not focusable +
+ + ) +} + +describe('useTypeaheadFocus', () => { + afterEach(cleanup) + + it('First element: when b is pressed, it should move focus the "b"utton 1', () => { + const {getByTestId, getByText} = render() + const container = getByTestId('container') + + fireEvent.keyDown(container, {key: 'b', code: 'b'}) + expect(getByText('button 1')).toEqual(document.activeElement) + }) + + it('Not first element: when t is pressed, it should move focus the "t"hird button', () => { + const {getByTestId, getByText} = render() + const container = getByTestId('container') + + fireEvent.keyDown(container, {key: 't', code: 't'}) + expect(getByText('third button')).toEqual(document.activeElement) + }) + + it('Case insensitive: when B is pressed, it should move focus the "b"utton 1', () => { + const {getByTestId, getByText} = render() + const container = getByTestId('container') + + fireEvent.keyDown(container, {key: 'B', code: 'B'}) + expect(getByText('button 1')).toEqual(document.activeElement) + }) + + it('Repeating letter: when b is pressed repeatedly, it should wrap through the buttons starting with "b", skipping the disabled button', () => { + const {getByTestId, getByText} = render() + const container = getByTestId('container') + + fireEvent.keyDown(container, {key: 'b', code: 'b'}) + expect(getByText('button 1')).toEqual(document.activeElement) + + fireEvent.keyDown(container, {key: 'b', code: 'b'}) + expect(getByText('Button 2')).toEqual(document.activeElement) + + fireEvent.keyDown(container, {key: 'b', code: 'b'}) + expect(getByText('button 5')).toEqual(document.activeElement) + + // should cycle back to start of the list + fireEvent.keyDown(container, {key: 'b', code: 'b'}) + expect(getByText('button 1')).toEqual(document.activeElement) + }) + + it('Space: when user presses Space, it should select the option', () => { + const mockFunction = jest.fn() + const {getByTestId, getByText} = render() + + const container = getByTestId('container') + fireEvent.keyDown(container, {key: 't', code: 't'}) + + const thirdButton = getByText('third button') + expect(thirdButton).toEqual(document.activeElement) + fireEvent.keyDown(thirdButton, {key: ' ', code: 'Space'}) + expect(mockFunction).toHaveBeenCalled() + }) + + it('Enter: when user is presses Enter, it should select the option', () => { + jest.useFakeTimers() + const mockFunction = jest.fn() + const {getByTestId, getByText} = render() + + const container = getByTestId('container') + fireEvent.keyDown(container, {key: 't', code: 't'}) + + const thirdButton = getByText('third button') + expect(thirdButton).toEqual(document.activeElement) + + fireEvent.keyDown(thirdButton, {key: 'Enter', code: 'Enter'}) + expect(mockFunction).toHaveBeenCalled() + }) + + it('Shortcuts: when a modifier is used, typeahead should not do anything', () => { + const {getByTestId, getByText} = render() + const container = getByTestId('container') + + fireEvent.keyDown(container, {metaKey: true, key: 'b', code: 'b'}) + expect(getByText('button 1')).not.toEqual(document.activeElement) + }) + + it('TextInput: when an textinput has focus, typeahead should not do anything', async () => { + const {getByTestId, getByPlaceholderText} = render() + const container = getByTestId('container') + + const input = getByPlaceholderText('Filter options') + expect(input).toEqual(document.activeElement) + + fireEvent.keyDown(container, {key: 'b', code: 'b'}) + expect(input).toEqual(document.activeElement) + }) + + it('Missing ref: when a ref is not attached, typeahead should break the component', async () => { + const {getByTestId, getByText} = render() + const container = getByTestId('container') + + fireEvent.keyDown(container, {key: 'b', code: 'b'}) + expect(getByText('button 1')).not.toEqual(document.activeElement) + }) +}) diff --git a/src/hooks/index.ts b/src/hooks/index.ts index a6dd3f13016..6f7570fa062 100644 --- a/src/hooks/index.ts +++ b/src/hooks/index.ts @@ -12,3 +12,4 @@ export {useRenderForcingRef} from './useRenderForcingRef' export {useProvidedStateOrCreate} from './useProvidedStateOrCreate' export {useMenuInitialFocus} from './useMenuInitialFocus' export {useTypeaheadFocus} from './useTypeaheadFocus' +export {useMnemonics} from './useMnemonics' diff --git a/src/hooks/useMnemonics.ts b/src/hooks/useMnemonics.ts new file mode 100644 index 00000000000..0d01085808b --- /dev/null +++ b/src/hooks/useMnemonics.ts @@ -0,0 +1,65 @@ +import React from 'react' +import {iterateFocusableElements} from '@primer/behaviors/utils' +import {useProvidedRefOrCreate} from './useProvidedRefOrCreate' + +/* + * A mnemonic indicates to the user which key to press (single) + * to activate a command or navigate to a component + * typically appearing in a menu title, menu item, or the text of a button. + */ + +export const useMnemonics = (open: boolean, providedRef?: React.RefObject) => { + const containerRef = useProvidedRefOrCreate(providedRef) + + React.useEffect(() => { + if (!open || !containerRef.current) return + const container = containerRef.current + + const handler = (event: KeyboardEvent) => { + // skip if a TextInput has focus + const activeElement = document.activeElement as HTMLElement + if (activeElement.tagName === 'INPUT') return + + // skip if used with modifier to preserve shortcuts like ⌘ + F + const hasModifier = event.ctrlKey || event.altKey || event.metaKey + if (hasModifier) return + + // skip if it's not a alphabet key + if (!isAlphabetKey(event)) return + + // if this is a typeahead event, don't propagate outside of menu + event.stopPropagation() + + const query = event.key.toLowerCase() + + let elementToFocus: HTMLElement | undefined + + const focusableItems = [...iterateFocusableElements(container)] + + const itemsStartingWithKey = focusableItems.filter(item => { + return item.textContent?.toLowerCase().trim().startsWith(query) + }) + + const currentActiveIndex = itemsStartingWithKey.indexOf(activeElement) + + // If the last element is already selected, cycle through the list + if (currentActiveIndex === itemsStartingWithKey.length - 1) { + elementToFocus = itemsStartingWithKey[0] + } else { + elementToFocus = itemsStartingWithKey.find((item, index) => { + return index > currentActiveIndex + }) + } + elementToFocus?.focus() + } + + container.addEventListener('keydown', handler) + return () => container.removeEventListener('keydown', handler) + }, [open, containerRef]) + + const isAlphabetKey = (event: KeyboardEvent) => { + return event.key.length === 1 && /[a-z\d]/i.test(event.key) + } + + return {containerRef} +} diff --git a/src/stories/ActionMenu/fixtures.stories.tsx b/src/stories/ActionMenu/fixtures.stories.tsx index 8e74c3677f9..e41457e7157 100644 --- a/src/stories/ActionMenu/fixtures.stories.tsx +++ b/src/stories/ActionMenu/fixtures.stories.tsx @@ -655,10 +655,10 @@ export function OverlayProps(): JSX.Element { ) } -export function TypeaheadTest(): JSX.Element { +export function MnemonicsTest(): JSX.Element { return ( <> -

Story to test typeahead

+

Menu mnemonics

Menu From 607129c505dfb2e1f98cc698a6044c248ab6cf2a Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Mon, 30 May 2022 16:18:33 +0200 Subject: [PATCH 2/5] add aria-keyshortcuts as part of useMnemonics --- src/__tests__/hooks/useMnemonics.test.tsx | 11 +++ src/hooks/useMnemonics.ts | 82 ++++++++++++++--------- 2 files changed, 61 insertions(+), 32 deletions(-) diff --git a/src/__tests__/hooks/useMnemonics.test.tsx b/src/__tests__/hooks/useMnemonics.test.tsx index 726e35ec1db..e594809c6ff 100644 --- a/src/__tests__/hooks/useMnemonics.test.tsx +++ b/src/__tests__/hooks/useMnemonics.test.tsx @@ -75,6 +75,17 @@ describe('useTypeaheadFocus', () => { expect(getByText('button 1')).toEqual(document.activeElement) }) + it('aria-keyshortcuts: it should add aria-keyshortcuts to focusable items', () => { + const {getByText} = render() + + expect(getByText('button 1')).toHaveAttribute('aria-keyshortcuts', 'b') + expect(getByText('Button 2')).toHaveAttribute('aria-keyshortcuts', 'b') + expect(getByText('third button')).toHaveAttribute('aria-keyshortcuts', 't') + expect(getByText('fourth button is disabled')).not.toHaveAttribute('aria-keyshortcuts') + expect(getByText('button 5')).toHaveAttribute('aria-keyshortcuts', 'b') + expect(getByText('not focusable')).not.toHaveAttribute('aria-keyshortcuts') + }) + it('Space: when user presses Space, it should select the option', () => { const mockFunction = jest.fn() const {getByTestId, getByText} = render() diff --git a/src/hooks/useMnemonics.ts b/src/hooks/useMnemonics.ts index 0d01085808b..f9f0061c61d 100644 --- a/src/hooks/useMnemonics.ts +++ b/src/hooks/useMnemonics.ts @@ -11,51 +11,69 @@ import {useProvidedRefOrCreate} from './useProvidedRefOrCreate' export const useMnemonics = (open: boolean, providedRef?: React.RefObject) => { const containerRef = useProvidedRefOrCreate(providedRef) - React.useEffect(() => { - if (!open || !containerRef.current) return - const container = containerRef.current + React.useEffect( + function addAriaKeyshortcuts() { + if (!open || !containerRef.current) return + const container = containerRef.current - const handler = (event: KeyboardEvent) => { - // skip if a TextInput has focus - const activeElement = document.activeElement as HTMLElement - if (activeElement.tagName === 'INPUT') return + const focusableItems = [...iterateFocusableElements(container)] - // skip if used with modifier to preserve shortcuts like ⌘ + F - const hasModifier = event.ctrlKey || event.altKey || event.metaKey - if (hasModifier) return + focusableItems.map(item => { + const firstLetter = item.textContent?.toLowerCase()[0] + if (firstLetter) item.setAttribute('aria-keyshortcuts', firstLetter) + }) + }, + [open, containerRef] + ) - // skip if it's not a alphabet key - if (!isAlphabetKey(event)) return + React.useEffect( + function handleKeyDown() { + if (!open || !containerRef.current) return + const container = containerRef.current - // if this is a typeahead event, don't propagate outside of menu - event.stopPropagation() + const handler = (event: KeyboardEvent) => { + // skip if a TextInput has focus + const activeElement = document.activeElement as HTMLElement + if (activeElement.tagName === 'INPUT') return - const query = event.key.toLowerCase() + // skip if used with modifier to preserve shortcuts like ⌘ + F + const hasModifier = event.ctrlKey || event.altKey || event.metaKey + if (hasModifier) return - let elementToFocus: HTMLElement | undefined + // skip if it's not a alphabet key + if (!isAlphabetKey(event)) return - const focusableItems = [...iterateFocusableElements(container)] + // if this is a typeahead event, don't propagate outside of menu + event.stopPropagation() - const itemsStartingWithKey = focusableItems.filter(item => { - return item.textContent?.toLowerCase().trim().startsWith(query) - }) + const query = event.key.toLowerCase() + + let elementToFocus: HTMLElement | undefined - const currentActiveIndex = itemsStartingWithKey.indexOf(activeElement) + const focusableItems = [...iterateFocusableElements(container)] - // If the last element is already selected, cycle through the list - if (currentActiveIndex === itemsStartingWithKey.length - 1) { - elementToFocus = itemsStartingWithKey[0] - } else { - elementToFocus = itemsStartingWithKey.find((item, index) => { - return index > currentActiveIndex + const itemsStartingWithKey = focusableItems.filter(item => { + return item.textContent?.toLowerCase().trim().startsWith(query) }) + + const currentActiveIndex = itemsStartingWithKey.indexOf(activeElement) + + // If the last element is already selected, cycle through the list + if (currentActiveIndex === itemsStartingWithKey.length - 1) { + elementToFocus = itemsStartingWithKey[0] + } else { + elementToFocus = itemsStartingWithKey.find((item, index) => { + return index > currentActiveIndex + }) + } + elementToFocus?.focus() } - elementToFocus?.focus() - } - container.addEventListener('keydown', handler) - return () => container.removeEventListener('keydown', handler) - }, [open, containerRef]) + container.addEventListener('keydown', handler) + return () => container.removeEventListener('keydown', handler) + }, + [open, containerRef] + ) const isAlphabetKey = (event: KeyboardEvent) => { return event.key.length === 1 && /[a-z\d]/i.test(event.key) From b2047a334cf536ff129700baee140fcbb79de4eb Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Mon, 30 May 2022 16:20:27 +0200 Subject: [PATCH 3/5] Create gold-falcons-shake.md --- .changeset/gold-falcons-shake.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/gold-falcons-shake.md diff --git a/.changeset/gold-falcons-shake.md b/.changeset/gold-falcons-shake.md new file mode 100644 index 00000000000..63b29344087 --- /dev/null +++ b/.changeset/gold-falcons-shake.md @@ -0,0 +1,5 @@ +--- +"@primer/react": patch +--- + +ActionMenu: Replace typeahead behavior with single key mnemonics From 793e5374ba1eef065f0380fccf4b2fece4510e01 Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Mon, 30 May 2022 16:55:36 +0200 Subject: [PATCH 4/5] support user configured aria-keyshortcuts --- src/__tests__/hooks/useMnemonics.test.tsx | 25 ++++++++++++++++++++- src/hooks/useMnemonics.ts | 19 +++++++++++----- src/stories/ActionMenu/fixtures.stories.tsx | 18 +++++++++++++++ 3 files changed, 55 insertions(+), 7 deletions(-) diff --git a/src/__tests__/hooks/useMnemonics.test.tsx b/src/__tests__/hooks/useMnemonics.test.tsx index e594809c6ff..d92352408f5 100644 --- a/src/__tests__/hooks/useMnemonics.test.tsx +++ b/src/__tests__/hooks/useMnemonics.test.tsx @@ -24,6 +24,9 @@ const Fixture = ({ + not focusable @@ -75,14 +78,34 @@ describe('useTypeaheadFocus', () => { expect(getByText('button 1')).toEqual(document.activeElement) }) + it('User defined aria-keyshortcuts: Should catch all shortcuts defined by user', () => { + const {getByTestId, getByText} = render() + const container = getByTestId('container') + + fireEvent.keyDown(container, {key: '6', code: '6'}) + expect(getByText('button 6')).toEqual(document.activeElement) + + // send focus elsewhere + fireEvent.keyDown(container, {key: 't', code: 't'}) + expect(getByText('third button')).toEqual(document.activeElement) + + fireEvent.keyDown(container, {key: 'e', code: 'e'}) + expect(getByText('button 6')).toEqual(document.activeElement) + }) + it('aria-keyshortcuts: it should add aria-keyshortcuts to focusable items', () => { const {getByText} = render() expect(getByText('button 1')).toHaveAttribute('aria-keyshortcuts', 'b') expect(getByText('Button 2')).toHaveAttribute('aria-keyshortcuts', 'b') expect(getByText('third button')).toHaveAttribute('aria-keyshortcuts', 't') - expect(getByText('fourth button is disabled')).not.toHaveAttribute('aria-keyshortcuts') expect(getByText('button 5')).toHaveAttribute('aria-keyshortcuts', 'b') + + // don't overwrite aria-keyshortcuts if it's already defined + expect(getByText('button 6')).toHaveAttribute('aria-keyshortcuts', '6 E') + + // not focusable items should not have aria-keyshortcuts + expect(getByText('fourth button is disabled')).not.toHaveAttribute('aria-keyshortcuts') expect(getByText('not focusable')).not.toHaveAttribute('aria-keyshortcuts') }) diff --git a/src/hooks/useMnemonics.ts b/src/hooks/useMnemonics.ts index f9f0061c61d..214c2d2cd4a 100644 --- a/src/hooks/useMnemonics.ts +++ b/src/hooks/useMnemonics.ts @@ -19,6 +19,9 @@ export const useMnemonics = (open: boolean, providedRef?: React.RefObject { + // if item already has aria-keyshortcuts defined by user, skip + if (item.getAttribute('aria-keyshortcuts')) return + const firstLetter = item.textContent?.toLowerCase()[0] if (firstLetter) item.setAttribute('aria-keyshortcuts', firstLetter) }) @@ -52,17 +55,21 @@ export const useMnemonics = (open: boolean, providedRef?: React.RefObject { - return item.textContent?.toLowerCase().trim().startsWith(query) + const itemsMatchingKey = focusableItems.filter(item => { + const keyshortcuts = item + .getAttribute('aria-keyshortcuts') + ?.split(' ') + .map(shortcut => shortcut.toLowerCase()) + return keyshortcuts && keyshortcuts.includes(query) }) - const currentActiveIndex = itemsStartingWithKey.indexOf(activeElement) + const currentActiveIndex = itemsMatchingKey.indexOf(activeElement) // If the last element is already selected, cycle through the list - if (currentActiveIndex === itemsStartingWithKey.length - 1) { - elementToFocus = itemsStartingWithKey[0] + if (currentActiveIndex === itemsMatchingKey.length - 1) { + elementToFocus = itemsMatchingKey[0] } else { - elementToFocus = itemsStartingWithKey.find((item, index) => { + elementToFocus = itemsMatchingKey.find((item, index) => { return index > currentActiveIndex }) } diff --git a/src/stories/ActionMenu/fixtures.stories.tsx b/src/stories/ActionMenu/fixtures.stories.tsx index e41457e7157..36fc4933577 100644 --- a/src/stories/ActionMenu/fixtures.stories.tsx +++ b/src/stories/ActionMenu/fixtures.stories.tsx @@ -671,6 +671,24 @@ export function MnemonicsTest(): JSX.Element { Order Group + + User defined + + + e + + + Disabled From d9a85d0c3430e090756cf7af2f51a2515379febe Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Wed, 29 Jun 2022 18:34:19 +0530 Subject: [PATCH 5/5] delete unused useTypeaheadFocus --- .../hooks/useMenuTypeaheadFocus.test.tsx | 196 ------------------ src/hooks/index.ts | 1 - src/hooks/useTypeaheadFocus.ts | 94 --------- 3 files changed, 291 deletions(-) delete mode 100644 src/__tests__/hooks/useMenuTypeaheadFocus.test.tsx delete mode 100644 src/hooks/useTypeaheadFocus.ts diff --git a/src/__tests__/hooks/useMenuTypeaheadFocus.test.tsx b/src/__tests__/hooks/useMenuTypeaheadFocus.test.tsx deleted file mode 100644 index a6e0ee7ef17..00000000000 --- a/src/__tests__/hooks/useMenuTypeaheadFocus.test.tsx +++ /dev/null @@ -1,196 +0,0 @@ -import React from 'react' -import {render, fireEvent, cleanup} from '@testing-library/react' -import {useTypeaheadFocus} from '../../hooks' -import {TYPEAHEAD_TIMEOUT} from '../../hooks/useTypeaheadFocus' - -const Component = ({ - onSelect = () => null, - hasInput = false, - refNotAttached = false -}: { - onSelect?: (event: React.KeyboardEvent) => void - hasInput?: boolean - refNotAttached?: boolean -}) => { - const containerRef = React.createRef() - useTypeaheadFocus(true, containerRef) // hard coding open=true for test - - return ( - <> -
- {/* eslint-disable-next-line jsx-a11y/no-autofocus */} - {hasInput && } - - - - - - not focusable -
- - ) -} - -describe('useTypeaheadFocus', () => { - afterEach(cleanup) - - it('First element: when b is pressed, it should move focus the "b"utton 1', () => { - const {getByTestId, getByText} = render() - const container = getByTestId('container') - - fireEvent.keyDown(container, {key: 'b', code: 'b'}) - expect(getByText('button 1')).toEqual(document.activeElement) - }) - - it('Not first element: when t is pressed, it should move focus the "t"hird button', () => { - const {getByTestId, getByText} = render() - const container = getByTestId('container') - - fireEvent.keyDown(container, {key: 't', code: 't'}) - expect(getByText('third button')).toEqual(document.activeElement) - }) - - it('Case insensitive: when B is pressed, it should move focus the "b"utton 1', () => { - const {getByTestId, getByText} = render() - const container = getByTestId('container') - - fireEvent.keyDown(container, {key: 'B', code: 'B'}) - expect(getByText('button 1')).toEqual(document.activeElement) - }) - - it('Repeating letter: when b is pressed repeatedly, it should wrap through the buttons starting with "b", skipping the disabled button', () => { - const {getByTestId, getByText} = render() - const container = getByTestId('container') - - fireEvent.keyDown(container, {key: 'b', code: 'b'}) - expect(getByText('button 1')).toEqual(document.activeElement) - - fireEvent.keyDown(container, {key: 'b', code: 'b'}) - expect(getByText('Button 2')).toEqual(document.activeElement) - - fireEvent.keyDown(container, {key: 'b', code: 'b'}) - expect(getByText('button 4')).toEqual(document.activeElement) - - fireEvent.keyDown(container, {key: 'b', code: 'b'}) - expect(getByText('button 1')).toEqual(document.activeElement) - }) - - it('Timeout: when presses b, waits and presses t, it should move focus the "t"hird button', async () => { - jest.useFakeTimers() - const {getByTestId, getByText} = render() - const container = getByTestId('container') - - fireEvent.keyDown(container, {key: 'b', code: 'b'}) - expect(getByText('button 1')).toEqual(document.activeElement) - - // if we press t now, the query would be "bt", - // and focus will stay wherever it is - fireEvent.keyDown(container, {key: 't', code: 't'}) - expect(getByText('button 1')).toEqual(document.activeElement) - - // but, if we simulate typeahead timeout, then type t - // it should jump to the third button - jest.advanceTimersByTime(TYPEAHEAD_TIMEOUT) - fireEvent.keyDown(container, {key: 't', code: 't'}) - expect(getByText('third button')).toEqual(document.activeElement) - }) - - it('Space: when user is typing and presses Space, it should not select the option', () => { - const mockFunction = jest.fn() - const {getByTestId, getByText} = render() - - const container = getByTestId('container') - fireEvent.keyDown(container, {key: 't', code: 't'}) - - const thirdButton = getByText('third button') - expect(thirdButton).toEqual(document.activeElement) - fireEvent.keyDown(thirdButton, {key: ' ', code: 'Space'}) - expect(mockFunction).not.toHaveBeenCalled() - }) - - it('Space after timeout: when user is presses Space after waiting, it should select the option', () => { - jest.useFakeTimers() - const mockFunction = jest.fn() - const {getByTestId, getByText} = render() - - const container = getByTestId('container') - fireEvent.keyDown(container, {key: 't', code: 't'}) - - const thirdButton = getByText('third button') - expect(thirdButton).toEqual(document.activeElement) - - jest.advanceTimersByTime(TYPEAHEAD_TIMEOUT) - fireEvent.keyDown(thirdButton, {key: ' ', code: 'Space'}) - expect(mockFunction).toHaveBeenCalled() - }) - - it('Enter: when user is presses Enter, it should instantly select the option', () => { - jest.useFakeTimers() - const mockFunction = jest.fn() - const {getByTestId, getByText} = render() - - const container = getByTestId('container') - fireEvent.keyDown(container, {key: 't', code: 't'}) - - const thirdButton = getByText('third button') - expect(thirdButton).toEqual(document.activeElement) - - fireEvent.keyDown(thirdButton, {key: 'Enter', code: 'Enter'}) - expect(mockFunction).toHaveBeenCalled() - }) - - it('Long string: when user starts typing a longer string "button 4", focus should jump to closest match', async () => { - jest.useFakeTimers() - const mockFunction = jest.fn() - const {getByTestId, getByText} = render() - const container = getByTestId('container') - - fireEvent.keyDown(container, {key: 'b', code: 'b'}) - expect(getByText('button 1')).toEqual(document.activeElement) - - fireEvent.keyDown(container, {key: 'u', code: 'u'}) - expect(getByText('button 1')).toEqual(document.activeElement) - - fireEvent.keyDown(container, {key: 't', code: 't'}) - fireEvent.keyDown(container, {key: 't', code: 't'}) - fireEvent.keyDown(container, {key: 'o', code: 'o'}) - fireEvent.keyDown(container, {key: 'n', code: 'n'}) - - // pressing Space should be treated as part of query - // and shouldn't select the option - fireEvent.keyDown(container, {key: ' ', code: 'Space'}) - expect(mockFunction).not.toHaveBeenCalled() - expect(getByText('button 1')).toEqual(document.activeElement) - - fireEvent.keyDown(container, {key: '4', code: '4'}) - // the query is now "button 4" and should move focus - expect(getByText('button 4')).toEqual(document.activeElement) - }) - - it('Shortcuts: when a modifier is used, typeahead should not do anything', () => { - const {getByTestId, getByText} = render() - const container = getByTestId('container') - - fireEvent.keyDown(container, {metaKey: true, key: 'b', code: 'b'}) - expect(getByText('button 1')).not.toEqual(document.activeElement) - }) - - it('TextInput: when an textinput has focus, typeahead should not do anything', async () => { - const {getByTestId, getByPlaceholderText} = render() - const container = getByTestId('container') - - const input = getByPlaceholderText('Filter options') - expect(input).toEqual(document.activeElement) - - fireEvent.keyDown(container, {key: 'b', code: 'b'}) - expect(input).toEqual(document.activeElement) - }) - - it('Missing ref: when a ref is not attached, typeahead should break the component', async () => { - const {getByTestId, getByText} = render() - const container = getByTestId('container') - - fireEvent.keyDown(container, {key: 'b', code: 'b'}) - expect(getByText('button 1')).not.toEqual(document.activeElement) - }) -}) diff --git a/src/hooks/index.ts b/src/hooks/index.ts index 6f7570fa062..47ec8e00a6f 100644 --- a/src/hooks/index.ts +++ b/src/hooks/index.ts @@ -11,5 +11,4 @@ export type {UseOverlaySettings} from './useOverlay' export {useRenderForcingRef} from './useRenderForcingRef' export {useProvidedStateOrCreate} from './useProvidedStateOrCreate' export {useMenuInitialFocus} from './useMenuInitialFocus' -export {useTypeaheadFocus} from './useTypeaheadFocus' export {useMnemonics} from './useMnemonics' diff --git a/src/hooks/useTypeaheadFocus.ts b/src/hooks/useTypeaheadFocus.ts deleted file mode 100644 index 7cc20ab701f..00000000000 --- a/src/hooks/useTypeaheadFocus.ts +++ /dev/null @@ -1,94 +0,0 @@ -import React from 'react' -import {iterateFocusableElements} from '@primer/behaviors/utils' -import {useProvidedRefOrCreate} from './useProvidedRefOrCreate' - -export const TYPEAHEAD_TIMEOUT = 1000 - -export const useTypeaheadFocus = (open: boolean, providedRef?: React.RefObject) => { - const containerRef = useProvidedRefOrCreate(providedRef) - - React.useEffect(() => { - if (!open || !containerRef.current) return - const container = containerRef.current - - let query = '' - let timeout: number | undefined - - const handler = (event: KeyboardEvent) => { - // skip if a TextInput has focus - const activeElement = document.activeElement as HTMLElement - if (activeElement.tagName === 'INPUT') return - - // skip if used with modifier to preserve shortcuts like ⌘ + F - const hasModifier = event.ctrlKey || event.altKey || event.metaKey - if (hasModifier) return - - if (query.length && event.code === 'Space') { - // prevent the menu from selecting an option - event.preventDefault() - } - - // skip if it's not a alphabet key - else if (!isAlphabetKey(event)) { - query = '' // reset the typeahead query - return - } - - query += event.key.toLowerCase() - - // if this is a typeahead event, don't propagate outside of menu - event.stopPropagation() - - // reset the query after timeout - window.clearTimeout(timeout) - timeout = window.setTimeout(() => (query = ''), TYPEAHEAD_TIMEOUT) - - let elementToFocus: HTMLElement | undefined - - const focusableItems = [...iterateFocusableElements(container)] - - const focusNextMatch = () => { - const itemsStartingWithKey = focusableItems.filter(item => { - return item.textContent?.toLowerCase().trim().startsWith(query) - }) - - const currentActiveIndex = itemsStartingWithKey.indexOf(activeElement) - - // If the last element is already selected, cycle through the list - if (currentActiveIndex === itemsStartingWithKey.length - 1) { - elementToFocus = itemsStartingWithKey[0] - } else { - elementToFocus = itemsStartingWithKey.find((item, index) => { - return index > currentActiveIndex - }) - } - elementToFocus?.focus() - } - - // Single character in query: Jump to the next match - if (query.length === 1) return focusNextMatch() - - // 2 characters in query but the user is pressing - // the same key, jump to the next match - if (query.length === 2 && query[0] === query[1]) { - query = query[0] // remove the second key - return focusNextMatch() - } - - // More > 1 characters in query - // If active element satisfies the query stay there, - if (activeElement.textContent?.toLowerCase().startsWith(query)) return - // otherwise move to the next one that does. - return focusNextMatch() - } - - container.addEventListener('keydown', handler) - return () => container.removeEventListener('keydown', handler) - }, [open, containerRef]) - - const isAlphabetKey = (event: KeyboardEvent) => { - return event.key.length === 1 && /[a-z\d]/i.test(event.key) - } - - return {containerRef} -}