Skip to content

Commit

Permalink
Ensure Escape propagates correctly in Combobox component (#1511)
Browse files Browse the repository at this point in the history
* ensure `Escape` propagates correctly in Combobox component

* update changelog
  • Loading branch information
RobinMalfait committed May 27, 2022
1 parent 08b419e commit eefc03c
Show file tree
Hide file tree
Showing 5 changed files with 142 additions and 0 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
Expand Up @@ -12,13 +12,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Add `by` prop for `Listbox`, `Combobox` and `RadioGroup` ([#1482](https://github.com/tailwindlabs/headlessui/pull/1482))
- Add `@headlessui/tailwindcss` plugin ([#1487](https://github.com/tailwindlabs/headlessui/pull/1487))

### Fixed

- Ensure `Escape` propagates correctly in `Combobox` component ([#1511](https://github.com/tailwindlabs/headlessui/pull/1511))

## [Unreleased - @headlessui/vue]

### Added

- Add `by` prop for `Listbox`, `Combobox` and `RadioGroup` ([#1482](https://github.com/tailwindlabs/headlessui/pull/1482))
- Add `@headlessui/tailwindcss` plugin ([#1487](https://github.com/tailwindlabs/headlessui/pull/1487))

### Fixed

- Ensure `Escape` propagates correctly in `Combobox` component ([#1511](https://github.com/tailwindlabs/headlessui/pull/1511))

## [Unreleased - @headlessui/tailwindcss]

- Nothing yet!
Expand Down
Expand Up @@ -1499,6 +1499,64 @@ describe('Keyboard interactions', () => {
assertActiveElement(getComboboxInput())
})
)

it(
'should not propagate the Escape event when the combobox is open',
suppressConsoleLogs(async () => {
let handleKeyDown = jest.fn()
render(
<div onKeyDown={handleKeyDown}>
<Combobox value="test" onChange={console.log}>
<Combobox.Input onChange={NOOP} />
<Combobox.Button>Trigger</Combobox.Button>
<Combobox.Options>
<Combobox.Option value="a">Option A</Combobox.Option>
<Combobox.Option value="b">Option B</Combobox.Option>
<Combobox.Option value="c">Option C</Combobox.Option>
</Combobox.Options>
</Combobox>
</div>
)

// Open combobox
await click(getComboboxButton())

// Close combobox
await press(Keys.Escape)

// We should never see the Escape event
expect(handleKeyDown).toHaveBeenCalledTimes(0)
})
)

it(
'should propagate the Escape event when the combobox is closed',
suppressConsoleLogs(async () => {
let handleKeyDown = jest.fn()
render(
<div onKeyDown={handleKeyDown}>
<Combobox value="test" onChange={console.log}>
<Combobox.Input onChange={NOOP} />
<Combobox.Button>Trigger</Combobox.Button>
<Combobox.Options>
<Combobox.Option value="a">Option A</Combobox.Option>
<Combobox.Option value="b">Option B</Combobox.Option>
<Combobox.Option value="c">Option C</Combobox.Option>
</Combobox.Options>
</Combobox>
</div>
)

// Focus the input field
await focus(getComboboxInput())

// Close combobox
await press(Keys.Escape)

// We should never see the Escape event
expect(handleKeyDown).toHaveBeenCalledTimes(1)
})
)
})

describe('`ArrowDown` key', () => {
Expand Down
Expand Up @@ -621,6 +621,7 @@ let Input = forwardRefWithAs(function Input<

case Keys.Backspace:
case Keys.Delete:
if (data.comboboxState !== ComboboxState.Open) return
if (data.mode !== ValueMode.Single) return
if (!data.nullable) return

Expand Down Expand Up @@ -707,13 +708,15 @@ let Input = forwardRefWithAs(function Input<
return actions.goToOption(Focus.Last)

case Keys.Escape:
if (data.comboboxState !== ComboboxState.Open) return
event.preventDefault()
if (data.optionsRef.current && !data.optionsPropsRef.current.static) {
event.stopPropagation()
}
return actions.closeCombobox()

case Keys.Tab:
if (data.comboboxState !== ComboboxState.Open) return
actions.selectActiveOption()
actions.closeCombobox()
break
Expand Down Expand Up @@ -830,6 +833,7 @@ let Button = forwardRefWithAs(function Button<TTag extends ElementType = typeof
return d.nextFrame(() => data.inputRef.current?.focus({ preventScroll: true }))

case Keys.Escape:
if (data.comboboxState !== ComboboxState.Open) return
event.preventDefault()
if (data.optionsRef.current && !data.optionsPropsRef.current.static) {
event.stopPropagation()
Expand Down
68 changes: 68 additions & 0 deletions packages/@headlessui-vue/src/components/combobox/combobox.test.ts
Expand Up @@ -1644,6 +1644,74 @@ describe('Keyboard interactions', () => {
assertActiveElement(getComboboxInput())
})
)

it(
'should not propagate the Escape event when the combobox is open',
suppressConsoleLogs(async () => {
let handleKeyDown = jest.fn()
renderTemplate({
template: html`
<Combobox v-model="value">
<ComboboxInput />
<ComboboxButton>Trigger</ComboboxButton>
<ComboboxOptions>
<ComboboxOption value="a">Option A</ComboboxOption>
<ComboboxOption value="b">Option B</ComboboxOption>
<ComboboxOption value="c">Option C</ComboboxOption>
</ComboboxOptions>
</Combobox>
`,
setup: () => ({ value: ref(null) }),
})

window.addEventListener('keydown', handleKeyDown)

// Open combobox
await click(getComboboxButton())

// Close combobox
await press(Keys.Escape)

// We should never see the Escape event
expect(handleKeyDown).toHaveBeenCalledTimes(0)

window.removeEventListener('keydown', handleKeyDown)
})
)

it(
'should propagate the Escape event when the combobox is closed',
suppressConsoleLogs(async () => {
let handleKeyDown = jest.fn()
renderTemplate({
template: html`
<Combobox v-model="value">
<ComboboxInput />
<ComboboxButton>Trigger</ComboboxButton>
<ComboboxOptions>
<ComboboxOption value="a">Option A</ComboboxOption>
<ComboboxOption value="b">Option B</ComboboxOption>
<ComboboxOption value="c">Option C</ComboboxOption>
</ComboboxOptions>
</Combobox>
`,
setup: () => ({ value: ref(null) }),
})

window.addEventListener('keydown', handleKeyDown)

// Focus the input field
await focus(getComboboxInput())

// Close combobox
await press(Keys.Escape)

// We should never see the Escape event
expect(handleKeyDown).toHaveBeenCalledTimes(1)

window.removeEventListener('keydown', handleKeyDown)
})
)
})

describe('`ArrowDown` key', () => {
Expand Down
4 changes: 4 additions & 0 deletions packages/@headlessui-vue/src/components/combobox/combobox.ts
Expand Up @@ -576,6 +576,7 @@ export let ComboboxButton = defineComponent({
return

case Keys.Escape:
if (api.comboboxState.value !== ComboboxStates.Open) return
event.preventDefault()
if (api.optionsRef.value && !api.optionsPropsRef.value.static) {
event.stopPropagation()
Expand Down Expand Up @@ -649,6 +650,7 @@ export let ComboboxInput = defineComponent({

case Keys.Backspace:
case Keys.Delete:
if (api.comboboxState.value !== ComboboxStates.Open) return
if (api.mode.value !== ValueMode.Single) return
if (!api.nullable.value) return

Expand Down Expand Up @@ -725,6 +727,7 @@ export let ComboboxInput = defineComponent({
return api.goToOption(Focus.Last)

case Keys.Escape:
if (api.comboboxState.value !== ComboboxStates.Open) return
event.preventDefault()
if (api.optionsRef.value && !api.optionsPropsRef.value.static) {
event.stopPropagation()
Expand All @@ -733,6 +736,7 @@ export let ComboboxInput = defineComponent({
break

case Keys.Tab:
if (api.comboboxState.value !== ComboboxStates.Open) return
api.selectActiveOption()
api.closeCombobox()
break
Expand Down

0 comments on commit eefc03c

Please sign in to comment.