Obviate mousetrap around Navigation Link popover (#61050)

* Remove mousetrap overlay from NavigationLink’s popover * Remove shouldn’t-be-necessary effect * Update “create a submenu” e2e test * Show link edit ui only if block is selected
WordPress · Apr 26, 2024 · 40395f9 · 40395f9
1 parent 0b185ea
commit 40395f9
Show file tree

Hide file tree

Showing 4 changed files with 85 additions and 126 deletions.
diff --git a/packages/block-library/src/navigation-link/edit.js b/packages/block-library/src/navigation-link/edit.js
@@ -247,9 +247,11 @@ export default function NavigationLinkEdit( {
 	const {
 		replaceBlock,
 		__unstableMarkNextChangeAsNotPersistent,
+		selectBlock,
 		selectPreviousBlock,
 	} = useDispatch( blockEditorStore );
-	const [ isLinkOpen, setIsLinkOpen ] = useState( false );
+	// Have the link editing ui open on mount when lacking a url and selected.
+	const [ isLinkOpen, setIsLinkOpen ] = useState( isSelected && ! url );
 	// Store what element opened the popover, so we know where to return focus to (toolbar button vs navigation link text)
 	const [ openedBy, setOpenedBy ] = useState( null );
 	// Use internal state instead of a ref to make sure that the component
@@ -304,27 +306,19 @@ export default function NavigationLinkEdit( {
 	 * Transform to submenu block.
 	 */
 	const transformToSubmenu = () => {
-		const innerBlocks = getBlocks( clientId );
+		let innerBlocks = getBlocks( clientId );
+		if ( innerBlocks.length === 0 ) {
+			innerBlocks = [ createBlock( 'core/navigation-link' ) ];
+			selectBlock( innerBlocks[ 0 ].clientId );
+		}
 		const newSubmenu = createBlock(
 			'core/navigation-submenu',
 			attributes,
-			innerBlocks.length > 0
-				? innerBlocks
-				: [ createBlock( 'core/navigation-link' ) ]
+			innerBlocks
 		);
 		replaceBlock( clientId, newSubmenu );
 	};
 
-	useEffect( () => {
-		// Show the LinkControl on mount if the URL is empty
-		// ( When adding a new menu item)
-		// This can't be done in the useState call because it conflicts
-		// with the autofocus behavior of the BlockListBlock component.
-		if ( ! url ) {
-			setIsLinkOpen( true );
-		}
-	}, [ url ] );
-
 	useEffect( () => {
 		// If block has inner blocks, transform to Submenu.
 		if ( hasChildren ) {
@@ -335,16 +329,6 @@ export default function NavigationLinkEdit( {
 		}
 	}, [ hasChildren ] );
 
-	/**
-	 * The hook shouldn't be necessary but due to a focus loss happening
-	 * when selecting a suggestion in the link popover, we force close on block unselection.
-	 */
-	useEffect( () => {
-		if ( ! isSelected ) {
-			setIsLinkOpen( false );
-		}
-	}, [ isSelected ] );
-
 	// If the LinkControl popover is open and the URL has changed, close the LinkControl and focus the label text.
 	useEffect( () => {
 		// We only want to do this when the URL has gone from nothing to a new URL AND the label looks like a URL

diff --git a/packages/block-library/src/navigation-link/link-ui.js b/packages/block-library/src/navigation-link/link-ui.js
@@ -148,7 +148,6 @@ function LinkUIBlockInserter( { clientId, onBack, onSelectBlock } ) {
 export function LinkUI( props ) {
 	const [ addingBlock, setAddingBlock ] = useState( false );
 	const [ focusAddBlockButton, setFocusAddBlockButton ] = useState( false );
-	const [ showBackdrop, setShowBackdrop ] = useState( true );
 	const { saveEntityRecord } = useDispatch( coreStore );
 	const pagesPermissions = useResourcePermissions( 'pages' );
 	const postsPermissions = useResourcePermissions( 'posts' );
@@ -214,102 +213,88 @@ export function LinkUI( props ) {
 	const { onClose: onSelectBlock } = props;
 
 	return (
-		<>
-			{ showBackdrop && (
+		<Popover
+			placement="bottom"
+			onClose={ props.onClose }
+			anchor={ props.anchor }
+			shift
+		>
+			{ ! addingBlock && (
 				<div
-					className="components-popover-pointer-events-trap"
-					aria-hidden="true"
-					onClick={ () => setShowBackdrop( false ) }
-				/>
-			) }
-			<Popover
-				placement="bottom"
-				onClose={ props.onClose }
-				anchor={ props.anchor }
-				shift
-			>
-				{ ! addingBlock && (
-					<div
-						role="dialog"
-						aria-labelledby={ dialogTitleId }
-						aria-describedby={ dialogDescritionId }
-					>
-						<VisuallyHidden>
-							<h2 id={ dialogTitleId }>{ __( 'Add link' ) }</h2>
+					role="dialog"
+					aria-labelledby={ dialogTitleId }
+					aria-describedby={ dialogDescritionId }
+				>
+					<VisuallyHidden>
+						<h2 id={ dialogTitleId }>{ __( 'Add link' ) }</h2>
 
-							<p id={ dialogDescritionId }>
-								{ __(
-									'Search for and add a link to your Navigation.'
-								) }
-							</p>
-						</VisuallyHidden>
-						<LinkControl
-							hasTextControl
-							hasRichPreviews
-							value={ link }
-							showInitialSuggestions
-							withCreateSuggestion={ userCanCreate }
-							createSuggestion={ handleCreate }
-							createSuggestionButtonText={ ( searchTerm ) => {
-								let format;
-
-								if ( type === 'post' ) {
-									/* translators: %s: search term. */
-									format = __(
-										'Create draft post: <mark>%s</mark>'
-									);
-								} else {
-									/* translators: %s: search term. */
-									format = __(
-										'Create draft page: <mark>%s</mark>'
-									);
-								}
+						<p id={ dialogDescritionId }>
+							{ __(
+								'Search for and add a link to your Navigation.'
+							) }
+						</p>
+					</VisuallyHidden>
+					<LinkControl
+						hasTextControl
+						hasRichPreviews
+						value={ link }
+						showInitialSuggestions
+						withCreateSuggestion={ userCanCreate }
+						createSuggestion={ handleCreate }
+						createSuggestionButtonText={ ( searchTerm ) => {
+							let format;
 
-								return createInterpolateElement(
-									sprintf( format, searchTerm ),
-									{
-										mark: <mark />,
-									}
+							if ( type === 'post' ) {
+								/* translators: %s: search term. */
+								format = __(
+									'Create draft post: <mark>%s</mark>'
+								);
+							} else {
+								/* translators: %s: search term. */
+								format = __(
+									'Create draft page: <mark>%s</mark>'
 								);
-							} }
-							noDirectEntry={ !! type }
-							noURLSuggestion={ !! type }
-							suggestionsQuery={ getSuggestionsQuery(
-								type,
-								kind
-							) }
-							onChange={ props.onChange }
-							onRemove={ props.onRemove }
-							onCancel={ props.onCancel }
-							renderControlBottom={ () =>
-								! link?.url?.length && (
-									<LinkUITools
-										focusAddBlockButton={
-											focusAddBlockButton
-										}
-										setAddingBlock={ () => {
-											setAddingBlock( true );
-											setFocusAddBlockButton( false );
-										} }
-									/>
-								)
 							}
-						/>
-					</div>
-				) }
 
-				{ addingBlock && (
-					<LinkUIBlockInserter
-						clientId={ props.clientId }
-						onBack={ () => {
-							setAddingBlock( false );
-							setFocusAddBlockButton( true );
+							return createInterpolateElement(
+								sprintf( format, searchTerm ),
+								{
+									mark: <mark />,
+								}
+							);
 						} }
-						onSelectBlock={ onSelectBlock }
+						noDirectEntry={ !! type }
+						noURLSuggestion={ !! type }
+						suggestionsQuery={ getSuggestionsQuery( type, kind ) }
+						onChange={ props.onChange }
+						onRemove={ props.onRemove }
+						onCancel={ props.onCancel }
+						renderControlBottom={ () =>
+							! link?.url?.length && (
+								<LinkUITools
+									focusAddBlockButton={ focusAddBlockButton }
+									setAddingBlock={ () => {
+										setAddingBlock( true );
+										setFocusAddBlockButton( false );
+									} }
+								/>
+							)
+						}
 					/>
-				) }
-			</Popover>
-		</>
+				</div>
+			) }
+
+			{ addingBlock && (
+				<LinkUIBlockInserter
+					clientId={ props.clientId }
+					onBack={ () => {
+						setAddingBlock( false );
+						setFocusAddBlockButton( true );
+					} }
+					onSelectBlock={ onSelectBlock }
+				/>
+			) }
+		</Popover>
 	);
 }
 

diff --git a/packages/block-library/src/navigation-link/style.scss b/packages/block-library/src/navigation-link/style.scss
@@ -28,15 +28,3 @@
 	margin-left: $grid-unit-10;
 	text-transform: uppercase;
 }
-
-// Ensure Popover `onClose` is fired for clicks on the canvas iframe.
-// See https: //github.com/WordPress/gutenberg/pull/57756#discussion_r1475852009.
-.components-popover-pointer-events-trap {
-	// Same z-index as popover, but rendered before the popover element
-	// in DOM order = it will display just under the popover
-	z-index: z-index(".components-popover");
-	position: fixed;
-	inset: 0;
-	background-color: transparent;
-	cursor: pointer;
-}
diff --git a/test/e2e/specs/editor/blocks/navigation.spec.js b/test/e2e/specs/editor/blocks/navigation.spec.js
@@ -226,6 +226,8 @@ test.describe( 'Navigation block', () => {
 				name: 'Add submenu',
 			} );
 			await addSubmenuButton.click();
+			await page.keyboard.type( '#yup' );
+			await page.keyboard.press( 'Enter' );
 
 			const postId = await editor.publishPost();
 			await page.goto( `/?p=${ postId }` );