Block Editor: LinkControl: Prevent focus loss in edit mode toggle (#1…

…9931) * Block Editor: LinkControl: Prevent focus loss in edit mode toggle * Block Library: Remove custom focus loss protection Previously used effect lifecycle to anticipate and respond to focus loss. Now, focus loss is prevented by LinkControl. See: 722a4d6dec * Block Editor: Rephrase and move forced input rendering comment * Block Editor: Ensure isEndingEditWithFocus always assigned as boolean
WordPress · Jan 30, 2020 · d66d53b · d66d53b
1 parent e46d52d
commit d66d53b
Show file tree

Hide file tree

Showing 7 changed files with 63 additions and 28 deletions.
diff --git a/packages/block-editor/src/components/link-control/index.js b/packages/block-editor/src/components/link-control/index.js
@@ -9,7 +9,7 @@ import { noop, startsWith } from 'lodash';
  */
 import { Button, ExternalLink, VisuallyHidden } from '@wordpress/components';
 import { __, sprintf } from '@wordpress/i18n';
-import { useCallback, useState, Fragment } from '@wordpress/element';
+import { useRef, useCallback, useState, Fragment, useEffect } from '@wordpress/element';
 import {
 	safeDecodeURI,
 	filterURLForDisplay,
@@ -19,6 +19,7 @@ import {
 } from '@wordpress/url';
 import { useInstanceId } from '@wordpress/compose';
 import { useSelect } from '@wordpress/data';
+import { focus } from '@wordpress/dom';
 
 /**
  * Internal dependencies
@@ -89,9 +90,11 @@ function LinkControl( {
 	onChange = noop,
 	showInitialSuggestions,
 } ) {
+	const wrapperNode = useRef();
 	const instanceId = useInstanceId( LinkControl );
 	const [ inputValue, setInputValue ] = useState( ( value && value.url ) || '' );
 	const [ isEditingLink, setIsEditingLink ] = useState( ! value || ! value.url );
+	const isEndingEditWithFocus = useRef( false );
 	const { fetchSearchSuggestions } = useSelect( ( select ) => {
 		const { getSettings } = select( 'core/block-editor' );
 		return {
@@ -100,6 +103,34 @@ function LinkControl( {
 	}, [] );
 	const displayURL = ( value && filterURLForDisplay( safeDecodeURI( value.url ) ) ) || '';
 
+	useEffect( () => {
+		// When `isEditingLink` is set to `false`, a focus loss could occur
+		// since the link input may be removed from the DOM. To avoid this,
+		// reinstate focus to a suitable target if focus has in-fact been lost.
+		// Note that the check is necessary because while typically unsetting
+		// edit mode would render the read-only mode's link element, it isn't
+		// guaranteed. The link input may continue to be shown if the next value
+		// is still unassigned after calling `onChange`.
+		const hadFocusLoss = (
+			isEndingEditWithFocus.current &&
+			wrapperNode.current &&
+			! wrapperNode.current.contains( document.activeElement )
+		);
+
+		if ( hadFocusLoss ) {
+			// Prefer to focus a natural focusable descendent of the wrapper,
+			// but settle for the wrapper if there are no other options.
+			const nextFocusTarget = (
+				focus.focusable.find( wrapperNode.current )[ 0 ] ||
+				wrapperNode.current
+			);
+
+			nextFocusTarget.focus();
+		}
+
+		isEndingEditWithFocus.current = false;
+	}, [ isEditingLink ] );
+
 	/**
 	 * onChange LinkControlSearchInput event handler
 	 *
@@ -156,6 +187,19 @@ function LinkControl( {
 		return couldBeURL && ! args.isInitialSuggestions ? results[ 0 ].concat( results[ 1 ] ) : results[ 0 ];
 	};
 
+	/**
+	 * Cancels editing state and marks that focus may need to be restored after
+	 * the next render, if focus was within the wrapper when editing finished.
+	 */
+	function stopEditing() {
+		isEndingEditWithFocus.current = (
+			!! wrapperNode.current &&
+			wrapperNode.current.contains( document.activeElement )
+		);
+
+		setIsEditingLink( false );
+	}
+
 	// Effects
 	const getSearchHandler = useCallback( ( val, args ) => {
 		const protocol = getProtocol( val ) || '';
@@ -198,7 +242,7 @@ function LinkControl( {
 							itemProps={ buildSuggestionItemProps( suggestion, index ) }
 							suggestion={ suggestion }
 							onClick={ () => {
-								setIsEditingLink( false );
+								stopEditing();
 								onChange( { ...value, ...suggestion } );
 							} }
 							isSelected={ index === selectedSuggestion }
@@ -212,13 +256,17 @@ function LinkControl( {
 	};
 
 	return (
-		<div className="block-editor-link-control">
+		<div
+			tabIndex={ -1 }
+			ref={ wrapperNode }
+			className="block-editor-link-control"
+		>
 			{ isEditingLink || ! value ?
 				<LinkControlSearchInput
 					value={ inputValue }
 					onChange={ onInputChange }
 					onSelect={ ( suggestion ) => {
-						setIsEditingLink( false );
+						stopEditing();
 						onChange( { ...value, ...suggestion } );
 					} }
 					renderSuggestions={ renderSearchResults }

diff --git a/packages/block-editor/src/components/link-control/test/__snapshots__/index.js.snap b/packages/block-editor/src/components/link-control/test/__snapshots__/index.js.snap
@@ -1,3 +1,3 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`Basic rendering should render 1`] = `"<div class=\\"block-editor-link-control\\"><form><div class=\\"components-base-control block-editor-url-input block-editor-link-control__search-input\\"><div class=\\"components-base-control__field\\"><input type=\\"text\\" aria-label=\\"URL\\" required=\\"\\" placeholder=\\"Search or type url\\" role=\\"combobox\\" aria-expanded=\\"false\\" aria-autocomplete=\\"list\\" aria-owns=\\"block-editor-url-input-suggestions-0\\" value=\\"\\"></div></div><button type=\\"reset\\" disabled=\\"\\" class=\\"components-button block-editor-link-control__search-reset has-icon\\" aria-label=\\"Reset\\"><svg aria-hidden=\\"true\\" role=\\"img\\" focusable=\\"false\\" xmlns=\\"http://www.w3.org/2000/svg\\" width=\\"20\\" height=\\"20\\" viewBox=\\"0 0 20 20\\" class=\\"dashicon dashicons-no-alt\\"><path d=\\"M14.95 6.46L11.41 10l3.54 3.54-1.41 1.41L10 11.42l-3.53 3.53-1.42-1.42L8.58 10 5.05 6.47l1.42-1.42L10 8.58l3.54-3.53z\\"></path></svg></button></form></div>"`;
+exports[`Basic rendering should render 1`] = `"<div tabindex=\\"-1\\" class=\\"block-editor-link-control\\"><form><div class=\\"components-base-control block-editor-url-input block-editor-link-control__search-input\\"><div class=\\"components-base-control__field\\"><input type=\\"text\\" aria-label=\\"URL\\" required=\\"\\" placeholder=\\"Search or type url\\" role=\\"combobox\\" aria-expanded=\\"false\\" aria-autocomplete=\\"list\\" aria-owns=\\"block-editor-url-input-suggestions-0\\" value=\\"\\"></div></div><button type=\\"reset\\" disabled=\\"\\" class=\\"components-button block-editor-link-control__search-reset has-icon\\" aria-label=\\"Reset\\"><svg aria-hidden=\\"true\\" role=\\"img\\" focusable=\\"false\\" xmlns=\\"http://www.w3.org/2000/svg\\" width=\\"20\\" height=\\"20\\" viewBox=\\"0 0 20 20\\" class=\\"dashicon dashicons-no-alt\\"><path d=\\"M14.95 6.46L11.41 10l3.54 3.54-1.41 1.41L10 11.42l-3.53 3.53-1.42-1.42L8.58 10 5.05 6.47l1.42-1.42L10 8.58l3.54-3.53z\\"></path></svg></button></form></div>"`;
diff --git a/packages/block-editor/src/components/link-control/test/index.js b/packages/block-editor/src/components/link-control/test/index.js
@@ -598,6 +598,9 @@ describe( 'Selecting links', () => {
 			const currentLinkHTML = currentLink.innerHTML;
 			const currentLinkAnchor = currentLink.querySelector( `[href="${ selectedLink.url }"]` );
 
+			// Make sure focus is retained after submission.
+			expect( container.contains( document.activeElement ) ).toBe( true );
+
 			expect( currentLinkHTML ).toEqual( expect.stringContaining( selectedLink.title ) );
 			expect( currentLinkHTML ).toEqual( expect.stringContaining( 'Edit' ) );
 			expect( currentLinkAnchor ).not.toBeNull();

diff --git a/packages/block-library/src/button/edit.js b/packages/block-library/src/button/edit.js
@@ -8,11 +8,7 @@ import { escape } from 'lodash';
  * WordPress dependencies
  */
 import { __ } from '@wordpress/i18n';
-import {
-	useCallback,
-	useEffect,
-	useState,
-} from '@wordpress/element';
+import { useCallback, useState } from '@wordpress/element';
 import {
 	compose,
 } from '@wordpress/compose';
@@ -85,14 +81,6 @@ function BorderPanel( { borderRadius = '', setAttributes } ) {
 
 function URLPicker( { isSelected, url, title, setAttributes, opensInNewTab, onToggleOpenInNewTab } ) {
 	const [ isURLPickerOpen, setIsURLPickerOpen ] = useState( false );
-	useEffect(
-		() => {
-			if ( ! isSelected ) {
-				setIsURLPickerOpen( false );
-			}
-		},
-		[ isSelected ]
-	);
 	const openLinkControl = () => {
 		setIsURLPickerOpen( true );
 	};

diff --git a/packages/block-library/src/navigation-link/edit.js b/packages/block-library/src/navigation-link/edit.js
@@ -68,16 +68,6 @@ function NavigationLinkEdit( {
 		}
 	}, [] );
 
-	/**
-	 * 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 ] );
-
 	return (
 		<Fragment>
 			<BlockControls>

diff --git a/packages/e2e-tests/specs/editor/blocks/buttons.test.js b/packages/e2e-tests/specs/editor/blocks/buttons.test.js
@@ -36,6 +36,9 @@ describe( 'Buttons', () => {
 		await pressKeyWithModifier( 'primary', 'k' );
 		await page.keyboard.type( 'https://www.wordpress.org/' );
 		await page.keyboard.press( 'Enter' );
+		// Make sure that the dialog is still opened, and that focus is retained
+		// within (focusing on the link preview).
+		await page.waitForSelector( ':focus.block-editor-link-control__search-item-title' );
 
 		expect( await getEditedPostContent() ).toMatchSnapshot();
 	} );

diff --git a/packages/e2e-tests/specs/editor/blocks/navigation.test.js b/packages/e2e-tests/specs/editor/blocks/navigation.test.js
@@ -57,6 +57,9 @@ async function updateActiveNavigationLink( { url, label } ) {
 		await page.keyboard.press( 'ArrowDown' );
 		// Select the suggestion.
 		await page.keyboard.press( 'Enter' );
+		// Make sure that the dialog is still opened, and that focus is retained
+		// within (focusing on the link preview).
+		await page.waitForSelector( ':focus.block-editor-link-control__search-item-title' );
 	}
 
 	if ( label ) {