From 0cf1d1a85ec45d3230a71e6b75e72d27d946fb3e Mon Sep 17 00:00:00 2001
From: Adi Dahiya <adahiya@palantir.com>
Date: Wed, 6 Dec 2023 18:16:43 -0500
Subject: [PATCH 1/3] [core] fix(Popover): return focus to target on ESC
 keypress

---
 .../core/src/components/popover/popover.tsx   | 43 ++++++++++---------
 1 file changed, 23 insertions(+), 20 deletions(-)

diff --git a/packages/core/src/components/popover/popover.tsx b/packages/core/src/components/popover/popover.tsx
index 632c350bbd..c9545a2b82 100644
--- a/packages/core/src/components/popover/popover.tsx
+++ b/packages/core/src/components/popover/popover.tsx
@@ -111,7 +111,9 @@ export interface PopoverProps<TProps extends DefaultPopoverTargetHTMLProps = Def
      * Whether the application should return focus to the last active element in the
      * document after this popover closes.
      *
-     * This is automatically set (overridden) to `false` for hover interaction popovers.
+     * This is automatically set (overridden) to:
+     *  - `false` for hover interaction popovers
+     *  - `true` when a popover closes due to an ESC keypress
      *
      * If you are attaching a popover _and_ a tooltip to the same target, you must take
      * care to either disable this prop for the popover _or_ disable the tooltip's
