From f9c2fa3f01cecff16d531d544db61bf5d160a826 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Fri, 11 Dec 2020 14:11:55 +0100 Subject: [PATCH] Add a useDialog hook and replace the duplicated PopoverWrapper (#27643) --- package-lock.json | 52 +++++++------- packages/compose/package.json | 1 + .../compose/src/hooks/use-dialog/README.md | 44 ++++++++++++ .../compose/src/hooks/use-dialog/index.js | 68 +++++++++++++++++++ .../src/hooks/use-focus-on-mount/index.js | 26 ++++--- packages/compose/src/index.js | 1 + .../edit-post/src/components/layout/index.js | 56 +++++++-------- .../src/components/layout/popover-wrapper.js | 61 ----------------- .../src/components/layout/style.scss | 11 --- .../edit-site/src/components/editor/index.js | 67 +++++++++--------- .../src/components/editor/popover-wrapper.js | 61 ----------------- .../src/components/editor/style.scss | 11 --- .../src/components/layout/interface.js | 57 ++++++++-------- .../src/components/layout/popover-wrapper.js | 61 ----------------- .../src/components/layout/style.scss | 12 ---- 15 files changed, 250 insertions(+), 339 deletions(-) create mode 100644 packages/compose/src/hooks/use-dialog/README.md create mode 100644 packages/compose/src/hooks/use-dialog/index.js delete mode 100644 packages/edit-post/src/components/layout/popover-wrapper.js delete mode 100644 packages/edit-site/src/components/editor/popover-wrapper.js delete mode 100644 packages/edit-widgets/src/components/layout/popover-wrapper.js diff --git a/package-lock.json b/package-lock.json index 52fe92d9eba9c9..99a28aa0a9505c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1155,30 +1155,6 @@ "@babel/helper-plugin-utils": "^7.10.4" } }, - "@babel/polyfill": { - "version": "7.12.1", - "resolved": "https://registry.npmjs.org/@babel/polyfill/-/polyfill-7.12.1.tgz", - "integrity": "sha512-X0pi0V6gxLi6lFZpGmeNa4zxtwEmCs42isWLNjZZDE0Y8yVfgu0T2OAHlzBbdYlqbW/YXVvoBHpATEM+goCj8g==", - "dev": true, - "requires": { - "core-js": "^2.6.5", - "regenerator-runtime": "^0.13.4" - }, - "dependencies": { - "core-js": { - "version": "2.6.12", - "resolved": "https://registry.npmjs.org/core-js/-/core-js-2.6.12.tgz", - "integrity": "sha512-Kb2wC0fvsWfQrgk8HU5lW6U/Lcs8+9aaYcy4ZFc6DDlo4nZ7n70dEgE5rtR0oG6ufKDUnrwfWL1mXR5ljDatrQ==", - "dev": true - }, - "regenerator-runtime": { - "version": "0.13.7", - "resolved": "https://registry.npmjs.org/regenerator-runtime/-/regenerator-runtime-0.13.7.tgz", - "integrity": "sha512-a54FxoJDIr27pgf7IgeQGxmqUNYrcV338lf/6gH456HZ/PhX+5BcwHXG9ajESmwe6WRO0tAzRUrRmNONWgkrew==", - "dev": true - } - } - }, "@babel/preset-env": { "version": "7.12.7", "resolved": "https://registry.npmjs.org/@babel/preset-env/-/preset-env-7.12.7.tgz", @@ -13394,6 +13370,7 @@ "clipboard": "^2.0.1", "lodash": "^4.17.19", "mousetrap": "^1.6.5", + "react-merge-refs": "^1.0.0", "react-resize-aware": "^3.0.1", "use-memo-one": "^1.1.1" } @@ -14834,6 +14811,33 @@ "keypather": "^1.10.2" } }, + "@babel/polyfill": { + "version": "7.10.4", + "resolved": "https://registry.npmjs.org/@babel/polyfill/-/polyfill-7.10.4.tgz", + "integrity": "sha512-8BYcnVqQ5kMD2HXoHInBH7H1b/uP3KdnwCYXOqFnXqguOyuu443WXusbIUbWEfY3Z0Txk0M1uG/8YuAMhNl6zg==", + "dev": true, + "requires": { + "core-js": "^2.6.5", + "regenerator-runtime": "^0.13.4" + }, + "dependencies": { + "core-js": { + "version": "2.6.11", + "resolved": "https://registry.npmjs.org/core-js/-/core-js-2.6.11.tgz", + "integrity": "sha512-5wjnpaT/3dV+XB4borEsnAYQchn00XSgTAWKDkEqv+K8KevjbzmofK6hfJ9TZIlpj2N0xQpazy7PiRQiWHqzWg==", + "dev": true + } + } + }, + "@babel/runtime": { + "version": "7.10.4", + "resolved": "https://registry.npmjs.org/@babel/runtime/-/runtime-7.10.4.tgz", + "integrity": "sha512-UpTN5yUJr9b4EX2CnGNWIvER7Ab83ibv0pcvvHc4UOdrBI5jb8bj+32cCwPX6xu0mt2daFNjYhoi+X7beH0RSw==", + "dev": true, + "requires": { + "regenerator-runtime": "^0.13.4" + } + }, "@dabh/diagnostics": { "version": "2.0.2", "resolved": "https://registry.npmjs.org/@dabh/diagnostics/-/diagnostics-2.0.2.tgz", diff --git a/packages/compose/package.json b/packages/compose/package.json index 84190b1643e864..af7dd33c7744ac 100644 --- a/packages/compose/package.json +++ b/packages/compose/package.json @@ -35,6 +35,7 @@ "clipboard": "^2.0.1", "lodash": "^4.17.19", "mousetrap": "^1.6.5", + "react-merge-refs": "^1.0.0", "react-resize-aware": "^3.0.1", "use-memo-one": "^1.1.1" }, diff --git a/packages/compose/src/hooks/use-dialog/README.md b/packages/compose/src/hooks/use-dialog/README.md new file mode 100644 index 00000000000000..6112326c41aefa --- /dev/null +++ b/packages/compose/src/hooks/use-dialog/README.md @@ -0,0 +1,44 @@ +`useDialog` +=========== + +React hook to be used on a dialog wrapper to enable the following behaviors: + + - constrained tabbing. + - focus on mount. + - return focus on unmount. + - focus outside. + +## Returned value + +The hooks returns an array composed of the two following values: + +### `ref` + +- Type: `Object|Function` + +A React ref that must be passed to the DOM element where the behavior should be attached. + +### `props` + +- Type: `Object` + +Extra props to apply to the wrapper. + +## Usage + +```jsx +import { __experimentalUseDialog as useDialog } from '@wordpress/compose'; + +const MyDialog = () => { + const [ ref, extraProps ] = useDialog( { + onClose: () => console.log('do something to close the dialog') + } ); + + return ( +
+
+ ); +}; +``` diff --git a/packages/compose/src/hooks/use-dialog/index.js b/packages/compose/src/hooks/use-dialog/index.js new file mode 100644 index 00000000000000..d0c5f80c1bb4b8 --- /dev/null +++ b/packages/compose/src/hooks/use-dialog/index.js @@ -0,0 +1,68 @@ +/** + * External dependencies + */ +import mergeRefs from 'react-merge-refs'; + +/** + * WordPress dependencies + */ +import { useRef, useCallback, useEffect } from '@wordpress/element'; +import { ESCAPE } from '@wordpress/keycodes'; + +/** + * Internal dependencies + */ +import useConstrainedTabbing from '../use-constrained-tabbing'; +import useFocusOnMount from '../use-focus-on-mount'; +import useFocusReturn from '../use-focus-return'; +import useFocusOutside from '../use-focus-outside'; + +/** + * Returns a ref and props to apply to a dialog wrapper to enable the following behaviors: + * - constrained tabbing. + * - focus on mount. + * - return focus on unmount. + * - focus outside. + * + * @param {Object} options Dialog Options. + */ +function useDialog( options ) { + const onClose = useRef(); + useEffect( () => { + onClose.current = options.onClose; + }, [ options.onClose ] ); + const constrainedTabbingRef = useConstrainedTabbing(); + const focusOnMountRef = useFocusOnMount(); + const focusReturnRef = useFocusReturn(); + const focusOutsideProps = useFocusOutside( options.onClose ); + const closeOnEscapeRef = useCallback( ( node ) => { + if ( ! node ) { + return; + } + + node.addEventListener( 'keydown', ( event ) => { + // Close on escape + if ( event.keyCode === ESCAPE && onClose.current ) { + event.stopPropagation(); + onClose.current(); + } + } ); + + node.addEventListener( 'mousedown', ( event ) => { + // I'm not really certain what this is for but it matches the previous behavior. + event.stopPropagation(); + } ); + }, [] ); + + return [ + mergeRefs( [ + constrainedTabbingRef, + focusOnMountRef, + focusReturnRef, + closeOnEscapeRef, + ] ), + focusOutsideProps, + ]; +} + +export default useDialog; diff --git a/packages/compose/src/hooks/use-focus-on-mount/index.js b/packages/compose/src/hooks/use-focus-on-mount/index.js index a3477686aeb93e..c7faeb742d51c3 100644 --- a/packages/compose/src/hooks/use-focus-on-mount/index.js +++ b/packages/compose/src/hooks/use-focus-on-mount/index.js @@ -2,7 +2,7 @@ * WordPress dependencies */ import { focus } from '@wordpress/dom'; -import { useEffect, useRef } from '@wordpress/element'; +import { useCallback, useEffect, useRef, useState } from '@wordpress/element'; /** * Hook used to focus the first tabbable element on mount. @@ -27,18 +27,21 @@ import { useEffect, useRef } from '@wordpress/element'; */ function useFocusOnMount( focusOnMount = 'firstElement' ) { const focusOnMountRef = useRef( focusOnMount ); + const [ node, setNode ] = useState(); useEffect( () => { focusOnMountRef.current = focusOnMount; }, [ focusOnMount ] ); - // Ideally we should be using a function ref (useCallback) + // Ideally we should be running the focus behavior in the useCallback directly // Right now we have some issues where the link popover remounts // prevents us from doing that. - // The downside of the current implementation is that it doesn't update if the "ref" changes. - const ref = useRef(); + const ref = useCallback( setNode, [] ); // Focus handling useEffect( () => { + if ( ! node ) { + return; + } /* * Without the setTimeout, the dom node is not being focused. Related: * https://stackoverflow.com/questions/35522220/react-ref-with-focus-doesnt-work-without-settimeout-my-example @@ -46,29 +49,32 @@ function useFocusOnMount( focusOnMount = 'firstElement' ) { * TODO: Treat the cause, not the symptom. */ const focusTimeout = setTimeout( () => { - if ( ! focusOnMountRef.current || ! ref.current ) { + if ( focusOnMountRef.current === false || ! node ) { return; } if ( focusOnMountRef.current === 'firstElement' ) { - const firstTabbable = focus.tabbable.find( ref.current )[ 0 ]; + const firstTabbable = focus.tabbable.find( node )[ 0 ]; if ( firstTabbable ) { firstTabbable.focus(); } else { - ref.current.focus(); + node.focus(); } return; } - if ( focusOnMountRef.current === 'container' ) { - ref.current.focus(); + if ( + focusOnMountRef.current === 'container' || + focusOnMountRef.current === true + ) { + node.focus(); } }, 0 ); return () => clearTimeout( focusTimeout ); - }, [] ); + }, [ node ] ); return ref; } diff --git a/packages/compose/src/index.js b/packages/compose/src/index.js index 4c1e333327cbf2..df5362ee366038 100644 --- a/packages/compose/src/index.js +++ b/packages/compose/src/index.js @@ -15,6 +15,7 @@ export { default as withState } from './higher-order/with-state'; // Hooks export { default as useConstrainedTabbing } from './hooks/use-constrained-tabbing'; export { default as useCopyOnClick } from './hooks/use-copy-on-click'; +export { default as __experimentalUseDialog } from './hooks/use-dialog'; export { default as __experimentalUseDragging } from './hooks/use-dragging'; export { default as useFocusOnMount } from './hooks/use-focus-on-mount'; export { default as __experimentalUseFocusOutside } from './hooks/use-focus-outside'; diff --git a/packages/edit-post/src/components/layout/index.js b/packages/edit-post/src/components/layout/index.js index 58b420e53b697b..013c9d18daccba 100644 --- a/packages/edit-post/src/components/layout/index.js +++ b/packages/edit-post/src/components/layout/index.js @@ -25,7 +25,10 @@ import { Popover, __unstableUseDrop as useDrop, } from '@wordpress/components'; -import { useViewportMatch } from '@wordpress/compose'; +import { + useViewportMatch, + __experimentalUseDialog as useDialog, +} from '@wordpress/compose'; import { PluginArea } from '@wordpress/plugins'; import { __ } from '@wordpress/i18n'; import { @@ -52,7 +55,6 @@ import SettingsSidebar from '../sidebar/settings-sidebar'; import MetaBoxes from '../meta-boxes'; import WelcomeGuide from '../welcome-guide'; import ActionsPanel from './actions-panel'; -import PopoverWrapper from './popover-wrapper'; const interfaceLabels = { secondarySidebar: __( 'Block library' ), @@ -169,6 +171,9 @@ function Layout( { settings } ) { useDrop( ref ); useEditorStyles( ref, settings.styles ); + const [ inserterDialogRef, inserterDialogProps ] = useDialog( { + onClose: () => setIsInserterOpened( false ), + } ); return ( <> @@ -194,34 +199,31 @@ function Layout( { settings } ) { secondarySidebar={ mode === 'visual' && isInserterOpened && ( - setIsInserterOpened( false ) } +
-
-
-
-
- +
+
+ { + if ( isMobileViewport ) { + setIsInserterOpened( false ); } - showInserterHelpPanel - onSelect={ () => { - if ( isMobileViewport ) { - setIsInserterOpened( false ); - } - } } - /> -
+ } } + />
- +
) } sidebar={ diff --git a/packages/edit-post/src/components/layout/popover-wrapper.js b/packages/edit-post/src/components/layout/popover-wrapper.js deleted file mode 100644 index 7fecda0c0119da..00000000000000 --- a/packages/edit-post/src/components/layout/popover-wrapper.js +++ /dev/null @@ -1,61 +0,0 @@ -/** - * WordPress dependencies - */ -import { - withConstrainedTabbing, - withFocusReturn, - withFocusOutside, -} from '@wordpress/components'; -import { Component } from '@wordpress/element'; -import { ESCAPE } from '@wordpress/keycodes'; -import { useFocusOnMount } from '@wordpress/compose'; - -function stopPropagation( event ) { - event.stopPropagation(); -} - -const DetectOutside = withFocusOutside( - class extends Component { - handleFocusOutside( event ) { - this.props.onFocusOutside( event ); - } - - render() { - return this.props.children; - } - } -); - -const FocusManaged = withConstrainedTabbing( - withFocusReturn( ( { children } ) => children ) -); - -export default function PopoverWrapper( { onClose, children, className } ) { - // Event handlers - const maybeClose = ( event ) => { - // Close on escape - if ( event.keyCode === ESCAPE && onClose ) { - event.stopPropagation(); - onClose(); - } - }; - - const ref = useFocusOnMount(); - - // Disable reason: this stops certain events from propagating outside of the component. - // - onMouseDown is disabled as this can cause interactions with other DOM elements - /* eslint-disable jsx-a11y/no-static-element-interactions */ - return ( -
- - { children } - -
- ); - /* eslint-enable jsx-a11y/no-static-element-interactions */ -} diff --git a/packages/edit-post/src/components/layout/style.scss b/packages/edit-post/src/components/layout/style.scss index b7dd51c31b4258..dfb1dc7fc4c2ae 100644 --- a/packages/edit-post/src/components/layout/style.scss +++ b/packages/edit-post/src/components/layout/style.scss @@ -95,17 +95,6 @@ background-color: $gray-400; } -// Ideally we don't need all these nested divs. -// Removing these requires a refactoring of the different a11y HoCs. -.edit-post-layout__inserter-panel-popover-wrapper { - &, - & > div, - & > div > div, - & > div > div > div { - height: 100%; - } -} - .edit-post-layout__inserter-panel { height: 100%; display: flex; diff --git a/packages/edit-site/src/components/editor/index.js b/packages/edit-site/src/components/editor/index.js index 380a1f3d1dae99..bcd180fe66735c 100644 --- a/packages/edit-site/src/components/editor/index.js +++ b/packages/edit-site/src/components/editor/index.js @@ -32,7 +32,10 @@ import { EntitiesSavedStates, UnsavedChangesWarning } from '@wordpress/editor'; import { __ } from '@wordpress/i18n'; import { PluginArea } from '@wordpress/plugins'; import { close } from '@wordpress/icons'; -import { useViewportMatch } from '@wordpress/compose'; +import { + useViewportMatch, + __experimentalUseDialog as useDialog, +} from '@wordpress/compose'; /** * Internal dependencies @@ -45,7 +48,6 @@ import KeyboardShortcuts from '../keyboard-shortcuts'; import GlobalStylesProvider from './global-styles-provider'; import NavigationSidebar from '../navigation-sidebar'; import URLQueryController from '../url-query-controller'; -import PopoverWrapper from './popover-wrapper'; const interfaceLabels = { secondarySidebar: __( 'Block Library' ), @@ -173,6 +175,10 @@ function Editor() { useEditorStyles( ref, settings.styles ); + const [ inserterDialogRef, inserterDialogProps ] = useDialog( { + onClose: () => setIsInserterOpened( false ), + } ); + return ( <> @@ -207,43 +213,38 @@ function Editor() { drawer={ } secondarySidebar={ isInserterOpen ? ( - - setIsInserterOpened( - false - ) +
-
-
-
+
+ { + if ( + isMobile + ) { setIsInserterOpened( false - ) + ); } - /> -
-
- { - if ( - isMobile - ) { - setIsInserterOpened( - false - ); - } - } } - /> -
+ } } + />
- +
) : null } sidebar={ diff --git a/packages/edit-site/src/components/editor/popover-wrapper.js b/packages/edit-site/src/components/editor/popover-wrapper.js deleted file mode 100644 index 7fecda0c0119da..00000000000000 --- a/packages/edit-site/src/components/editor/popover-wrapper.js +++ /dev/null @@ -1,61 +0,0 @@ -/** - * WordPress dependencies - */ -import { - withConstrainedTabbing, - withFocusReturn, - withFocusOutside, -} from '@wordpress/components'; -import { Component } from '@wordpress/element'; -import { ESCAPE } from '@wordpress/keycodes'; -import { useFocusOnMount } from '@wordpress/compose'; - -function stopPropagation( event ) { - event.stopPropagation(); -} - -const DetectOutside = withFocusOutside( - class extends Component { - handleFocusOutside( event ) { - this.props.onFocusOutside( event ); - } - - render() { - return this.props.children; - } - } -); - -const FocusManaged = withConstrainedTabbing( - withFocusReturn( ( { children } ) => children ) -); - -export default function PopoverWrapper( { onClose, children, className } ) { - // Event handlers - const maybeClose = ( event ) => { - // Close on escape - if ( event.keyCode === ESCAPE && onClose ) { - event.stopPropagation(); - onClose(); - } - }; - - const ref = useFocusOnMount(); - - // Disable reason: this stops certain events from propagating outside of the component. - // - onMouseDown is disabled as this can cause interactions with other DOM elements - /* eslint-disable jsx-a11y/no-static-element-interactions */ - return ( -
- - { children } - -
- ); - /* eslint-enable jsx-a11y/no-static-element-interactions */ -} diff --git a/packages/edit-site/src/components/editor/style.scss b/packages/edit-site/src/components/editor/style.scss index 52960903d41431..cb8048a51cd5c4 100644 --- a/packages/edit-site/src/components/editor/style.scss +++ b/packages/edit-site/src/components/editor/style.scss @@ -25,17 +25,6 @@ background-color: $white; } -// Ideally we don't need all these nested divs. -// Removing these requires a refactoring of the different a11y HoCs. -.edit-site-editor__inserter-panel-popover-wrapper { - &, - & > div, - & > div > div, - & > div > div > div { - height: 100%; - } -} - .edit-site-editor__inserter-panel { height: 100%; display: flex; diff --git a/packages/edit-widgets/src/components/layout/interface.js b/packages/edit-widgets/src/components/layout/interface.js index 768fc01e6ae407..db6440afbd6291 100644 --- a/packages/edit-widgets/src/components/layout/interface.js +++ b/packages/edit-widgets/src/components/layout/interface.js @@ -2,7 +2,10 @@ * WordPress dependencies */ import { Button } from '@wordpress/components'; -import { useViewportMatch } from '@wordpress/compose'; +import { + __experimentalUseDialog as useDialog, + useViewportMatch, +} from '@wordpress/compose'; import { close } from '@wordpress/icons'; import { __experimentalLibrary as Library, @@ -18,7 +21,6 @@ import { __ } from '@wordpress/i18n'; */ import Header from '../header'; import WidgetAreasBlockEditorContent from '../widget-areas-block-editor-content'; -import PopoverWrapper from './popover-wrapper'; import useWidgetLibraryInsertionPoint from '../../hooks/use-widget-library-insertion-point'; const interfaceLabels = { @@ -61,6 +63,10 @@ function Interface( { blockEditorSettings } ) { } }, [ isInserterOpened, isHugeViewport ] ); + const [ inserterDialogRef, inserterDialogProps ] = useDialog( { + onClose: () => setIsInserterOpened( false ), + } ); + return ( } secondarySidebar={ isInserterOpened && ( - setIsInserterOpened( false ) } +
-
-
-
-
- { - if ( isMobileViewport ) { - setIsInserterOpened( false ); - } - } } - rootClientId={ rootClientId } - __experimentalInsertionIndex={ - insertionIndex +
+
+
+ { + if ( isMobileViewport ) { + setIsInserterOpened( false ); } - /> -
+ } } + rootClientId={ rootClientId } + __experimentalInsertionIndex={ insertionIndex } + />
- +
) } sidebar={ diff --git a/packages/edit-widgets/src/components/layout/popover-wrapper.js b/packages/edit-widgets/src/components/layout/popover-wrapper.js deleted file mode 100644 index 7fecda0c0119da..00000000000000 --- a/packages/edit-widgets/src/components/layout/popover-wrapper.js +++ /dev/null @@ -1,61 +0,0 @@ -/** - * WordPress dependencies - */ -import { - withConstrainedTabbing, - withFocusReturn, - withFocusOutside, -} from '@wordpress/components'; -import { Component } from '@wordpress/element'; -import { ESCAPE } from '@wordpress/keycodes'; -import { useFocusOnMount } from '@wordpress/compose'; - -function stopPropagation( event ) { - event.stopPropagation(); -} - -const DetectOutside = withFocusOutside( - class extends Component { - handleFocusOutside( event ) { - this.props.onFocusOutside( event ); - } - - render() { - return this.props.children; - } - } -); - -const FocusManaged = withConstrainedTabbing( - withFocusReturn( ( { children } ) => children ) -); - -export default function PopoverWrapper( { onClose, children, className } ) { - // Event handlers - const maybeClose = ( event ) => { - // Close on escape - if ( event.keyCode === ESCAPE && onClose ) { - event.stopPropagation(); - onClose(); - } - }; - - const ref = useFocusOnMount(); - - // Disable reason: this stops certain events from propagating outside of the component. - // - onMouseDown is disabled as this can cause interactions with other DOM elements - /* eslint-disable jsx-a11y/no-static-element-interactions */ - return ( -
- - { children } - -
- ); - /* eslint-enable jsx-a11y/no-static-element-interactions */ -} diff --git a/packages/edit-widgets/src/components/layout/style.scss b/packages/edit-widgets/src/components/layout/style.scss index 4b7c34cf9451d3..b42fc1a39aad42 100644 --- a/packages/edit-widgets/src/components/layout/style.scss +++ b/packages/edit-widgets/src/components/layout/style.scss @@ -1,15 +1,3 @@ - -// Ideally we don't need all these nested divs. -// Removing these requires a refactoring of the different a11y HoCs. -.edit-widgets-layout__inserter-panel-popover-wrapper { - &, - & > div, - & > div > div, - & > div > div > div { - height: 100%; - } -} - .edit-widgets-layout__inserter-panel { height: 100%; display: flex;