From eefc03ce16298c8f6cf38d10d3a90054c059a8ff Mon Sep 17 00:00:00 2001
From: Robin Malfait <malfait.robin@gmail.com>
Date: Fri, 27 May 2022 17:57:28 +0200
Subject: [PATCH] Ensure `Escape` propagates correctly in `Combobox` component
 (#1511)

* ensure `Escape` propagates correctly in Combobox component

* update changelog
---
 CHANGELOG.md                                  |  8 +++
 .../src/components/combobox/combobox.test.tsx | 58 ++++++++++++++++
 .../src/components/combobox/combobox.tsx      |  4 ++
 .../src/components/combobox/combobox.test.ts  | 68 +++++++++++++++++++
 .../src/components/combobox/combobox.ts       |  4 ++
 5 files changed, 142 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9794709f9..abe3b4a26 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -12,6 +12,10 @@ 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
@@ -19,6 +23,10 @@ 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/tailwindcss]
 
 - Nothing yet!
diff --git a/packages/@headlessui-react/src/components/combobox/combobox.test.tsx b/packages/@headlessui-react/src/components/combobox/combobox.test.tsx
index 8284245a3..fd33475de 100644
--- a/packages/@headlessui-react/src/components/combobox/combobox.test.tsx
+++ b/packages/@headlessui-react/src/components/combobox/combobox.test.tsx
@@ -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', () => {
diff --git a/packages/@headlessui-react/src/components/combobox/combobox.tsx b/packages/@headlessui-react/src/components/combobox/combobox.tsx
index df98d329b..efad64f75 100644
--- a/packages/@headlessui-react/src/components/combobox/combobox.tsx
+++ b/packages/@headlessui-react/src/components/combobox/combobox.tsx
@@ -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
 
@@ -707,6 +708,7 @@ 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()
@@ -714,6 +716,7 @@ let Input = forwardRefWithAs(function Input<
         return actions.closeCombobox()
 
       case Keys.Tab:
+        if (data.comboboxState !== ComboboxState.Open) return
         actions.selectActiveOption()
         actions.closeCombobox()
         break
@@ -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()
diff --git a/packages/@headlessui-vue/src/components/combobox/combobox.test.ts b/packages/@headlessui-vue/src/components/combobox/combobox.test.ts
index c579fb01d..ca10160e4 100644
--- a/packages/@headlessui-vue/src/components/combobox/combobox.test.ts
+++ b/packages/@headlessui-vue/src/components/combobox/combobox.test.ts
@@ -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', () => {
diff --git a/packages/@headlessui-vue/src/components/combobox/combobox.ts b/packages/@headlessui-vue/src/components/combobox/combobox.ts
index 5e70ac2d5..29658ab1b 100644
--- a/packages/@headlessui-vue/src/components/combobox/combobox.ts
+++ b/packages/@headlessui-vue/src/components/combobox/combobox.ts
@@ -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()
@@ -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
 
@@ -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()
@@ -733,6 +736,7 @@ export let ComboboxInput = defineComponent({
           break
 
         case Keys.Tab:
+          if (api.comboboxState.value !== ComboboxStates.Open) return
           api.selectActiveOption()
           api.closeCombobox()
           break