[EuiFlyout] Scope close events to mask when ownFocus=true (#5876)

* euiOverlayMask accept ref; mock updates * euiFocusTrap pass event to callback * euiFlyout take advantage of ref and event data * CL * cypress tests * use maskRef prop instead of forwardRef * clean up * outsideClickCloses=false test; comment * refactor onClickOutside; update name to hasOverlayMask
elastic · May 6, 2022 · b3f3f5d · b3f3f5d
1 parent b7f6eac
commit b3f3f5d
Show file tree

Hide file tree

Showing 7 changed files with 119 additions and 25 deletions.
diff --git a/src/components/collapsible_nav/collapsible_nav.test.tsx b/src/components/collapsible_nav/collapsible_nav.test.tsx
@@ -14,8 +14,8 @@ import { EuiCollapsibleNav } from './collapsible_nav';
 import { EuiOverlayMaskProps } from '../overlay_mask';
 
 jest.mock('../overlay_mask', () => ({
-  EuiOverlayMask: ({ headerZindexLocation, ...props }: any) => (
-    <div {...props} />
+  EuiOverlayMask: ({ headerZindexLocation, maskRef, ...props }: any) => (
+    <div {...props} ref={maskRef} />
   ),
 }));
 

diff --git a/src/components/flyout/flyout.spec.tsx b/src/components/flyout/flyout.spec.tsx
@@ -10,6 +10,7 @@
 
 import React, { useState } from 'react';
 
+import { EuiGlobalToastList } from '../toast';
 import { EuiFlyout } from './flyout';
 
 const childrenDefault = (
@@ -21,13 +22,17 @@ const childrenDefault = (
   </>
 );
 
-const Flyout = ({ children = childrenDefault }) => {
+const Flyout = ({ children = childrenDefault, ...rest }) => {
   const [isOpen, setIsOpen] = useState(true);
 
   return (
     <>
       {isOpen ? (
-        <EuiFlyout data-test-subj="flyoutSpec" onClose={() => setIsOpen(false)}>
+        <EuiFlyout
+          data-test-subj="flyoutSpec"
+          onClose={() => setIsOpen(false)}
+          {...rest}
+        >
           {children}
         </EuiFlyout>
       ) : null}
@@ -62,7 +67,7 @@ describe('EuiFlyout', () => {
     });
   });
 
-  describe('Close behavior', () => {
+  describe('Close behavior: standard', () => {
     it('closes the flyout when the close button is clicked', () => {
       cy.mount(<Flyout />);
       cy.realPress('Enter').then(() => {
@@ -76,6 +81,39 @@ describe('EuiFlyout', () => {
         expect(cy.get('[data-test-subj="flyoutSpec"]').should('not.exist'));
       });
     });
+  });
+
+  describe('Close behavior: outside clicks', () => {
+    // We're using toasts here to trigger outside clicks, as a UX case where
+    // we would generally expect toasts overlaid on top of a flyout *not* to close the flyout
+    const FlyoutWithToasts = ({ children = childrenDefault, ...rest }) => {
+      const [isOpen, setIsOpen] = useState(true);
+
+      return (
+        <>
+          {isOpen ? (
+            <EuiFlyout
+              data-test-subj="flyoutSpec"
+              onClose={() => setIsOpen(false)}
+              {...rest}
+            >
+              {children}
+            </EuiFlyout>
+          ) : null}
+          <EuiGlobalToastList
+            toasts={[
+              {
+                title: 'Toast!',
+                text: 'Yeah toast',
+                id: 'a',
+              },
+            ]}
+            dismissToast={() => {}}
+            toastLifeTimeMs={10000}
+          />
+        </>
+      );
+    };
 
     it('closes the flyout when the overlay mask is clicked', () => {
       cy.mount(<Flyout />);
@@ -86,6 +124,15 @@ describe('EuiFlyout', () => {
         });
     });
 
+    it('does not close the flyout when `outsideClickCloses=false` and the overlay mask is clicked', () => {
+      cy.mount(<Flyout outsideClickCloses={false} />);
+      cy.get('.euiOverlayMask')
+        .realClick()
+        .then(() => {
+          expect(cy.get('[data-test-subj="flyoutSpec"]').should('exist'));
+        });
+    });
+
     it('does not close the flyout when the overlay mask is only the target of mouseup', () => {
       cy.mount(<Flyout />);
       cy.get('[data-test-subj="itemD"]').realMouseDown().realMouseMove(-100, 0);
@@ -95,5 +142,23 @@ describe('EuiFlyout', () => {
           expect(cy.get('[data-test-subj="flyoutSpec"]').should('exist'));
         });
     });
+
+    it('does not close the flyout when the toast is clicked when `ownFocus=true`', () => {
+      cy.mount(<FlyoutWithToasts />);
+      cy.get('[data-test-subj="toastCloseButton"]')
+        .realClick()
+        .then(() => {
+          expect(cy.get('[data-test-subj="flyoutSpec"]').should('exist'));
+        });
+    });
+
+    it('closes the flyout when the toast is clicked when `ownFocus=false`', () => {
+      cy.mount(<FlyoutWithToasts ownFocus={false} outsideClickCloses={true} />);
+      cy.get('[data-test-subj="toastCloseButton"]')
+        .realClick()
+        .then(() => {
+          expect(cy.get('[data-test-subj="flyoutSpec"]').should('not.exist'));
+        });
+    });
   });
 });
diff --git a/src/components/flyout/flyout.test.tsx b/src/components/flyout/flyout.test.tsx
@@ -13,8 +13,8 @@ import { requiredProps, takeMountedSnapshot } from '../../test';
 import { EuiFlyout, SIZES, PADDING_SIZES, SIDES } from './flyout';
 
 jest.mock('../overlay_mask', () => ({
-  EuiOverlayMask: ({ headerZindexLocation, ...props }: any) => (
-    <div {...props} />
+  EuiOverlayMask: ({ headerZindexLocation, maskRef, ...props }: any) => (
+    <div {...props} ref={maskRef} />
   ),
 }));
 

diff --git a/src/components/flyout/flyout.tsx b/src/components/flyout/flyout.tsx
@@ -8,14 +8,15 @@
 
 import React, {
   useEffect,
+  useRef,
   useState,
   forwardRef,
   ComponentPropsWithRef,
   CSSProperties,
   ElementType,
   Fragment,
   FunctionComponent,
-  MouseEvent,
+  MouseEvent as ReactMouseEvent,
   MutableRefObject,
 } from 'react';
 import classnames from 'classnames';
@@ -81,7 +82,7 @@ export const PADDING_SIZES = keysOf(paddingSizeToClassNameMap);
 type _EuiFlyoutPaddingSize = typeof PADDING_SIZES[number];
 
 interface _EuiFlyoutProps {
-  onClose: () => void;
+  onClose: (event: MouseEvent | TouchEvent | KeyboardEvent) => void;
   /**
    * Defines the width of the panel.
    * Pass a predefined size of `s | m | l`, or pass any number/string compatible with the CSS `width` attribute
@@ -204,6 +205,7 @@ export const EuiFlyout = forwardRef(
       | null
   ) => {
     const Element = as || defaultElement;
+    const maskRef = useRef<HTMLDivElement>(null);
     /**
      * Setting the initial state of pushed based on the `type` prop
      * and if the current window size is large enough (larger than `pushMinBreakpoint`)
@@ -282,7 +284,7 @@ export const EuiFlyout = forwardRef(
     const onKeyDown = (event: KeyboardEvent) => {
       if (!isPushed && event.key === keys.ESCAPE) {
         event.preventDefault();
-        onClose();
+        onClose(event);
       }
     };
 
@@ -336,22 +338,30 @@ export const EuiFlyout = forwardRef(
               data-test-subj="euiFlyoutCloseButton"
               {...closeButtonProps}
               className={closeButtonClasses}
-              onClick={(e: MouseEvent<HTMLButtonElement>) => {
-                onClose();
-                closeButtonProps?.onClick && closeButtonProps.onClick(e);
+              onClick={(e: ReactMouseEvent<HTMLButtonElement>) => {
+                onClose(e.nativeEvent);
+                closeButtonProps?.onClick?.(e);
               }}
             />
           )}
         </EuiI18n>
       );
     }
 
-    const isDefaultConfiguration = ownFocus && !isPushed;
-    const onClickOutside =
-      (isDefaultConfiguration && outsideClickCloses !== false) ||
-      outsideClickCloses === true
-        ? onClose
-        : undefined;
+    const hasOverlayMask = ownFocus && !isPushed;
+    const onClickOutside = (event: MouseEvent | TouchEvent) => {
+      // Do not close the flyout for any external click
+      if (outsideClickCloses === false) return undefined;
+      if (hasOverlayMask) {
+        // The overlay mask is present, so only clicks on the mask should close the flyout, regardless of outsideClickCloses
+        if (event.target === maskRef.current) return onClose(event);
+      } else {
+        // No overlay mask is present, so any outside clicks should close the flyout
+        if (outsideClickCloses === true) return onClose(event);
+      }
+      // Otherwise if ownFocus is false and outsideClickCloses is undefined, outside clicks should not close the flyout
+      return undefined;
+    };
     /*
      * Trap focus even when `ownFocus={false}`, otherwise closing
      * the flyout won't return focus to the originating button.
@@ -386,9 +396,13 @@ export const EuiFlyout = forwardRef(
     );
 
     // If ownFocus is set, wrap with an overlay and allow the user to click it to close it.
-    if (isDefaultConfiguration) {
+    const mergedMaskProps = {
+      ...maskProps,
+      maskRef: useCombinedRefs([maskProps?.maskRef, maskRef]),
+    };
+    if (hasOverlayMask) {
       flyout = (
-        <EuiOverlayMask headerZindexLocation="below" {...maskProps}>
+        <EuiOverlayMask headerZindexLocation="below" {...mergedMaskProps}>
           {flyout}
         </EuiOverlayMask>
       );

diff --git a/src/components/focus_trap/focus_trap.tsx b/src/components/focus_trap/focus_trap.tsx
@@ -98,14 +98,14 @@ export class EuiFocusTrap extends Component<EuiFocusTrapProps, State> {
     document.removeEventListener('touchend', this.onMouseupOutside);
   };
 
-  handleOutsideClick: ReactFocusOnProps['onClickOutside'] = (...args) => {
+  handleOutsideClick: ReactFocusOnProps['onClickOutside'] = (event) => {
     const { onClickOutside, clickOutsideDisables, closeOnMouseup } = this.props;
     if (clickOutsideDisables) {
       this.setState({ hasBeenDisabledByClick: true });
     }
 
     if (onClickOutside) {
-      closeOnMouseup ? this.addMouseupListener() : onClickOutside(...args);
+      closeOnMouseup ? this.addMouseupListener() : onClickOutside(event);
     }
   };
 

diff --git a/src/components/overlay_mask/overlay_mask.tsx b/src/components/overlay_mask/overlay_mask.tsx
@@ -14,14 +14,17 @@
 import React, {
   FunctionComponent,
   HTMLAttributes,
+  MutableRefObject,
   ReactNode,
+  Ref,
   useEffect,
   useRef,
   useState,
 } from 'react';
 import { createPortal } from 'react-dom';
 import classNames from 'classnames';
 import { CommonProps, keysOf } from '../common';
+import { useCombinedRefs } from '../../services';
 
 export interface EuiOverlayMaskInterface {
   /**
@@ -36,6 +39,10 @@ export interface EuiOverlayMaskInterface {
    * Should the mask visually sit above or below the EuiHeader (controlled by z-index)
    */
   headerZindexLocation?: 'above' | 'below';
+  /**
+   * React ref to be passed to the wrapping container
+   */
+  maskRef?: Ref<HTMLDivElement> | MutableRefObject<HTMLDivElement>;
 }
 
 export type EuiOverlayMaskProps = CommonProps &
@@ -50,9 +57,11 @@ export const EuiOverlayMask: FunctionComponent<EuiOverlayMaskProps> = ({
   children,
   onClick,
   headerZindexLocation = 'above',
+  maskRef,
   ...rest
 }) => {
   const overlayMaskNode = useRef<HTMLDivElement>();
+  const combinedMaskRef = useCombinedRefs([overlayMaskNode, maskRef]);
   const [isPortalTargetReady, setIsPortalTargetReady] = useState(false);
 
   useEffect(() => {
@@ -65,9 +74,9 @@ export const EuiOverlayMask: FunctionComponent<EuiOverlayMaskProps> = ({
 
   useEffect(() => {
     if (typeof document !== 'undefined') {
-      overlayMaskNode.current = document.createElement('div');
+      combinedMaskRef(document.createElement('div'));
     }
-  }, []);
+  }, []); // eslint-disable-line react-hooks/exhaustive-deps
 
   useEffect(() => {
     const portalTarget = overlayMaskNode.current;

diff --git a/upcoming_changelogs/5876.md b/upcoming_changelogs/5876.md
@@ -0,0 +1,6 @@
+- Updated `EuiOverlayMask` to accept a React ref
+
+**Bug fixes**
+
+- Fixed `EuiFlyout` `outsideClickCloses` not being scoped to overlay mask when `ownFocus=true`
+