diff --git a/src/components/collapsible_nav/collapsible_nav.test.tsx b/src/components/collapsible_nav/collapsible_nav.test.tsx index 650930c587d..d714aa84b26 100644 --- 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) => ( -
+ EuiOverlayMask: ({ headerZindexLocation, maskRef, ...props }: any) => ( +
), })); diff --git a/src/components/flyout/flyout.spec.tsx b/src/components/flyout/flyout.spec.tsx index 930dc65f706..881733b5be6 100644 --- 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 ? ( - setIsOpen(false)}> + setIsOpen(false)} + {...rest} + > {children} ) : 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(); 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 ? ( + setIsOpen(false)} + {...rest} + > + {children} + + ) : null} + {}} + toastLifeTimeMs={10000} + /> + + ); + }; it('closes the flyout when the overlay mask is clicked', () => { cy.mount(); @@ -86,6 +124,15 @@ describe('EuiFlyout', () => { }); }); + it('does not close the flyout when `outsideClickCloses=false` and the overlay mask is clicked', () => { + cy.mount(); + 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(); 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(); + 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(); + 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 index 79bfa265fc4..a27ec279443 100644 --- 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) => ( -
+ EuiOverlayMask: ({ headerZindexLocation, maskRef, ...props }: any) => ( +
), })); diff --git a/src/components/flyout/flyout.tsx b/src/components/flyout/flyout.tsx index f79f5b5a241..ecf2612669d 100644 --- a/src/components/flyout/flyout.tsx +++ b/src/components/flyout/flyout.tsx @@ -8,6 +8,7 @@ import React, { useEffect, + useRef, useState, forwardRef, ComponentPropsWithRef, @@ -15,7 +16,7 @@ import React, { 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(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,9 +338,9 @@ export const EuiFlyout = forwardRef( data-test-subj="euiFlyoutCloseButton" {...closeButtonProps} className={closeButtonClasses} - onClick={(e: MouseEvent) => { - onClose(); - closeButtonProps?.onClick && closeButtonProps.onClick(e); + onClick={(e: ReactMouseEvent) => { + onClose(e.nativeEvent); + closeButtonProps?.onClick?.(e); }} /> )} @@ -346,12 +348,20 @@ export const EuiFlyout = forwardRef( ); } - 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 = ( - + {flyout} ); diff --git a/src/components/focus_trap/focus_trap.tsx b/src/components/focus_trap/focus_trap.tsx index b210e9f902c..b6403ff39de 100644 --- 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 { 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 index 114fa01dc8a..dc51bb8c0c9 100644 --- a/src/components/overlay_mask/overlay_mask.tsx +++ b/src/components/overlay_mask/overlay_mask.tsx @@ -14,7 +14,9 @@ import React, { FunctionComponent, HTMLAttributes, + MutableRefObject, ReactNode, + Ref, useEffect, useRef, useState, @@ -22,6 +24,7 @@ import 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 | MutableRefObject; } export type EuiOverlayMaskProps = CommonProps & @@ -50,9 +57,11 @@ export const EuiOverlayMask: FunctionComponent = ({ children, onClick, headerZindexLocation = 'above', + maskRef, ...rest }) => { const overlayMaskNode = useRef(); + const combinedMaskRef = useCombinedRefs([overlayMaskNode, maskRef]); const [isPortalTargetReady, setIsPortalTargetReady] = useState(false); useEffect(() => { @@ -65,9 +74,9 @@ export const EuiOverlayMask: FunctionComponent = ({ 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 new file mode 100644 index 00000000000..716db53c327 --- /dev/null +++ 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` +