From 0ea79967c10bde4b9dad30659e1b3c57724a9732 Mon Sep 17 00:00:00 2001
From: Riad Benguella <benguella@gmail.com>
Date: Wed, 9 Dec 2020 08:45:11 +0100
Subject: [PATCH] Remove autoFocus prop from URLInput and from the inserter
 search form (#27578)

---
 .../src/components/inserter/menu.js           |  5 --
 .../src/components/inserter/quick-inserter.js |  7 ---
 .../src/components/inserter/search-form.js    |  6 --
 .../src/components/link-control/index.js      | 16 +++---
 .../src/components/link-control/test/index.js | 56 ++-----------------
 .../src/components/url-input/README.md        |  6 --
 .../src/components/url-input/index.js         |  4 --
 .../block-library/src/navigation-link/edit.js |  2 +-
 .../specs/editor/blocks/buttons.test.js       |  8 +++
 .../src/components/layout/popover-wrapper.js  |  4 ++
 .../src/components/editor/popover-wrapper.js  |  4 ++
 .../src/components/layout/popover-wrapper.js  |  4 ++
 packages/format-library/src/link/inline.js    | 30 ++--------
 13 files changed, 41 insertions(+), 111 deletions(-)

diff --git a/packages/block-editor/src/components/inserter/menu.js b/packages/block-editor/src/components/inserter/menu.js
index 73577c5af45697..09ecba780a32d4 100644
--- a/packages/block-editor/src/components/inserter/menu.js
+++ b/packages/block-editor/src/components/inserter/menu.js
@@ -116,10 +116,6 @@ function InserterMenu( {
 		/>
 	);
 
-	// Disable reason (no-autofocus): The inserter menu is a modal display, not one which
-	// is always visible, and one which already incurs this behavior of autoFocus via
-	// Popover's focusOnMount.
-	/* eslint-disable jsx-a11y/no-autofocus */
 	return (
 		<div className="block-editor-inserter__menu">
 			<div className="block-editor-inserter__main-area">
@@ -173,7 +169,6 @@ function InserterMenu( {
 			) }
 		</div>
 	);
-	/* eslint-enable jsx-a11y/no-autofocus */
 }
 
 export default InserterMenu;
diff --git a/packages/block-editor/src/components/inserter/quick-inserter.js b/packages/block-editor/src/components/inserter/quick-inserter.js
index 9df3800d09096b..c3e2cc8699b6c3 100644
--- a/packages/block-editor/src/components/inserter/quick-inserter.js
+++ b/packages/block-editor/src/components/inserter/quick-inserter.js
@@ -80,12 +80,6 @@ export default function QuickInserter( {
 		setInserterIsOpened( true );
 	};
 
-	// Disable reason (no-autofocus): The inserter menu is a modal display, not one which
-	// is always visible, and one which already incurs this behavior of autoFocus via
-	// Popover's focusOnMount.
-	// Disable reason (no-static-element-interactions): Navigational key-presses within
-	// the menu are prevented from triggering WritingFlow and ObserveTyping interactions.
-	/* eslint-disable jsx-a11y/no-autofocus, jsx-a11y/no-static-element-interactions */
 	return (
 		<div
 			className={ classnames( 'block-editor-inserter__quick-inserter', {
@@ -129,5 +123,4 @@ export default function QuickInserter( {
 			) }
 		</div>
 	);
-	/* eslint-enable jsx-a11y/no-autofocus, jsx-a11y/no-static-element-interactions */
 }
diff --git a/packages/block-editor/src/components/inserter/search-form.js b/packages/block-editor/src/components/inserter/search-form.js
index 35d1231b8d3315..be6807b55f1f9f 100644
--- a/packages/block-editor/src/components/inserter/search-form.js
+++ b/packages/block-editor/src/components/inserter/search-form.js
@@ -16,10 +16,6 @@ function InserterSearchForm( { className, onChange, value, placeholder } ) {
 	const instanceId = useInstanceId( InserterSearchForm );
 	const searchInput = useRef();
 
-	// Disable reason (no-autofocus): The inserter menu is a modal display, not one which
-	// is always visible, and one which already incurs this behavior of autoFocus via
-	// Popover's focusOnMount.
-	/* eslint-disable jsx-a11y/no-autofocus */
 	return (
 		<div
 			className={ classnames(
@@ -39,7 +35,6 @@ function InserterSearchForm( { className, onChange, value, placeholder } ) {
 				id={ `block-editor-inserter__search-${ instanceId }` }
 				type="search"
 				placeholder={ placeholder }
-				autoFocus
 				onChange={ ( event ) => onChange( event.target.value ) }
 				autoComplete="off"
 				value={ value || '' }
@@ -59,7 +54,6 @@ function InserterSearchForm( { className, onChange, value, placeholder } ) {
 			</div>
 		</div>
 	);
-	/* eslint-enable jsx-a11y/no-autofocus */
 }
 
 export default InserterSearchForm;
diff --git a/packages/block-editor/src/components/link-control/index.js b/packages/block-editor/src/components/link-control/index.js
index 470aa862bb1190..d2bac48450bb0d 100644
--- a/packages/block-editor/src/components/link-control/index.js
+++ b/packages/block-editor/src/components/link-control/index.js
@@ -120,6 +120,7 @@ function LinkControl( {
 		withCreateSuggestion = true;
 	}
 
+	const isMounting = useRef( true );
 	const wrapperNode = useRef();
 	const [ internalInputValue, setInternalInputValue ] = useState(
 		( value && value.url ) || ''
@@ -142,19 +143,20 @@ function LinkControl( {
 	}, [ forceIsEditingLink ] );
 
 	useEffect( () => {
-		// When `isEditingLink` is set to `false`, a focus loss could occur
+		if ( isMounting.current ) {
+			isMounting.current = false;
+			return;
+		}
+		// When `isEditingLink` changes, 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(
-				wrapperNode.current.ownerDocument.activeElement
-			);
+		const hadFocusLoss = ! wrapperNode.current.contains(
+			wrapperNode.current.ownerDocument.activeElement
+		);
 
 		if ( hadFocusLoss ) {
 			// Prefer to focus a natural focusable descendent of the wrapper,
diff --git a/packages/block-editor/src/components/link-control/test/index.js b/packages/block-editor/src/components/link-control/test/index.js
index 2c1efc9352bdd0..53d04f6e023336 100644
--- a/packages/block-editor/src/components/link-control/test/index.js
+++ b/packages/block-editor/src/components/link-control/test/index.js
@@ -8,7 +8,7 @@ import { default as lodash, first, last, nth, uniqueId } from 'lodash';
 /**
  * WordPress dependencies
  */
-import { useState, useRef } from '@wordpress/element';
+import { useState } from '@wordpress/element';
 import { UP, DOWN, ENTER } from '@wordpress/keycodes';
 /**
  * Internal dependencies
@@ -594,12 +594,13 @@ describe( 'Default search suggestions', () => {
 			Simulate.click( currentLinkBtn );
 		} );
 
+		const searchInput = getURLInput();
+		searchInput.focus();
+
 		await eventLoopTick();
 
 		const searchResultElements = getSearchResults();
 
-		const searchInput = getURLInput();
-
 		// search input is set to the URL value
 		expect( searchInput.value ).toEqual( fauxEntitySuggestions[ 0 ].url );
 
@@ -1372,6 +1373,7 @@ describe( 'Selecting links', () => {
 
 				// Search Input UI
 				const searchInput = getURLInput();
+				searchInput.focus();
 				const form = container.querySelector( 'form' );
 
 				// Simulate searching for a term
@@ -1540,54 +1542,6 @@ describe( 'Selecting links', () => {
 			expect( mockFetchSearchSuggestions ).toHaveBeenCalledTimes( 1 );
 		} );
 	} );
-
-	it( 'does not forcefully regain focus if onChange handler had shifted it', () => {
-		// Regression: Previously, there had been issues where if `onChange`
-		// would programmatically shift focus, LinkControl would try to force it
-		// back, based on its internal logic to determine whether it had focus
-		// when finishing an edit occuring _before_ `onChange` having been run.
-		//
-		// See: https://github.com/WordPress/gutenberg/pull/19462
-
-		const LinkControlConsumer = () => {
-			const focusTarget = useRef();
-
-			return (
-				<>
-					<div tabIndex={ -1 } data-expected ref={ focusTarget } />
-					<LinkControl
-						onChange={ () => focusTarget.current.focus() }
-					/>
-				</>
-			);
-		};
-
-		act( () => {
-			render( <LinkControlConsumer />, container );
-		} );
-
-		// Change value.
-		const form = container.querySelector( 'form' );
-		const searchInput = getURLInput();
-
-		// Simulate searching for a term
-		act( () => {
-			Simulate.change( searchInput, {
-				target: { value: 'https://example.com' },
-			} );
-		} );
-		act( () => {
-			Simulate.keyDown( searchInput, { keyCode: ENTER } );
-		} );
-		act( () => {
-			Simulate.submit( form );
-		} );
-
-		const isExpectedFocusTarget = document.activeElement.hasAttribute(
-			'data-expected'
-		);
-		expect( isExpectedFocusTarget ).toBe( true );
-	} );
 } );
 
 describe( 'Addition Settings UI', () => {
diff --git a/packages/block-editor/src/components/url-input/README.md b/packages/block-editor/src/components/url-input/README.md
index d78a1cb09d56a8..b2837bb6dca430 100644
--- a/packages/block-editor/src/components/url-input/README.md
+++ b/packages/block-editor/src/components/url-input/README.md
@@ -129,12 +129,6 @@ Renders the URL input field used by the `URLInputButton` component. It can be us
 
 *Optional.* If this property is added, a label will be generated using label property as the content.
 
-### `autoFocus: Boolean`
-
-*Optional.* By default, the input will gain focus when it is rendered, as typically it is displayed conditionally. For example when clicking on `URLInputButton` or editing a block.
-
-If you are not conditionally rendering this component set this property to `false`.
-
 ### `className: String`
 
 *Optional.* Adds and optional class to the parent `div` that wraps the URLInput field and popover
diff --git a/packages/block-editor/src/components/url-input/index.js b/packages/block-editor/src/components/url-input/index.js
index 9f4da46d3866f0..944c9694fda8fe 100644
--- a/packages/block-editor/src/components/url-input/index.js
+++ b/packages/block-editor/src/components/url-input/index.js
@@ -22,7 +22,6 @@ import { withInstanceId, withSafeTimeout, compose } from '@wordpress/compose';
 import { withSelect } from '@wordpress/data';
 import { isURL } from '@wordpress/url';
 
-/* eslint-disable jsx-a11y/no-autofocus */
 class URLInput extends Component {
 	constructor( props ) {
 		super( props );
@@ -393,7 +392,6 @@ class URLInput extends Component {
 			placeholder = __( 'Paste URL or type to search' ),
 			__experimentalRenderControl: renderControl,
 			value = '',
-			autoFocus = true,
 		} = this.props;
 
 		const {
@@ -415,7 +413,6 @@ class URLInput extends Component {
 		const inputProps = {
 			value,
 			required: true,
-			autoFocus,
 			className: 'block-editor-url-input__input',
 			type: 'text',
 			onChange: this.onChange,
@@ -539,7 +536,6 @@ class URLInput extends Component {
 		return null;
 	}
 }
-/* eslint-enable jsx-a11y/no-autofocus */
 
 /**
  * @see https://github.com/WordPress/gutenberg/blob/master/packages/block-editor/src/components/url-input/README.md
diff --git a/packages/block-library/src/navigation-link/edit.js b/packages/block-library/src/navigation-link/edit.js
index 97658947513320..4f438723393990 100644
--- a/packages/block-library/src/navigation-link/edit.js
+++ b/packages/block-library/src/navigation-link/edit.js
@@ -170,7 +170,7 @@ function NavigationLinkEdit( {
 
 	// 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 cconflicts
+	// This can't be done in the useState call because it conflicts
 	// with the autofocus behavior of the BlockListBlock component.
 	useEffect( () => {
 		if ( ! url ) {
diff --git a/packages/e2e-tests/specs/editor/blocks/buttons.test.js b/packages/e2e-tests/specs/editor/blocks/buttons.test.js
index 3938bc2edb6c1d..9253d29b1cbd41 100644
--- a/packages/e2e-tests/specs/editor/blocks/buttons.test.js
+++ b/packages/e2e-tests/specs/editor/blocks/buttons.test.js
@@ -24,7 +24,15 @@ describe( 'Buttons', () => {
 		// Regression: https://github.com/WordPress/gutenberg/pull/19885
 		await insertBlock( 'Buttons' );
 		await pressKeyWithModifier( 'primary', 'k' );
+		await page.waitForFunction(
+			() => !! document.activeElement.closest( '.block-editor-url-input' )
+		);
 		await page.keyboard.press( 'Escape' );
+		await page.waitForFunction(
+			() =>
+				document.activeElement ===
+				document.querySelector( '.block-editor-rich-text__editable' )
+		);
 		await page.keyboard.type( 'WordPress' );
 
 		expect( await getEditedPostContent() ).toMatchSnapshot();
diff --git a/packages/edit-post/src/components/layout/popover-wrapper.js b/packages/edit-post/src/components/layout/popover-wrapper.js
index 375ba0ace1179d..7fecda0c0119da 100644
--- a/packages/edit-post/src/components/layout/popover-wrapper.js
+++ b/packages/edit-post/src/components/layout/popover-wrapper.js
@@ -8,6 +8,7 @@ import {
 } from '@wordpress/components';
 import { Component } from '@wordpress/element';
 import { ESCAPE } from '@wordpress/keycodes';
+import { useFocusOnMount } from '@wordpress/compose';
 
 function stopPropagation( event ) {
 	event.stopPropagation();
@@ -39,11 +40,14 @@ export default function PopoverWrapper( { onClose, children, className } ) {
 		}
 	};
 
+	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 (
 		<div
+			ref={ ref }
 			className={ className }
 			onKeyDown={ maybeClose }
 			onMouseDown={ stopPropagation }
diff --git a/packages/edit-site/src/components/editor/popover-wrapper.js b/packages/edit-site/src/components/editor/popover-wrapper.js
index 375ba0ace1179d..7fecda0c0119da 100644
--- a/packages/edit-site/src/components/editor/popover-wrapper.js
+++ b/packages/edit-site/src/components/editor/popover-wrapper.js
@@ -8,6 +8,7 @@ import {
 } from '@wordpress/components';
 import { Component } from '@wordpress/element';
 import { ESCAPE } from '@wordpress/keycodes';
+import { useFocusOnMount } from '@wordpress/compose';
 
 function stopPropagation( event ) {
 	event.stopPropagation();
@@ -39,11 +40,14 @@ export default function PopoverWrapper( { onClose, children, className } ) {
 		}
 	};
 
+	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 (
 		<div
+			ref={ ref }
 			className={ className }
 			onKeyDown={ maybeClose }
 			onMouseDown={ stopPropagation }
diff --git a/packages/edit-widgets/src/components/layout/popover-wrapper.js b/packages/edit-widgets/src/components/layout/popover-wrapper.js
index 375ba0ace1179d..7fecda0c0119da 100644
--- a/packages/edit-widgets/src/components/layout/popover-wrapper.js
+++ b/packages/edit-widgets/src/components/layout/popover-wrapper.js
@@ -8,6 +8,7 @@ import {
 } from '@wordpress/components';
 import { Component } from '@wordpress/element';
 import { ESCAPE } from '@wordpress/keycodes';
+import { useFocusOnMount } from '@wordpress/compose';
 
 function stopPropagation( event ) {
 	event.stopPropagation();
@@ -39,11 +40,14 @@ export default function PopoverWrapper( { onClose, children, className } ) {
 		}
 	};
 
+	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 (
 		<div
+			ref={ ref }
 			className={ className }
 			onKeyDown={ maybeClose }
 			onMouseDown={ stopPropagation }
diff --git a/packages/format-library/src/link/inline.js b/packages/format-library/src/link/inline.js
index fef83cac47a05d..4c78c9d1475a99 100644
--- a/packages/format-library/src/link/inline.js
+++ b/packages/format-library/src/link/inline.js
@@ -1,12 +1,7 @@
-/**
- * External dependencies
- */
-import { uniqueId } from 'lodash';
-
 /**
  * WordPress dependencies
  */
-import { useMemo, useState } from '@wordpress/element';
+import { useState, useRef } from '@wordpress/element';
 import { __ } from '@wordpress/i18n';
 import { withSpokenMessages, Popover } from '@wordpress/components';
 import { prependHTTP } from '@wordpress/url';
@@ -35,22 +30,6 @@ function InlineLinkUI( {
 	stopAddingLink,
 	contentRef,
 } ) {
-	/**
-	 * A unique key is generated when switching between editing and not editing
-	 * a link, based on:
-	 *
-	 * - This component may be rendered _either_ when a link is active _or_
-	 *   when adding or editing a link.
-	 * - It's only desirable to shift focus into the Popover when explicitly
-	 *   adding or editing a link, not when in the inline boundary of a link.
-	 * - Focus behavior can only be controlled on a Popover at the time it
-	 *   mounts, so a new instance of the component must be mounted to
-	 *   programmatically enact the focusOnMount behavior.
-	 *
-	 * @type {string}
-	 */
-	const mountingKey = useMemo( uniqueId, [ addingLink ] );
-
 	/**
 	 * Pending settings to be applied to the next link. When inserting a new
 	 * link, toggle values cannot be applied immediately, because there is not
@@ -146,11 +125,14 @@ function InlineLinkUI( {
 
 	const anchorRef = useAnchorRef( { ref: contentRef, value, settings } );
 
+	// The focusOnMount prop shouldn't evolve during render of a Popover
+	// otherwise it causes a render of the content.
+	const focusOnMount = useRef( addingLink ? 'firstElement' : false );
+
 	return (
 		<Popover
-			key={ mountingKey }
 			anchorRef={ anchorRef }
-			focusOnMount={ addingLink ? 'firstElement' : false }
+			focusOnMount={ focusOnMount.current }
 			onClose={ stopAddingLink }
 			position="bottom center"
 		>