Update navigation editor menu selection dropdown (#29202)

* Initial wrangling of add menu form into modal * Rename dropdown toggle * Fix focus return * Close modal on menu creation * Make separator-less menu groups possible * Rename menu option * Style primary menu item * Improve string handling in AddMenu component * Close dropdown on menu creation * Dismiss menu creation notices on re-attempt * Use focus on mount hook * Add some extra hints for screenreader users * Use destructuring * Refactor menu items into MenuSwitcher component * Use z-index file for z-index * Add e2e test * Remove wrapping function * Use separate functions for opening and closing modal
WordPress · Mar 8, 2021 · 4d4c3f0 · 4d4c3f0
1 parent a5d0ebc
commit 4d4c3f0
Show file tree

Hide file tree

Showing 14 changed files with 222 additions and 67 deletions.
diff --git a/packages/base-styles/_z-index.scss b/packages/base-styles/_z-index.scss
@@ -137,6 +137,7 @@ $z-layers: (
 	".components-popover.edit-site-more-menu__content": 99998,
 	".components-popover.block-editor-rich-text__inline-format-toolbar": 99998,
 	".components-popover.block-editor-warning__dropdown": 99998,
+	".components-popover.edit-navigation-header__menu-switcher-dropdown": 99998,
 
 	".components-autocomplete__results": 1000000,
 

diff --git a/packages/components/src/menu-group/index.js b/packages/components/src/menu-group/index.js
@@ -9,15 +9,22 @@ import classnames from 'classnames';
 import { Children } from '@wordpress/element';
 import { useInstanceId } from '@wordpress/compose';
 
-export function MenuGroup( { children, className = '', label } ) {
+export function MenuGroup( {
+	children,
+	className = '',
+	label,
+	hideSeparator,
+} ) {
 	const instanceId = useInstanceId( MenuGroup );
 
 	if ( ! Children.count( children ) ) {
 		return null;
 	}
 
 	const labelId = `components-menu-group-label-${ instanceId }`;
-	const classNames = classnames( className, 'components-menu-group' );
+	const classNames = classnames( className, 'components-menu-group', {
+		'has-hidden-separator': hideSeparator,
+	} );
 
 	return (
 		<div className={ classNames }>

diff --git a/packages/components/src/menu-group/style.scss b/packages/components/src/menu-group/style.scss
@@ -2,6 +2,12 @@
 	margin-top: $grid-unit-10;
 	padding-top: $grid-unit-10;
 	border-top: $border-width solid $gray-900;
+
+	&.has-hidden-separator {
+		border-top: none;
+		margin-top: 0;
+		padding-top: 0;
+	}
 }
 
 .components-menu-group__label {

diff --git a/packages/components/src/menu-item/style.scss b/packages/components/src/menu-item/style.scss
@@ -18,6 +18,15 @@
 		margin-left: -2px; // This optically balances the icon.
 		margin-right: $grid-unit-10;
 	}
+
+	&.is-primary {
+		justify-content: center;
+
+		.components-menu-item__item {
+			// Override the default right margin.
+			margin-right: 0;
+		}
+	}
 }
 
 .components-menu-item__info-wrapper {

diff --git a/packages/components/src/menu-items-choice/index.js b/packages/components/src/menu-items-choice/index.js
@@ -37,6 +37,7 @@ export default function MenuItemsChoice( {
 				} }
 				onMouseEnter={ () => onHover( item.value ) }
 				onMouseLeave={ () => onHover( null ) }
+				aria-label={ item[ 'aria-label' ] }
 			>
 				{ item.label }
 			</MenuItem>

diff --git a/packages/e2e-tests/specs/experiments/__snapshots__/navigation-editor.test.js.snap b/packages/e2e-tests/specs/experiments/__snapshots__/navigation-editor.test.js.snap
@@ -1,6 +1,8 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`Navigation editor allows creation of a menu 1`] = `
+exports[`Navigation editor allows creation of a menu when there are existing menu items 1`] = `"<!-- wp:navigation {\\"orientation\\":\\"vertical\\"} /-->"`;
+
+exports[`Navigation editor allows creation of a menu when there are no current menu items 1`] = `
 "<!-- wp:navigation {\\"orientation\\":\\"vertical\\"} -->
 <!-- wp:page-list /-->
 <!-- /wp:navigation -->"

diff --git a/packages/e2e-tests/specs/experiments/navigation-editor.test.js b/packages/e2e-tests/specs/experiments/navigation-editor.test.js
@@ -108,8 +108,8 @@ describe( 'Navigation editor', () => {
 		await setUpResponseMocking( [] );
 	} );
 
-	it( 'allows creation of a menu', async () => {
-		const menuResponse = {
+	it( 'allows creation of a menu when there are no current menu items', async () => {
+		const menuPostResponse = {
 			id: 4,
 			description: '',
 			name: 'Main Menu',
@@ -133,8 +133,8 @@ describe( 'Navigation editor', () => {
 		// Prepare the menu endpoint for creating a menu.
 		await setUpResponseMocking( [
 			...getMenuMocks( {
-				GET: [ menuResponse ],
-				POST: menuResponse,
+				GET: [ menuPostResponse ],
+				POST: menuPostResponse,
 			} ),
 			...getMenuItemMocks( { GET: [] } ),
 		] );
@@ -166,6 +166,69 @@ describe( 'Navigation editor', () => {
 		expect( await getSerializedBlocks() ).toMatchSnapshot();
 	} );
 
+	it( 'allows creation of a menu when there are existing menu items', async () => {
+		const menuPostResponse = {
+			id: 4,
+			description: '',
+			name: 'New Menu',
+			slug: 'new-menu',
+			meta: [],
+			auto_add: false,
+		};
+
+		await setUpResponseMocking( [
+			...getMenuMocks( {
+				GET: assignMockMenuIds( menusFixture ),
+				POST: menuPostResponse,
+			} ),
+			...getMenuItemMocks( { GET: menuItemsFixture } ),
+		] );
+		await visitNavigationEditor();
+
+		// Wait for the header to show the menu name.
+		await page.waitForXPath( '//h2[contains(., "Editing: Test Menu 1")]', {
+			visible: true,
+		} );
+
+		// Open up the menu creation dialog and create a new menu.
+		const switchMenuButton = await page.waitForXPath(
+			'//button[.="Switch menu"]'
+		);
+		await switchMenuButton.click();
+
+		const createMenuButton = await page.waitForXPath(
+			'//button[.="Create a new menu"]'
+		);
+		await createMenuButton.click();
+
+		const menuNameInputLabel = await page.waitForXPath(
+			'//form//label[.="Menu name"]'
+		);
+		await menuNameInputLabel.click();
+
+		await setUpResponseMocking( [
+			...getMenuMocks( {
+				GET: assignMockMenuIds( [
+					...menusFixture,
+					{ name: 'New menu', slug: 'new-menu' },
+				] ),
+				POST: menuPostResponse,
+			} ),
+			...getMenuItemMocks( { GET: [] } ),
+		] );
+
+		await page.keyboard.type( 'New menu' );
+		await page.keyboard.press( 'Enter' );
+
+		// A snackbar will appear when menu creation has completed.
+		await page.waitForXPath( '//div[contains(., "Menu created")]' );
+
+		// An empty navigation block will appear.
+		await page.waitForSelector( 'div[aria-label="Block: Navigation"]' );
+
+		expect( await getSerializedBlocks() ).toMatchSnapshot();
+	} );
+
 	it( 'displays the first menu from the REST response when at least one menu exists', async () => {
 		await setUpResponseMocking( [
 			...getMenuMocks( { GET: assignMockMenuIds( menusFixture ) } ),

diff --git a/packages/edit-navigation/src/components/add-menu/index.js b/packages/edit-navigation/src/components/add-menu/index.js
@@ -9,25 +9,40 @@ import classnames from 'classnames';
 import { useState } from '@wordpress/element';
 import { useDispatch } from '@wordpress/data';
 import { TextControl, Button } from '@wordpress/components';
+import { useFocusOnMount } from '@wordpress/compose';
 import { __, sprintf } from '@wordpress/i18n';
 import { store as noticesStore } from '@wordpress/notices';
 
 const menuNameMatches = ( menuName ) => ( menu ) =>
 	menu.name.toLowerCase() === menuName.toLowerCase();
 
-export default function AddMenu( { className, menus, onCreate } ) {
+export default function AddMenu( {
+	className,
+	menus,
+	onCreate,
+	titleText,
+	helpText,
+	focusInputOnMount = false,
+} ) {
 	const [ menuName, setMenuName ] = useState( '' );
-	const { createErrorNotice, createInfoNotice } = useDispatch( noticesStore );
+	const { createErrorNotice, createInfoNotice, removeNotice } = useDispatch(
+		noticesStore
+	);
 	const [ isCreatingMenu, setIsCreatingMenu ] = useState( false );
 	const { saveMenu } = useDispatch( 'core' );
 
+	const inputRef = useFocusOnMount( focusInputOnMount );
+
 	const createMenu = async ( event ) => {
 		event.preventDefault();
 
 		if ( ! menuName.length ) {
 			return;
 		}
 
+		// Remove any existing notices so duplicates aren't created.
+		removeNotice( 'edit-navigation-error' );
+
 		if ( some( menus, menuNameMatches( menuName ) ) ) {
 			const message = sprintf(
 				// translators: %s: the name of a menu.
@@ -56,27 +71,18 @@ export default function AddMenu( { className, menus, onCreate } ) {
 		setIsCreatingMenu( false );
 	};
 
-	const hasMenus = menus?.length;
-
-	const titleText = hasMenus
-		? __( 'Create a new menu' )
-		: __( 'Create your first menu' );
-
-	const helpText = hasMenus
-		? __( 'A short descriptive name for your menu.' )
-		: __( 'A short descriptive name for your first menu.' );
-
 	return (
 		<form
 			className={ classnames( 'edit-navigation-add-menu', className ) }
 			onSubmit={ createMenu }
 		>
-			<h3 className="edit-navigation-add-menu__title">{ titleText }</h3>
+			{ titleText && (
+				<h3 className="edit-navigation-add-menu__title">
+					{ titleText }
+				</h3>
+			) }
 			<TextControl
-				// Disable reason: it should focus.
-				//
-				// eslint-disable-next-line jsx-a11y/no-autofocus
-				autoFocus
+				ref={ inputRef }
 				label={ __( 'Menu name' ) }
 				value={ menuName }
 				onChange={ setMenuName }

diff --git a/packages/edit-navigation/src/components/header/index.js b/packages/edit-navigation/src/components/header/index.js
@@ -7,21 +7,14 @@ import { find } from 'lodash';
  * WordPress dependencies
  */
 import { __, sprintf } from '@wordpress/i18n';
-import {
-	Button,
-	Dropdown,
-	DropdownMenu,
-	MenuGroup,
-	MenuItemsChoice,
-	Popover,
-} from '@wordpress/components';
+import { Button, Dropdown, DropdownMenu, Popover } from '@wordpress/components';
 
 /**
  * Internal dependencies
  */
 import SaveButton from './save-button';
 import ManageLocations from './manage-locations';
-import AddMenu from '../add-menu';
+import MenuSwitcher from '../menu-switcher';
 
 export default function Header( {
 	menus,
@@ -64,49 +57,32 @@ export default function Header( {
 					<DropdownMenu
 						icon={ null }
 						toggleProps={ {
+							children: __( 'Switch menu' ),
+							'aria-label': __(
+								'Switch menu, or create a new menu'
+							),
 							showTooltip: false,
-							children: __( 'Select menu' ),
 							isTertiary: true,
 							disabled: ! menus?.length,
 							__experimentalIsFocusable: true,
 						} }
 						popoverProps={ {
+							className:
+								'edit-navigation-header__menu-switcher-dropdown',
 							position: 'bottom left',
 						} }
 					>
-						{ () => (
-							<MenuGroup>
-								<MenuItemsChoice
-									value={ selectedMenuId }
-									onSelect={ onSelectMenu }
-									choices={ menus.map( ( menu ) => ( {
-										value: menu.id,
-										label: menu.name,
-									} ) ) }
-								/>
-							</MenuGroup>
-						) }
-					</DropdownMenu>
-
-					<Dropdown
-						position="bottom left"
-						renderToggle={ ( { isOpen, onToggle } ) => (
-							<Button
-								isTertiary
-								aria-expanded={ isOpen }
-								onClick={ onToggle }
-							>
-								{ __( 'Add new' ) }
-							</Button>
-						) }
-						renderContent={ () => (
-							<AddMenu
-								className="edit-navigation-header__add-menu"
+						{ ( { onClose } ) => (
+							<MenuSwitcher
 								menus={ menus }
-								onCreate={ onSelectMenu }
+								selectedMenuId={ selectedMenuId }
+								onSelectMenu={ ( menuId ) => {
+									onSelectMenu( menuId );
+									onClose();
+								} }
 							/>
 						) }
-					/>
+					</DropdownMenu>
 
 					<Dropdown
 						contentClassName="edit-navigation-header__manage-locations"

diff --git a/packages/edit-navigation/src/components/header/style.scss b/packages/edit-navigation/src/components/header/style.scss
@@ -25,6 +25,7 @@
 	display: flex;
 }
 
-.edit-navigation-header__add-menu {
-	min-width: 220px;
+.edit-navigation-header__menu-switcher-dropdown {
+	// Appear below the modal overlay.
+	z-index: z-index(".components-popover.edit-navigation-header__menu-switcher-dropdown");
 }
diff --git a/packages/edit-navigation/src/components/layout/empty-state.js b/packages/edit-navigation/src/components/layout/empty-state.js
@@ -2,6 +2,7 @@
  * WordPress dependencies
  */
 import { Card, CardBody } from '@wordpress/components';
+import { __ } from '@wordpress/i18n';
 
 /**
  * Internal dependencies
@@ -12,7 +13,11 @@ export default function EmptyState() {
 	return (
 		<Card className="edit-navigation-empty-state">
 			<CardBody>
-				<AddMenu />
+				<AddMenu
+					titleText={ __( 'Create your first menu' ) }
+					helpText={ __( 'A short descriptive name for your menu.' ) }
+					focusInputOnMount
+				/>
 			</CardBody>
 		</Card>
 	);