Skip to content

Commit

Permalink
fix(useCombobox): initial focus behaviour (#1526)
Browse files Browse the repository at this point in the history
* fix(useCombobox): initial focus behaviour

* use getInitialValue

* remove unneeded tests
  • Loading branch information
silviuaavram authored Aug 2, 2023
1 parent b498830 commit cae0884
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 26 deletions.
46 changes: 23 additions & 23 deletions src/hooks/useCombobox/__tests__/getInputProps.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
mouseMoveItemAtIndex,
tab,
clickOnInput,
initialFocusAndOpenTestCases,
} from '../testUtils'
import utils from '../../utils'
import useCombobox from '..'
Expand Down Expand Up @@ -413,29 +414,28 @@ describe('getInputProps', () => {
})

describe('initial focus', () => {
test('is grabbed when isOpen is passed as true', () => {
renderCombobox({isOpen: true})

expect(getInput()).toHaveFocus()
})

test('is grabbed when initialIsOpen is passed as true', () => {
renderCombobox({initialIsOpen: true})

expect(getInput()).toHaveFocus()
})

test('is grabbed when defaultIsOpen is passed as true', () => {
renderCombobox({defaultIsOpen: true})

expect(getInput()).toHaveFocus()
})

test('is not grabbed when initial open is set to default (false)', () => {
renderCombobox()

expect(getInput()).not.toHaveFocus()
})
for (const [
initialIsOpen,
defaultIsOpen,
isOpen,
status,
] of initialFocusAndOpenTestCases) {
/* eslint-disable */
test(`is ${
status ? '' : 'not '
}grabbed when initialIsOpen: ${initialIsOpen}, defaultIsOpen: ${defaultIsOpen} and props.isOpen: ${isOpen}`, () => {
renderCombobox({isOpen, defaultIsOpen, initialIsOpen})

if (status) {
expect(getInput()).toHaveFocus()
expect(getItems()).toHaveLength(items.length)
} else {
expect(getInput()).not.toHaveFocus()
expect(getItems()).toHaveLength(0)
}
})
/* eslint-enable */
}
})

describe('event handlers', () => {
Expand Down
5 changes: 2 additions & 3 deletions src/hooks/useCombobox/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
useControlPropsValidator,
useElementIds,
getItemAndIndex,
getInitialValue,
} from '../utils'
import {
getInitialState,
Expand All @@ -31,8 +32,6 @@ function useCombobox(userProps = {}) {
...userProps,
}
const {
initialIsOpen,
defaultIsOpen,
items,
scrollIntoView,
environment,
Expand Down Expand Up @@ -106,7 +105,7 @@ function useCombobox(userProps = {}) {
})
// Focus the input on first render if required.
useEffect(() => {
const focusOnOpen = initialIsOpen || defaultIsOpen || isOpen
const focusOnOpen = getInitialValue(props, 'isOpen')

if (focusOnOpen && inputRef.current) {
inputRef.current.focus()
Expand Down
23 changes: 23 additions & 0 deletions src/hooks/useCombobox/testUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,3 +117,26 @@ function DropdownCombobox({renderSpy, renderItem, ...props}) {
export const renderUseCombobox = props => {
return renderHook(() => useCombobox({items, ...props}))
}

// format is: [initialIsOpen, defaultIsOpen, props.isOpen, menu is open && input is focused]
export const initialFocusAndOpenTestCases = [
[undefined, undefined, undefined, false],
[undefined, undefined, true, true],
[true, true, true, true],
[true, false, true, true],
[false, true, true, true],
[false, false, true, true],
[undefined, undefined, false, false],
[true, true, false, false],
[true, false, false, false],
[false, true, false, false],
[false, false, false, false],
[false, undefined, undefined, false],
[false, false, undefined, false],
[false, true, undefined, false],
[true, undefined, undefined, true],
[true, false, undefined, true],
[true, true, undefined, true],
[undefined, false, undefined, false],
[undefined, true, undefined, true],
]

0 comments on commit cae0884

Please sign in to comment.