Make the inserter behave as a popover (#24429)

WordPress · Aug 10, 2020 · 4e5019e · 4e5019e
1 parent 1fdbab7
commit 4e5019e
Show file tree

Hide file tree

Showing 8 changed files with 151 additions and 57 deletions.
diff --git a/packages/block-editor/src/components/inserter/quick-inserter.js b/packages/block-editor/src/components/inserter/quick-inserter.js
@@ -193,8 +193,12 @@ function QuickInserter( {
 	const onBrowseAll = () => {
 		// We have to select the previous block because the menu inserter
 		// inserts the new block after the selected one.
+		// Ideally, this selection shouldn't focus the block to avoid the setTimeout.
 		selectBlock( previousBlockClientId );
-		setInsererIsOpened( true );
+		// eslint-disable-next-line @wordpress/react-no-unsafe-timeout
+		setTimeout( () => {
+			setInsererIsOpened( true );
+		} );
 	};
 
 	// Disable reason (no-autofocus): The inserter menu is a modal display, not one which

diff --git a/packages/e2e-test-utils/src/inserter.js b/packages/e2e-test-utils/src/inserter.js
@@ -58,6 +58,7 @@ async function toggleGlobalBlockInserter() {
  */
 export async function searchForBlock( searchTerm ) {
 	await openGlobalBlockInserter();
+	await page.waitForSelector( INSERTER_SEARCH_SELECTOR );
 	await page.focus( INSERTER_SEARCH_SELECTOR );
 	await pressKeyWithModifier( 'primary', 'a' );
 	await page.keyboard.type( searchTerm );
@@ -71,10 +72,11 @@ export async function searchForBlock( searchTerm ) {
 export async function searchForPattern( searchTerm ) {
 	await openGlobalBlockInserter();
 	// Select the patterns tab
-	const [ tab ] = await page.$x(
+	const tab = await page.waitForXPath(
 		'//div[contains(@class, "block-editor-inserter__tabs")]//button[.="Patterns"]'
 	);
 	await tab.click();
+	await page.waitForSelector( INSERTER_SEARCH_SELECTOR );
 	await page.focus( INSERTER_SEARCH_SELECTOR );
 	await pressKeyWithModifier( 'primary', 'a' );
 	await page.keyboard.type( searchTerm );
@@ -96,10 +98,11 @@ export async function searchForReusableBlock( searchTerm ) {
 	);
 
 	// Select the reusable blocks tab.
-	const [ tab ] = await page.$x(
+	const tab = await page.waitForXPath(
 		'//div[contains(@class, "block-editor-inserter__tabs")]//button[text()="Reusable"]'
 	);
 	await tab.click();
+	await page.waitForSelector( INSERTER_SEARCH_SELECTOR );
 	await page.focus( INSERTER_SEARCH_SELECTOR );
 	await pressKeyWithModifier( 'primary', 'a' );
 	await page.keyboard.type( searchTerm );
@@ -117,6 +120,12 @@ export async function insertBlock( searchTerm ) {
 		await page.$x( `//button//span[contains(text(), '${ searchTerm }')]` )
 	 )[ 0 ];
 	await insertButton.click();
+	// We should wait until the inserter closes and the focus moves to the content.
+	await page.waitForFunction( () =>
+		document.body
+			.querySelector( '.block-editor-block-list__layout' )
+			.contains( document.activeElement )
+	);
 }
 
 /**
@@ -133,6 +142,12 @@ export async function insertPattern( searchTerm ) {
 		)
 	 )[ 0 ];
 	await insertButton.click();
+	// We should wait until the inserter closes and the focus moves to the content.
+	await page.waitForFunction( () =>
+		document.body
+			.querySelector( '.block-editor-block-list__layout' )
+			.contains( document.activeElement )
+	);
 }
 
 /**
@@ -148,4 +163,10 @@ export async function insertReusableBlock( searchTerm ) {
 		await page.$x( `//button//span[contains(text(), '${ searchTerm }')]` )
 	 )[ 0 ];
 	await insertButton.click();
+	// We should wait until the inserter closes and the focus moves to the content.
+	await page.waitForFunction( () =>
+		document.body
+			.querySelector( '.block-editor-block-list__layout' )
+			.contains( document.activeElement )
+	);
 }
diff --git a/packages/e2e-tests/specs/editor/plugins/align-hook.test.js b/packages/e2e-tests/specs/editor/plugins/align-hook.test.js
@@ -10,6 +10,7 @@ import {
 	insertBlock,
 	selectBlockByClientId,
 	setPostContent,
+	clickBlockToolbarButton,
 } from '@wordpress/e2e-test-utils';
 
 const alignLabels = {
@@ -21,9 +22,6 @@ const alignLabels = {
 };
 
 describe( 'Align Hook Works As Expected', () => {
-	const CHANGE_ALIGNMENT_BUTTON_SELECTOR =
-		'.block-editor-block-toolbar .components-dropdown-menu__toggle[aria-label="Change alignment"]';
-
 	beforeAll( async () => {
 		await activatePlugin( 'gutenberg-test-align-hook' );
 	} );
@@ -37,11 +35,7 @@ describe( 'Align Hook Works As Expected', () => {
 	} );
 
 	const getAlignmentToolbarLabels = async () => {
-		const element = await page.waitForSelector(
-			CHANGE_ALIGNMENT_BUTTON_SELECTOR
-		);
-		await element.click();
-
+		await clickBlockToolbarButton( 'Change alignment' );
 		const buttonLabels = await page.evaluate( () => {
 			return Array.from(
 				document.querySelectorAll(
@@ -57,20 +51,14 @@ describe( 'Align Hook Works As Expected', () => {
 	const createShowsTheExpectedButtonsTest = ( blockName, buttonLabels ) => {
 		it( 'Shows the expected buttons on the alignment toolbar', async () => {
 			await insertBlock( blockName );
-
 			expect( await getAlignmentToolbarLabels() ).toEqual( buttonLabels );
 		} );
 	};
 
 	const createDoesNotApplyAlignmentByDefaultTest = ( blockName ) => {
 		it( 'Does not apply any alignment by default', async () => {
 			await insertBlock( blockName );
-
-			// verify no alignment button is in pressed state
-			const element = await page.waitForSelector(
-				CHANGE_ALIGNMENT_BUTTON_SELECTOR
-			);
-			await element.click();
+			await clickBlockToolbarButton( 'Change alignment' );
 			const pressedButtons = await page.$$(
 				'.components-dropdown-menu__menu button.is-active'
 			);
@@ -95,14 +83,11 @@ describe( 'Align Hook Works As Expected', () => {
 				'.components-dropdown-menu__menu button.is-active';
 			// set the specified alignment.
 			await insertBlock( blockName );
-			const element = await page.waitForSelector(
-				CHANGE_ALIGNMENT_BUTTON_SELECTOR
-			);
-			await element.click();
+			await clickBlockToolbarButton( 'Change alignment' );
 			await ( await page.$x( BUTTON_XPATH ) )[ 0 ].click();
 
 			// verify the button of the specified alignment is pressed.
-			await page.click( CHANGE_ALIGNMENT_BUTTON_SELECTOR );
+			await clickBlockToolbarButton( 'Change alignment' );
 			let pressedButtons = await page.$$( BUTTON_PRESSED_SELECTOR );
 			expect( pressedButtons ).toHaveLength( 1 );
 
@@ -119,11 +104,11 @@ describe( 'Align Hook Works As Expected', () => {
 			);
 
 			// remove the alignment.
-			await page.click( CHANGE_ALIGNMENT_BUTTON_SELECTOR );
+			await clickBlockToolbarButton( 'Change alignment' );
 			await ( await page.$x( BUTTON_XPATH ) )[ 0 ].click();
 
 			// verify no alignment button is in pressed state.
-			await page.click( CHANGE_ALIGNMENT_BUTTON_SELECTOR );
+			await clickBlockToolbarButton( 'Change alignment' );
 			pressedButtons = await page.$$( BUTTON_PRESSED_SELECTOR );
 			expect( pressedButtons ).toHaveLength( 0 );
 
@@ -139,7 +124,7 @@ describe( 'Align Hook Works As Expected', () => {
 			);
 
 			// verify no alignment button is in pressed state after parsing the block.
-			await page.click( CHANGE_ALIGNMENT_BUTTON_SELECTOR );
+			await clickBlockToolbarButton( 'Change alignment' );
 			pressedButtons = await page.$$( BUTTON_PRESSED_SELECTOR );
 			expect( pressedButtons ).toHaveLength( 0 );
 		} );
@@ -149,6 +134,8 @@ describe( 'Align Hook Works As Expected', () => {
 		const BLOCK_NAME = 'Test No Alignment Set';
 		it( 'Shows no alignment buttons on the alignment toolbar', async () => {
 			await insertBlock( BLOCK_NAME );
+			const CHANGE_ALIGNMENT_BUTTON_SELECTOR =
+				'.block-editor-block-toolbar .components-dropdown-menu__toggle[aria-label="Change alignment"]';
 			const changeAlignmentButton = await page.$(
 				CHANGE_ALIGNMENT_BUTTON_SELECTOR
 			);
@@ -205,10 +192,7 @@ describe( 'Align Hook Works As Expected', () => {
 		it( 'Applies the selected alignment by default', async () => {
 			await insertBlock( BLOCK_NAME );
 			// verify the correct alignment button is pressed
-			const element = await page.waitForSelector(
-				CHANGE_ALIGNMENT_BUTTON_SELECTOR
-			);
-			await element.click();
+			await clickBlockToolbarButton( 'Change alignment' );
 			const selectedAlignmentControls = await page.$x(
 				SELECTED_ALIGNMENT_CONTROL_SELECTOR
 			);
@@ -225,10 +209,7 @@ describe( 'Align Hook Works As Expected', () => {
 		it( 'Can remove the default alignment and the align attribute equals none but alignnone class is not applied', async () => {
 			await insertBlock( BLOCK_NAME );
 			// remove the alignment.
-			const element = await page.waitForSelector(
-				CHANGE_ALIGNMENT_BUTTON_SELECTOR
-			);
-			await element.click();
+			await clickBlockToolbarButton( 'Change alignment' );
 			const [ selectedAlignmentControl ] = await page.$x(
 				SELECTED_ALIGNMENT_CONTROL_SELECTOR
 			);

diff --git a/packages/e2e-tests/specs/editor/various/keyboard-navigable-blocks.test.js b/packages/e2e-tests/specs/editor/various/keyboard-navigable-blocks.test.js
@@ -23,7 +23,6 @@ const navigateToContentEditorTop = async () => {
 	// Use 'Ctrl+`' to return to the top of the editor
 	await pressKeyWithModifier( 'ctrl', '`' );
 	await pressKeyWithModifier( 'ctrl', '`' );
-	await pressKeyWithModifier( 'ctrl', '`' );
 };
 
 const tabThroughParagraphBlock = async ( paragraphText ) => {

diff --git a/packages/edit-post/src/components/header/header-toolbar/index.js b/packages/edit-post/src/components/header/header-toolbar/index.js
@@ -20,8 +20,10 @@ import {
 	__experimentalToolbarItem as ToolbarItem,
 } from '@wordpress/components';
 import { plus } from '@wordpress/icons';
+import { useRef } from '@wordpress/element';
 
 function HeaderToolbar() {
+	const inserterButton = useRef();
 	const { setIsInserterOpened } = useDispatch( 'core/edit-post' );
 	const {
 		hasFixedToolbar,
@@ -72,11 +74,22 @@ function HeaderToolbar() {
 			aria-label={ toolbarAriaLabel }
 		>
 			<ToolbarItem
+				ref={ inserterButton }
 				as={ Button }
 				className="edit-post-header-toolbar__inserter-toggle"
 				isPrimary
 				isPressed={ isInserterOpened }
-				onClick={ () => setIsInserterOpened( ! isInserterOpened ) }
+				onMouseDown={ ( event ) => {
+					event.preventDefault();
+				} }
+				onClick={ () => {
+					if ( isInserterOpened ) {
+						// Focusing the inserter button closes the inserter popover
+						inserterButton.current.focus();
+					} else {
+						setIsInserterOpened( true );
+					}
+				} }
 				disabled={ ! isInserterEnabled }
 				icon={ plus }
 				label={ _x(

diff --git a/packages/edit-post/src/components/layout/index.js b/packages/edit-post/src/components/layout/index.js
@@ -50,6 +50,7 @@ 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 = {
 	leftSidebar: __( 'Block library' ),
@@ -178,29 +179,36 @@ function Layout() {
 					leftSidebar={
 						mode === 'visual' &&
 						isInserterOpened && (
-							<div className="edit-post-layout__inserter-panel">
-								<div className="edit-post-layout__inserter-panel-header">
-									<Button
-										icon={ close }
-										onClick={ () =>
-											setIsInserterOpened( false )
-										}
-									/>
-								</div>
-								<div className="edit-post-layout__inserter-panel-content">
-									<Library
-										showMostUsedBlocks={
-											showMostUsedBlocks
-										}
-										showInserterHelpPanel
-										onSelect={ () => {
-											if ( isMobileViewport ) {
-												setIsInserterOpened( false );
+							<PopoverWrapper
+								className="edit-post-layout__inserter-panel-popover-wrapper"
+								onClose={ () => setIsInserterOpened( false ) }
+							>
+								<div className="edit-post-layout__inserter-panel">
+									<div className="edit-post-layout__inserter-panel-header">
+										<Button
+											icon={ close }
+											onClick={ () =>
+												setIsInserterOpened( false )
+											}
+										/>
+									</div>
+									<div className="edit-post-layout__inserter-panel-content">
+										<Library
+											showMostUsedBlocks={
+												showMostUsedBlocks
 											}
-										} }
-									/>
+											showInserterHelpPanel
+											onSelect={ () => {
+												if ( isMobileViewport ) {
+													setIsInserterOpened(
+														false
+													);
+												}
+											} }
+										/>
+									</div>
 								</div>
-							</div>
+							</PopoverWrapper>
 						)
 					}
 					sidebar={

diff --git a/packages/edit-post/src/components/layout/popover-wrapper.js b/packages/edit-post/src/components/layout/popover-wrapper.js
@@ -0,0 +1,57 @@
+/**
+ * WordPress dependencies
+ */
+import {
+	withConstrainedTabbing,
+	withFocusReturn,
+	withFocusOutside,
+} from '@wordpress/components';
+import { Component } from '@wordpress/element';
+import { ESCAPE } from '@wordpress/keycodes';
+
+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();
+		}
+	};
+
+	// 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 (
+		<div
+			className={ className }
+			onKeyDown={ maybeClose }
+			onMouseDown={ stopPropagation }
+		>
+			<DetectOutside onFocusOutside={ onClose }>
+				<FocusManaged>{ children }</FocusManaged>
+			</DetectOutside>
+		</div>
+	);
+	/* 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
@@ -111,6 +111,17 @@
 	background-color: $light-gray-700;
 }
 
+// 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;