Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
5 changes: 5 additions & 0 deletions .changeset/menus-typeahead.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react': minor
---

ActionMenu2 + DropdownMenu2: Implement typeahead search. [See detailed spec.](https://github.com/github/primer/issues/518#issuecomment-999104848)
6 changes: 4 additions & 2 deletions src/ActionMenu2.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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} from './hooks'
import {useProvidedRefOrCreate, useProvidedStateOrCreate, useMenuInitialFocus, useTypeaheadFocus} from './hooks'
import {Divider} from './ActionList2/Divider'
import {ActionListContainerContext} from './ActionList2/ActionListContainerContext'
import {Button, ButtonProps} from './Button2'
Expand Down Expand Up @@ -95,7 +95,9 @@ const Overlay: React.FC<MenuOverlayProps> = ({children, ...overlayProps}) => {
'anchorRef'
>

const {containerRef, openWithFocus} = useMenuInitialFocus(open, onOpen)
const containerRef = React.createRef<HTMLDivElement>()
const {openWithFocus} = useMenuInitialFocus(open, onOpen, containerRef)
useTypeaheadFocus(open, containerRef)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note 1: Changed both of these hooks to accept a ref if it's passed (or create a new one if it's not passed)

Note 2: Did not combine these into one hook called useMenuFocus, we might want to re-use typeahead logic in NavigationTree 🤷


return (
<AnchoredOverlay
Expand Down
6 changes: 4 additions & 2 deletions src/DropdownMenu2.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {TriangleDownIcon} from '@primer/octicons-react'
import {Button, ButtonProps} from './Button2'
import {AnchoredOverlay, AnchoredOverlayProps} from './AnchoredOverlay'
import {OverlayProps} from './Overlay'
import {useProvidedRefOrCreate, useProvidedStateOrCreate, useMenuInitialFocus} from './hooks'
import {useProvidedRefOrCreate, useProvidedStateOrCreate, useMenuInitialFocus, useTypeaheadFocus} from './hooks'
import {Divider} from './ActionList2/Divider'
import {ActionListContainerContext} from './ActionList2/ActionListContainerContext'
import {MandateProps} from './utils/types'
Expand Down Expand Up @@ -99,7 +99,9 @@ const Overlay: React.FC<MenuOverlayProps> = ({children, ...overlayProps}) => {
'anchorRef'
>

const {containerRef, openWithFocus} = useMenuInitialFocus(open, onOpen)
const containerRef = React.createRef<HTMLDivElement>()
const {openWithFocus} = useMenuInitialFocus(open, onOpen, containerRef)
useTypeaheadFocus(open, containerRef)

return (
<AnchoredOverlay
Expand Down
6 changes: 4 additions & 2 deletions src/__tests__/hooks/useMenuInitialFocus.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ import {useMenuInitialFocus} from '../../hooks'
const Component = () => {
const [open, setOpen] = React.useState(false)
const onOpen = () => setOpen(!open)
const {containerRef, openWithFocus} = useMenuInitialFocus(open, onOpen)

const containerRef = React.createRef<HTMLDivElement>()
const {openWithFocus} = useMenuInitialFocus(open, onOpen, containerRef)

return (
<>
Expand All @@ -25,7 +27,7 @@ const Component = () => {
)
}

describe('useMenuFocus', () => {
describe('useMenuInitialFocus', () => {
afterEach(cleanup)

it('should focus first element when opened with Enter', async () => {
Expand Down
196 changes: 196 additions & 0 deletions src/__tests__/hooks/useMenuTypeaheadFocus.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,196 @@
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<HTMLButtonElement>) => void
hasInput?: boolean
refNotAttached?: boolean
}) => {
const containerRef = React.createRef<HTMLDivElement>()
useTypeaheadFocus(true, containerRef) // hard coding open=true for test

return (
<>
<div ref={refNotAttached ? undefined : containerRef} data-testid="container">
{/* eslint-disable-next-line jsx-a11y/no-autofocus */}
{hasInput && <input autoFocus type="text" placeholder="Filter options" />}
<button onKeyDown={onSelect}>button 1</button>
<button onKeyDown={onSelect}>Button 2</button>
<button disabled>button 3 is disabled</button>
<button onKeyDown={onSelect}>button 4</button>
<button onKeyDown={onSelect}>third button</button>
<span>not focusable</span>
</div>
</>
)
}

describe('useTypeaheadFocus', () => {
afterEach(cleanup)

it('First element: when b is pressed, it should move focus the "b"utton 1', () => {
const {getByTestId, getByText} = render(<Component />)
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(<Component />)
const container = getByTestId('container')

fireEvent.keyDown(container, {key: 't', code: 't'})
expect(getByText('third button')).toEqual(document.activeElement)
})

it('Case insinsitive: when B is pressed, it should move focus the "b"utton 1', () => {
const {getByTestId, getByText} = render(<Component />)
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(<Component />)
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(<Component />)
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(<Component onSelect={mockFunction} />)

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(<Component onSelect={mockFunction} />)

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(<Component onSelect={mockFunction} />)

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(<Component onSelect={mockFunction} />)
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(<Component />)
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(<Component hasInput={true} />)
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(<Component refNotAttached={true} />)
const container = getByTestId('container')

fireEvent.keyDown(container, {key: 'b', code: 'b'})
expect(getByText('button 1')).not.toEqual(document.activeElement)
})
})
1 change: 1 addition & 0 deletions src/hooks/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ export type {UseOverlaySettings} from './useOverlay'
export {useRenderForcingRef} from './useRenderForcingRef'
export {useProvidedStateOrCreate} from './useProvidedStateOrCreate'
export {useMenuInitialFocus} from './useMenuInitialFocus'
export {useTypeaheadFocus} from './useTypeaheadFocus'
6 changes: 3 additions & 3 deletions src/hooks/useMenuInitialFocus.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import React from 'react'
import {iterateFocusableElements} from '@primer/behaviors/utils'
import {useProvidedRefOrCreate} from './useProvidedRefOrCreate'

type Gesture = 'anchor-click' | 'anchor-key-press'
type Callback = (gesture: Gesture, event?: React.KeyboardEvent<HTMLElement>) => unknown

export const useMenuInitialFocus = (open: boolean, onOpen?: Callback) => {
const containerRef = React.createRef<HTMLDivElement>()
export const useMenuInitialFocus = (open: boolean, onOpen?: Callback, providedRef?: React.RefObject<HTMLElement>) => {
const containerRef = useProvidedRefOrCreate(providedRef)
const [openingKey, setOpeningKey] = React.useState<string | undefined>(undefined)

const openWithFocus: Callback = (gesture, event) => {
Expand All @@ -19,7 +20,6 @@ export const useMenuInitialFocus = (open: boolean, onOpen?: Callback) => {
* ArrowDown | Space | Enter: first element
* ArrowUp: last element
*/

React.useEffect(() => {
if (!open) return
if (!openingKey || !containerRef.current) return
Expand Down
94 changes: 94 additions & 0 deletions src/hooks/useTypeaheadFocus.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
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<HTMLElement>) => {
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().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 charachter in query: Jump to the next match
if (query.length === 1) return focusNextMatch()

// 2 charachters 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 charachters 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}
}
Loading