Fix popover auto-focus and click outside handler

polaris-react/src/components/ColorPicker/components/Slidable/Slidable.tsx

@@ -66,7 +66,7 @@ export class Slidable extends PureComponent<SlidableProps, State> {
        * This is a workaround to enable event listeners to be
        * re-attached when moving from one document to another
        * when using a React portal across iframes.
-       * The resize observer works because when the clientWidth
+       * Using a resize observer works because when the clientWidth
        * will go from 0 to the real width after the node
        * gets rendered in its new place.
        */

polaris-react/src/components/KeypressListener/KeypressListener.tsx

@@ -7,6 +7,7 @@ export interface NonMutuallyExclusiveProps {
   keyCode: Key;
   handler(event: KeyboardEvent): void;
   keyEvent?: KeyEvent;
+  document?: Document;
 }
 
 export type KeypressListenerProps = NonMutuallyExclusiveProps &
@@ -23,6 +24,7 @@ export function KeypressListener({
   keyEvent = 'keyup',
   options,
   useCapture,
+  document: ownerDocument = document,
 }: KeypressListenerProps) {
   const tracked = useRef({handler, keyCode});
 
@@ -38,9 +40,13 @@ export function KeypressListener({
   }, []);
 
   useEffect(() => {
-    document.addEventListener(keyEvent, handleKeyEvent, useCapture || options);
+    ownerDocument.addEventListener(
+      keyEvent,
+      handleKeyEvent,
+      useCapture || options,
+    );
     return () => {
-      document.removeEventListener(
+      ownerDocument.removeEventListener(
         keyEvent,
         handleKeyEvent,
         useCapture || options,

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

@@ -110,8 +110,8 @@ const PopoverComponent = forwardRef<PopoverPublicAPI, PopoverProps>(
     },
     ref,
   ) {
+    const [isDisplayed, setIsDisplay] = useState(false);
     const [activatorNode, setActivatorNode] = useState<HTMLElement>();
-
     const overlayRef = useRef<PopoverOverlay>(null);
     const activatorContainer = useRef<HTMLElement>(null);
 
@@ -191,6 +191,38 @@ const PopoverComponent = forwardRef<PopoverPublicAPI, PopoverProps>(
       });
     }, [id, active, ariaHaspopup]);
 
+    useEffect(() => {
+      if (!activatorContainer.current) {
+        return;
+      }
+
+      const observer = new ResizeObserver(() => {
+        /**
+         * This is a workaround to prevent rendering the Popover when the content is moved into a React portal that
+         * hasn't been rendered. We don't want to render the Popover in this case because the auto-focus logic will
+         * break. Therefore, we hold back from rendering the Popover until the activatorContainer is displayed.
+         * The activatorContainer is displayed if it has an offsetParent or if the activatorContainer is the body,
+         * if it has a clientWidth bigger than 0.
+         * See: https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/offsetParent
+         */
+
+        setIsDisplay(
+          Boolean(
+            activatorContainer.current &&
+              (activatorContainer.current.offsetParent !== null ||
+                (activatorContainer.current ===
+                  activatorContainer.current.ownerDocument.body &&
+                  activatorContainer.current.clientWidth > 0)),
+          ),
+        );
+      });
+
+      observer.observe(activatorContainer.current);
+      return () => {
+        observer.disconnect();
+      };
+    }, []);
+
     useEffect(() => {
       if (!activatorNode && activatorContainer.current) {
         setActivatorNode(
@@ -217,23 +249,24 @@ const PopoverComponent = forwardRef<PopoverPublicAPI, PopoverProps>(
       setAccessibilityAttributes();
     }, [activatorNode, setAccessibilityAttributes]);
 
-    const portal = activatorNode ? (
-      <Portal idPrefix="popover">
-        <PopoverOverlay
-          ref={overlayRef}
-          id={id}
-          activator={activatorNode}
-          preferInputActivator={preferInputActivator}
-          onClose={handleClose}
-          active={active}
-          fixed={fixed}
-          zIndexOverride={zIndexOverride}
-          {...rest}
-        >
-          {children}
-        </PopoverOverlay>
-      </Portal>
-    ) : null;
+    const portal =
+      activatorNode && isDisplayed ? (
+        <Portal idPrefix="popover">
+          <PopoverOverlay
+            ref={overlayRef}
+            id={id}
+            activator={activatorNode}
+            preferInputActivator={preferInputActivator}
+            onClose={handleClose}
+            active={active}
+            fixed={fixed}
+            zIndexOverride={zIndexOverride}
+            {...rest}
+          >
+            {children}
+          </PopoverOverlay>
+        </Portal>
+      ) : null;
 
     return (
       <WrapperComponent ref={activatorContainer}>

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

@@ -59,6 +59,7 @@ export interface PopoverOverlayProps {
 
 interface State {
   transitionStatus: TransitionStatus;
+  window?: Window | null;
 }
 
 export class PopoverOverlay extends PureComponent<PopoverOverlayProps, State> {
@@ -74,6 +75,7 @@ export class PopoverOverlay extends PureComponent<PopoverOverlayProps, State> {
   private contentNode = createRef<HTMLDivElement>();
   private enteringTimer?: number;
   private overlayRef: React.RefObject<PositionedOverlay>;
+  private observer?: ResizeObserver;
 
   constructor(props: PopoverOverlayProps) {
     super(props);
@@ -97,6 +99,22 @@ export class PopoverOverlay extends PureComponent<PopoverOverlayProps, State> {
 
       this.changeTransitionStatus(TransitionStatus.Entered);
     }
+
+    this.observer = new ResizeObserver(() => {
+      this.setState({
+        /**
+         * This is a workaround to enable event listeners to be
+         * re-attached when moving from one document to another
+         * when using a React portal across iframes.
+         * Using a resize observer works because when the clientWidth
+         * will go from 0 to the real width after the node
+         * gets rendered in its new place.
+         */
+        window: this.props.activator.ownerDocument.defaultView,
+      });
+    });
+
+    this.observer.observe(this.props.activator);
   }
 
   componentDidUpdate(oldProps: PopoverOverlayProps) {
@@ -116,10 +134,16 @@ export class PopoverOverlay extends PureComponent<PopoverOverlayProps, State> {
       this.clearTransitionTimeout();
       this.setState({transitionStatus: TransitionStatus.Exited});
     }
+
+    if (this.props.activator !== oldProps.activator) {
+      this.observer?.disconnect();
+      this.observer?.observe(this.props.activator);
+    }
   }
 
   componentWillUnmount() {
     this.clearTransitionTimeout();
+    this.observer?.disconnect();
   }
 
   render() {
@@ -232,19 +256,25 @@ export class PopoverOverlay extends PureComponent<PopoverOverlayProps, State> {
       fluidContent && styles['Content-fluidContent'],
     );
 
+    const {window} = this.state;
+
     return (
       <div className={className} {...overlay.props}>
         <EventListener
           event="click"
           handler={this.handleClick}
-          window={this.contentNode.current?.ownerDocument.defaultView}
+          window={window}
         />
         <EventListener
           event="touchstart"
           handler={this.handleClick}
-          window={this.contentNode.current?.ownerDocument.defaultView}
+          window={window}
+        />
+        <KeypressListener
+          keyCode={Key.Escape}
+          handler={this.handleEscape}
+          document={window?.document}
         />
-        <KeypressListener keyCode={Key.Escape} handler={this.handleEscape} />
         <div
           className={styles.FocusTracker}
           // eslint-disable-next-line jsx-a11y/no-noninteractive-tabindex

polaris-react/tests/setup/tests.ts

@@ -11,7 +11,7 @@ if (typeof window !== 'undefined' && !matchMedia.isMocked()) {
 
 // eslint-disable-next-line jest/require-top-level-describe
 beforeEach(() => {
-  global.ResizeObserver = jest.fn().mockImplementation(() => ({
+  jest.spyOn(global, 'ResizeObserver').mockImplementation(() => ({
     observe: jest.fn(),
     unobserve: jest.fn(),
     disconnect: jest.fn(),