From a8419d8a5ba0186ca49f7021081db9578a5cf7f7 Mon Sep 17 00:00:00 2001
From: Robin Malfait <malfait.robin@gmail.com>
Date: Tue, 24 Aug 2021 11:26:19 +0200
Subject: [PATCH] Fix broken `escape` key behaviour (#754)

* fix broken `escape` key behaviour

We've "fixed" an issue when we had nested Dialogs ([#430](https://github.com/tailwindlabs/headlessui/pull/430)).
The `escape` would not close the correct Dialog. The issue here was with
the logic to know whether we were the last Dialog or not. The issue was
_not_ how we implemented the `close` functionality.

To make things easier, we moved the global window event to a scoped div
(the Dialog itself). While that fixed the nested Dialog issue, it
introduced this bug where `escape` would not close if you click on a
non-focusable element like a span in the Dialog.

Since that PR we did a bunch of improvements on how the underlying
"stacking" system worked.
This PR reverts to the "global" window event listener so that we can
still catch all of the `escape` keydown events.

Fixes: #524
Fixes: #693

* update changelog
---
 src/components/dialog/dialog.test.tsx | 84 +++++++++++++++++++++++++++
 src/components/dialog/dialog.tsx      | 23 ++++----
 2 files changed, 95 insertions(+), 12 deletions(-)

diff --git a/src/components/dialog/dialog.test.tsx b/src/components/dialog/dialog.test.tsx
index 6fa6127..fcdc88f 100644
--- a/src/components/dialog/dialog.test.tsx
+++ b/src/components/dialog/dialog.test.tsx
@@ -430,6 +430,90 @@ describe('Keyboard interactions', () => {
         assertDialog({ state: DialogState.InvisibleUnmounted })
       })
     )
+
+    it(
+      'should be possible to close the dialog with Escape, when a field is focused',
+      suppressConsoleLogs(async () => {
+        function Example() {
+          let [isOpen, setIsOpen] = useState(false)
+          return (
+            <>
+              <button id="trigger" onClick={() => setIsOpen(v => !v)}>
+                Trigger
+              </button>
+              <Dialog open={isOpen} onClose={setIsOpen}>
+                Contents
+                <input id="name" />
+                <TabSentinel />
+              </Dialog>
+            </>
+          )
+        }
+        render(<Example />)
+
+        assertDialog({ state: DialogState.InvisibleUnmounted })
+
+        // Open dialog
+        await click(document.getElementById('trigger'))
+
+        // Verify it is open
+        assertDialog({
+          state: DialogState.Visible,
+          attributes: { id: 'headlessui-dialog-1' },
+        })
+
+        // Close dialog
+        await press(Keys.Escape)
+
+        // Verify it is close
+        assertDialog({ state: DialogState.InvisibleUnmounted })
+      })
+    )
+
+    it(
+      'should not be possible to close the dialog with Escape, when a field is focused but cancels the event',
+      suppressConsoleLogs(async () => {
+        function Example() {
+          let [isOpen, setIsOpen] = useState(false)
+          return (
+            <>
+              <button id="trigger" onClick={() => setIsOpen(v => !v)}>
+                Trigger
+              </button>
+              <Dialog open={isOpen} onClose={setIsOpen}>
+                Contents
+                <input
+                  id="name"
+                  onKeyDown={event => {
+                    event.preventDefault()
+                    event.stopPropagation()
+                  }}
+                />
+                <TabSentinel />
+              </Dialog>
+            </>
+          )
+        }
+        render(<Example />)
+
+        assertDialog({ state: DialogState.InvisibleUnmounted })
+
+        // Open dialog
+        await click(document.getElementById('trigger'))
+
+        // Verify it is open
+        assertDialog({
+          state: DialogState.Visible,
+          attributes: { id: 'headlessui-dialog-1' },
+        })
+
+        // Try to close the dialog
+        await press(Keys.Escape)
+
+        // Verify it is still open
+        assertDialog({ state: DialogState.Visible })
+      })
+    )
   })
 })
 
diff --git a/src/components/dialog/dialog.tsx b/src/components/dialog/dialog.tsx
index 2055bd4..839ca69 100644
--- a/src/components/dialog/dialog.tsx
+++ b/src/components/dialog/dialog.tsx
@@ -7,15 +7,14 @@ import React, {
   useMemo,
   useReducer,
   useRef,
+  useState,
 
   // Types
   ContextType,
   ElementType,
   MouseEvent as ReactMouseEvent,
-  KeyboardEvent as ReactKeyboardEvent,
   MutableRefObject,
   Ref,
-  useState,
 } from 'react'
 
 import { Props } from '../../types'
@@ -217,6 +216,16 @@ let DialogRoot = forwardRefWithAs(function Dialog<
     close()
   })
 
+  // Handle `Escape` to close
+  useWindowEvent('keydown', event => {
+    if (event.key !== Keys.Escape) return
+    if (dialogState !== DialogStates.Open) return
+    if (hasNestedDialogs) return
+    event.preventDefault()
+    event.stopPropagation()
+    close()
+  })
+
   // Scroll lock
   useEffect(() => {
     if (dialogState !== DialogStates.Open) return
@@ -282,16 +291,6 @@ let DialogRoot = forwardRefWithAs(function Dialog<
     onClick(event: ReactMouseEvent) {
       event.stopPropagation()
     },
-
-    // Handle `Escape` to close
-    onKeyDown(event: ReactKeyboardEvent) {
-      if (event.key !== Keys.Escape) return
-      if (dialogState !== DialogStates.Open) return
-      if (hasNestedDialogs) return
-      event.preventDefault()
-      event.stopPropagation()
-      close()
-    },
   }
   let passthroughProps = rest