@@ -131,8 +133,10 @@ export interface PopoverProps<TProps extends DefaultPopoverTargetHTMLProps = Def
 }
 
 export interface PopoverState {
-    isOpen: boolean;
     hasDarkParent: boolean;
+    // when an ESC keypress interaction closes the overlay, we want to force-enable `shouldReturnFocusOnClose` behavior
+    isClosingViaEscapeKeypress: boolean;
+    isOpen: boolean;
 }
 
 /**
@@ -174,6 +178,7 @@ export class Popover<
 
     public state: PopoverState = {
         hasDarkParent: false,
+        isClosingViaEscapeKeypress: false,
         isOpen: this.getIsOpen(this.props),
     };
 
@@ -439,17 +444,9 @@ export class Popover<
     };
 
     private renderPopover = (popperProps: PopperChildrenProps) => {
-        const {
-            autoFocus,
-            enforceFocus,
-            backdropProps,
-            canEscapeKeyClose,
-            hasBackdrop,
-            interactionKind,
-            shouldReturnFocusOnClose,
-            usePortal,
-        } = this.props;
-        const { isOpen } = this.state;
+        const { autoFocus, enforceFocus, backdropProps, canEscapeKeyClose, hasBackdrop, interactionKind, usePortal } =
+            this.props;
+        const { isClosingViaEscapeKeypress: isClosingViaKeyboardInteraction, isOpen } = this.state;
 
         // compute an appropriate transform origin so the scale animation points towards target
         const transformOrigin = getTransformOrigin(
@@ -490,6 +487,12 @@ export class Popover<
         );
 
         const defaultAutoFocus = this.isHoverInteractionKind() ? false : undefined;
+        // if hover interaction, it doesn't make sense to take over focus control
+        const shouldReturnFocusOnClose = this.isHoverInteractionKind()
+            ? false
+            : isClosingViaKeyboardInteraction
+              ? true
+              : this.props.shouldReturnFocusOnClose;
 
         return (
             <Overlay
@@ -513,8 +516,7 @@ export class Popover<
                 portalClassName={this.props.portalClassName}
                 portalContainer={this.props.portalContainer}
                 portalStopPropagationEvents={this.props.portalStopPropagationEvents}
-                // if hover interaction, it doesn't make sense to take over focus control
-                shouldReturnFocusOnClose={this.isHoverInteractionKind() ? false : shouldReturnFocusOnClose}
+                shouldReturnFocusOnClose={shouldReturnFocusOnClose}
             >
                 <div className={Classes.POPOVER_TRANSITION_CONTAINER} ref={popperProps.ref} style={popperProps.style}>
                     <ResizeSensor onResize={this.reposition}>
@@ -717,11 +719,7 @@ export class Popover<
         if (!shouldIgnoreClick) {
             // ensure click did not originate from within inline popover before closing
             if (!this.props.disabled && !this.isElementInPopover(e.target as HTMLElement)) {
-                if (this.props.isOpen == null) {
-                    this.setState(prevState => ({ isOpen: !prevState.isOpen }));
-                } else {
-                    this.setOpenState(!this.props.isOpen, e);
-                }
+                this.setOpenState(!this.props.isOpen, e);
             }
         }
     };
@@ -747,6 +745,7 @@ export class Popover<
                 // non-null assertion because the only time `e` is undefined is when in controlled mode
                 // or the rare special case in uncontrolled mode when the `disabled` flag is toggled true
                 this.props.onClose?.(e!);
+                this.setState({ isClosingViaEscapeKeypress: isEscapeKeypressEvent(e?.nativeEvent) });
             }
         }
     }
@@ -763,6 +762,10 @@ export class Popover<
     }
 }
 
+function isEscapeKeypressEvent(e?: Event) {
+    return e instanceof KeyboardEvent && e.key === "Escape";
+}
+
 function noop() {
     // no-op
 }

From 86f3149f0d51ecc04ebab63adc071c056bcdc872 Mon Sep 17 00:00:00 2001
From: Adi Dahiya <adahiya@palantir.com>
Date: Thu, 7 Dec 2023 10:09:33 -0500
Subject: [PATCH 2/3] revert handleTargetClick change

---
 packages/core/src/components/popover/popover.tsx | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/packages/core/src/components/popover/popover.tsx b/packages/core/src/components/popover/popover.tsx
index c9545a2b82..51472fccf2 100644
--- a/packages/core/src/components/popover/popover.tsx
+++ b/packages/core/src/components/popover/popover.tsx
@@ -719,7 +719,11 @@ export class Popover<
         if (!shouldIgnoreClick) {
             // ensure click did not originate from within inline popover before closing
             if (!this.props.disabled && !this.isElementInPopover(e.target as HTMLElement)) {
-                this.setOpenState(!this.props.isOpen, e);
+                if (this.props.isOpen == null) {
+                    this.setState(prevState => ({ isOpen: !prevState.isOpen }));
+                } else {
+                    this.setOpenState(!this.props.isOpen, e);
+                }
             }
         }
     };

From ba2f2c9cde1c71edde6c3ce9a682e11dd2a49eea Mon Sep 17 00:00:00 2001
From: Adi Dahiya <adahiya@palantir.com>
Date: Thu, 7 Dec 2023 10:11:18 -0500
Subject: [PATCH 3/3] revert rename typo

---
 packages/core/src/components/popover/popover.tsx | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/packages/core/src/components/popover/popover.tsx b/packages/core/src/components/popover/popover.tsx
index 51472fccf2..819ac97db1 100644
--- a/packages/core/src/components/popover/popover.tsx
+++ b/packages/core/src/components/popover/popover.tsx
@@ -446,7 +446,7 @@ export class Popover<
     private renderPopover = (popperProps: PopperChildrenProps) => {
         const { autoFocus, enforceFocus, backdropProps, canEscapeKeyClose, hasBackdrop, interactionKind, usePortal } =
             this.props;
-        const { isClosingViaEscapeKeypress: isClosingViaKeyboardInteraction, isOpen } = this.state;
+        const { isClosingViaEscapeKeypress, isOpen } = this.state;
 
         // compute an appropriate transform origin so the scale animation points towards target
         const transformOrigin = getTransformOrigin(
@@ -490,7 +490,7 @@ export class Popover<
         // if hover interaction, it doesn't make sense to take over focus control
         const shouldReturnFocusOnClose = this.isHoverInteractionKind()
             ? false
-            : isClosingViaKeyboardInteraction
+            : isClosingViaEscapeKeypress
               ? true
               : this.props.shouldReturnFocusOnClose;