[PopoverOverlay] Escape only closes popover if event.target is a descendant

.changeset/violet-rice-appear.md

@@ -0,0 +1,5 @@
+---
+'@shopify/polaris': patch
+---
+
+`PopoverOverlay` closes focused `Popover` when pressing `Escape`

polaris-react/src/components/Combobox/tests/Combobox.test.tsx

@@ -302,8 +302,14 @@ describe('<Combobox />', () => {
 
       triggerFocus(combobox);
 
+      const target = combobox.find(TextField)!.find('input')!.domNode;
+      const keyupEvent = new KeyboardEvent('keyup', {
+        keyCode: Key.Escape,
+        bubbles: true,
+      });
+
       combobox.act(() => {
-        dispatchKeyup(Key.Escape);
+        target?.dispatchEvent(keyupEvent);
       });
 
       expect(combobox).toContainReactComponent(Popover, {
@@ -433,8 +439,3 @@ function triggerOptionSelected(combobox: any) {
 }
 
 function noop() {}
-
-function dispatchKeyup(key: Key) {
-  const event: KeyboardEventInit & {keyCode: Key} = {keyCode: key};
-  document.dispatchEvent(new KeyboardEvent('keyup', event));
-}

polaris-react/src/components/Popover/components/PopoverOverlay/PopoverOverlay.tsx

@@ -299,8 +299,19 @@ export class PopoverOverlay extends PureComponent<PopoverOverlayProps, State> {
     this.props.onClose(PopoverCloseSource.ScrollOut);
   };
 
-  private handleEscape = () => {
-    this.props.onClose(PopoverCloseSource.EscapeKeypress);
+  private handleEscape = (event: Event) => {
+    const target = event.target as HTMLElement;
+    const {
+      contentNode,
+      props: {activator},
+    } = this;
+    const composedPath = event.composedPath();
+    const wasDescendant = wasContentNodeDescendant(composedPath, contentNode);
+    const isActivatorDescendant = nodeContainsDescendant(activator, target);
+
+    if (wasDescendant || isActivatorDescendant) {
+      this.props.onClose(PopoverCloseSource.EscapeKeypress);
+    }
   };
 
   private handleFocusFirstItem = () => {

polaris-react/src/components/Popover/components/PopoverOverlay/tests/PopoverOverlay.test.tsx

@@ -8,29 +8,10 @@ import {PositionedOverlay} from '../../../../PositionedOverlay';
 import {PopoverOverlay} from '../PopoverOverlay';
 import {Popover} from '../../../Popover';
 
-interface HandlerMap {
-  [eventName: string]: (event: any) => void;
-}
-
-const listenerMap: HandlerMap = {};
-
 describe('<PopoverOverlay />', () => {
-  let addEventListener: jest.SpyInstance;
-  let removeEventListener: jest.SpyInstance;
-
   let rafSpy: jest.SpyInstance;
 
   beforeEach(() => {
-    addEventListener = jest.spyOn(document, 'addEventListener');
-    addEventListener.mockImplementation((event, callback) => {
-      listenerMap[event] = callback;
-    });
-
-    removeEventListener = jest.spyOn(document, 'removeEventListener');
-    removeEventListener.mockImplementation((event) => {
-      listenerMap[event] = noop;
-    });
-
     rafSpy = jest.spyOn(window, 'requestAnimationFrame');
     rafSpy.mockImplementation((callback) => callback());
   });
@@ -40,9 +21,6 @@ describe('<PopoverOverlay />', () => {
       (document.activeElement as HTMLElement).blur();
     }
 
-    addEventListener.mockRestore();
-    removeEventListener.mockRestore();
-
     rafSpy.mockRestore();
   });
 
@@ -204,24 +182,6 @@ describe('<PopoverOverlay />', () => {
     });
   });
 
-  it('calls the onClose callback when the escape key is pressed', () => {
-    const spy = jest.fn();
-
-    mountWithApp(
-      <PopoverOverlay
-        active
-        id="PopoverOverlay-1"
-        activator={activator}
-        onClose={spy}
-      >
-        {children}
-      </PopoverOverlay>,
-    );
-
-    listenerMap.keyup({keyCode: Key.Escape});
-    expect(spy).toHaveBeenCalledTimes(1);
-  });
-
   it('does not call the onClose callback when a descendent HTMLElement is clicked', () => {
     const spy = jest.fn();
 
@@ -316,6 +276,77 @@ describe('<PopoverOverlay />', () => {
     expect(spy).not.toHaveBeenCalled();
   });
 
+  describe('when the Escape key is pressed', () => {
+    it('calls the onClose callback if event target is descendant', () => {
+      const spy = jest.fn();
+
+      const popoverOverlay = mountWithApp(
+        <PopoverOverlay
+          active
+          id="PopoverOverlay-1"
+          activator={activator}
+          onClose={spy}
+        >
+          <TextField
+            label="Store name"
+            value="Click me"
+            onChange={() => {}}
+            autoComplete="off"
+          />
+        </PopoverOverlay>,
+      );
+
+      const target = popoverOverlay.find(TextField)!.find('input')!.domNode!;
+      triggerEscape(target);
+
+      expect(spy).toHaveBeenCalledTimes(1);
+    });
+
+    it('calls the onClose callback if event target is activator descendant', () => {
+      const spy = jest.fn();
+
+      const popoverOverlay = mountWithApp(
+        <PopoverOverlay
+          active
+          id="PopoverOverlay-1"
+          activator={activator}
+          onClose={spy}
+        >
+          <TextField
+            label="Store name"
+            value="Click me"
+            onChange={() => {}}
+            autoComplete="off"
+          />
+        </PopoverOverlay>,
+      );
+
+      const target = popoverOverlay.find(TextField)!.find('input')!.domNode!;
+      triggerEscape(target);
+
+      expect(spy).toHaveBeenCalledTimes(1);
+    });
+
+    it('does not call the onClose callback if event target is not descendant or activator descendant', () => {
+      const spy = jest.fn();
+
+      mountWithApp(
+        <PopoverOverlay
+          active
+          id="PopoverOverlay-1"
+          activator={activator}
+          onClose={spy}
+        >
+          {children}
+        </PopoverOverlay>,
+      );
+
+      triggerEscape();
+
+      expect(spy).toHaveBeenCalledTimes(0);
+    });
+  });
+
   describe('deleting descendant elements', () => {
     const DeleteButton = () => {
       const [show, setShow] = React.useState(true);
@@ -567,3 +598,12 @@ describe('<PopoverOverlay />', () => {
 });
 
 function noop() {}
+
+function triggerEscape(target?: Element) {
+  const keyupEvent = new KeyboardEvent('keyup', {
+    keyCode: Key.Escape,
+    bubbles: true,
+  });
+
+  (target || document).dispatchEvent(keyupEvent);
+}