diff --git a/packages/eui/src/components/focus_trap/focus_trap.stories.tsx b/packages/eui/src/components/focus_trap/focus_trap.stories.tsx index c37f331b9309..475290b7ee9e 100644 --- a/packages/eui/src/components/focus_trap/focus_trap.stories.tsx +++ b/packages/eui/src/components/focus_trap/focus_trap.stories.tsx @@ -14,7 +14,7 @@ import { EuiButton } from '../button'; import { EuiFieldText } from '../form'; import { EuiSpacer } from '../spacer'; import { EuiPanel } from '../panel'; -import { EuiProvider } from '../provider'; +import { EuiComponentDefaultsProvider } from '../provider'; import { EuiFocusTrap, EuiFocusTrapProps } from './focus_trap'; const meta: Meta = { @@ -98,13 +98,15 @@ export const Iframe: Story = { export const EuiProviderComponentDefaults: Story = { render: ({ ...args }) => ( - + This story is passing all controls and their arguments to EuiProvider's `componentDefaults` instead of to EuiFocusTrap directly. It's primarily useful for testing that configured defaults behave the same way as individual props. - + ), }; diff --git a/packages/eui/src/components/focus_trap/focus_trap.tsx b/packages/eui/src/components/focus_trap/focus_trap.tsx index e71a892de1e1..2df83c43d115 100644 --- a/packages/eui/src/components/focus_trap/focus_trap.tsx +++ b/packages/eui/src/components/focus_trap/focus_trap.tsx @@ -6,13 +6,24 @@ * Side Public License, v 1. */ -import React, { Component, FunctionComponent, CSSProperties } from 'react'; +import React, { + FunctionComponent, + CSSProperties, + useState, + useEffect, + useCallback, + useRef, +} from 'react'; import { FocusOn } from 'react-focus-on'; import { ReactFocusOnProps } from 'react-focus-on/dist/es5/types'; import { RemoveScrollBar } from 'react-remove-scroll-bar'; +import { + findElementBySelectorOrRef, + ElementTarget, + useUpdateEffect, +} from '../../services'; import { CommonProps } from '../common'; -import { findElementBySelectorOrRef, ElementTarget } from '../../services'; import { usePropsWithComponentDefaults } from '../provider/component_defaults'; export type FocusTarget = ElementTarget; @@ -82,117 +93,115 @@ export type EuiFocusTrapProps = Omit< returnFocus?: ReactFocusOnProps['returnFocus']; }; -export const EuiFocusTrap: FunctionComponent = (props) => { - const propsWithDefaults = usePropsWithComponentDefaults( - 'EuiFocusTrap', - props - ); - return ; -}; - -interface State { - hasBeenDisabledByClick: boolean; -} - -class EuiFocusTrapClass extends Component { - static defaultProps = { - clickOutsideDisables: false, - disabled: false, - returnFocus: true, - noIsolation: true, - scrollLock: false, - crossFrame: false, - gapMode: 'padding', // EUI defaults to padding because Kibana's body/layout CSS ignores `margin` - }; - - state: State = { - hasBeenDisabledByClick: false, - }; - - lastInterceptedEvent: Event | null = null; - preventFocusExit = false; - - componentDidMount() { - this.setInitialFocus(this.props.initialFocus); - } - - componentDidUpdate(prevProps: EuiFocusTrapProps) { - if (prevProps.disabled === true && this.props.disabled === false) { - this.setState({ hasBeenDisabledByClick: false }); - } - } - - componentWillUnmount() { - this.removeMouseupListener(); - } +export const EuiFocusTrap: FunctionComponent = (_props) => { + const props = usePropsWithComponentDefaults('EuiFocusTrap', _props); + const { + children, + disabled, + clickOutsideDisables = false, + returnFocus = true, + noIsolation = true, + crossFrame = false, + scrollLock = false, + initialFocus, + gapMode = 'padding', + closeOnMouseup, + onClickOutside, + ...rest + } = props; + const [hasBeenDisabledByClick, setHasBeenDisabledByClick] = useState(false); + + const isDisabled = disabled || hasBeenDisabledByClick; // Programmatically sets focus on a nested DOM node; optional - setInitialFocus = (initialFocus?: FocusTarget) => { + const setInitialFocus = (initialFocus?: FocusTarget) => { if (!initialFocus) return; + const node = findElementBySelectorOrRef(initialFocus); + if (!node) return; // `data-autofocus` is part of the 'react-focus-on' API node.setAttribute('data-autofocus', 'true'); }; - onMouseupOutside = (e: MouseEvent | TouchEvent) => { - this.removeMouseupListener(); - // Timeout gives precedence to the consumer to initiate close if it has toggle behavior. - // Otherwise this event may occur first and the consumer toggle will reopen the flyout. - setTimeout(() => this.props.onClickOutside?.(e)); - }; + // Stabilize the onClickOutside callback + const onClickOutsideRef = useRef(onClickOutside); + onClickOutsideRef.current = onClickOutside; + + // We use a ref to store the listener to prevent circular dependencies + // while still ensuring the listeners can properly be cleaned up + const mouseupListenerRef = useRef< + ((e: MouseEvent | TouchEvent) => void) | null + >(null); + + const removeMouseupListener = useCallback(() => { + if (mouseupListenerRef.current) { + document.removeEventListener('mouseup', mouseupListenerRef.current); + document.removeEventListener('touchend', mouseupListenerRef.current); + mouseupListenerRef.current = null; + } + }, []); - addMouseupListener = () => { - document.addEventListener('mouseup', this.onMouseupOutside); - document.addEventListener('touchend', this.onMouseupOutside); - }; + const addMouseupListener = useCallback(() => { + removeMouseupListener(); - removeMouseupListener = () => { - document.removeEventListener('mouseup', this.onMouseupOutside); - document.removeEventListener('touchend', this.onMouseupOutside); - }; + mouseupListenerRef.current = (e: MouseEvent | TouchEvent) => { + removeMouseupListener(); + // Timeout gives precedence to the consumer to initiate close if it has toggle behavior. + // Otherwise this event may occur first and the consumer toggle will reopen the flyout. + setTimeout(() => onClickOutsideRef.current?.(e)); + }; + document.addEventListener('mouseup', mouseupListenerRef.current); + document.addEventListener('touchend', mouseupListenerRef.current); + }, [removeMouseupListener]); + + const handleOutsideClick = useCallback( + (event: MouseEvent | TouchEvent) => { + if (clickOutsideDisables) { + setHasBeenDisabledByClick(true); + } + + if (onClickOutside) { + closeOnMouseup ? addMouseupListener() : onClickOutside(event); + } + }, + [clickOutsideDisables, closeOnMouseup, onClickOutside, addMouseupListener] + ); - handleOutsideClick: ReactFocusOnProps['onClickOutside'] = (event) => { - const { onClickOutside, clickOutsideDisables, closeOnMouseup } = this.props; - if (clickOutsideDisables) { - this.setState({ hasBeenDisabledByClick: true }); - } + useEffect(() => { + setInitialFocus(initialFocus); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); - if (onClickOutside) { - closeOnMouseup ? this.addMouseupListener() : onClickOutside(event); + useUpdateEffect(() => { + if (!disabled) { + setHasBeenDisabledByClick(false); } + }, [disabled]); + + // listener cleanup on unmount + useEffect(() => () => removeMouseupListener(), [removeMouseupListener]); + + const focusOnProps = { + returnFocus, + noIsolation, + initialFocus, + crossFrame, + enabled: !isDisabled, + ...rest, + onClickOutside: handleOutsideClick, + /** + * `scrollLock` should always be unset on FocusOn, as it can prevent scrolling on + * portals (i.e. popovers, comboboxes, dropdown menus, etc.) within modals & flyouts + * @see https://github.com/theKashey/react-focus-on/issues/49 + */ + scrollLock: false, }; - render() { - const { - children, - clickOutsideDisables, - disabled, - returnFocus, - noIsolation, - scrollLock, - gapMode, - ...rest - } = this.props; - const isDisabled = disabled || this.state.hasBeenDisabledByClick; - const focusOnProps = { - returnFocus, - noIsolation, - enabled: !isDisabled, - ...rest, - onClickOutside: this.handleOutsideClick, - /** - * `scrollLock` should always be unset on FocusOn, as it can prevent scrolling on - * portals (i.e. popovers, comboboxes, dropdown menus, etc.) within modals & flyouts - * @see https://github.com/theKashey/react-focus-on/issues/49 - */ - scrollLock: false, - }; - return ( - - {children} - {!isDisabled && scrollLock && } - - ); - } -} + return ( + + {children} + {!isDisabled && scrollLock && } + + ); +};