Block Editor: Change default URLInput autoFocus to false

packages/block-editor/CHANGELOG.md

@@ -1,5 +1,9 @@
 ## Master
 
+### Breaking Changes
+
+- The default `URLInput` `autoFocus` prop value has changed from `true` to `false`. If you relied on the auto-focus behavior of the input, you must explicitly assign an `autoFocus={ true }` prop. Note that (in accordance with the purpose of this change) it is usually inadvisable to auto-focus an input. Refer to the [component README](https://github.com/WordPress/gutenberg/blob/master/packages/block-editor/src/components/url-input/README.md) for more information.
+
 ## 3.7.0 (2020-02-10)
 
 ### New Features

packages/block-editor/src/components/link-control/index.js

@@ -179,7 +179,7 @@ function LinkControl( {
 	);
 	const [ isResolvingLink, setIsResolvingLink ] = useState( false );
 	const [ errorMessage, setErrorMessage ] = useState( null );
-	const isEndingEditWithFocus = useRef( false );
+	const isTogglingEditingWithFocusIntent = useRef( false );
 
 	const { fetchSearchSuggestions } = useSelect( ( select ) => {
 		const { getSettings } = select( 'core/block-editor' );
@@ -209,7 +209,7 @@ function LinkControl( {
 		// guaranteed. The link input may continue to be shown if the next value
 		// is still unassigned after calling `onChange`.
 		const hadFocusLoss =
-			isEndingEditWithFocus.current &&
+			isTogglingEditingWithFocusIntent.current &&
 			wrapperNode.current &&
 			! wrapperNode.current.contains( document.activeElement );
 
@@ -223,7 +223,7 @@ function LinkControl( {
 			nextFocusTarget.focus();
 		}
 
-		isEndingEditWithFocus.current = false;
+		isTogglingEditingWithFocusIntent.current = false;
 	}, [ isEditingLink ] );
 
 	/**
@@ -241,6 +241,40 @@ function LinkControl( {
 		};
 	}, [] );
 
+	/**
+	 * Sets editing state and marks that focus may need to be restored after
+	 * the next render, using an optional explicit `setFocus` argument. Normally
+	 * it should not be necessary to call this function, and instead rely on the
+	 * behavior of `setIsEditingLinkWithRetainedFocus`. However, in cases where
+	 * editing state is toggled in response to button clicks, this function
+	 * normalizes browser inconsistencies where focus may not be assigned,
+	 * considering any button click as effectively having focus.
+	 *
+	 * @see https://developer.mozilla.org/en-US/docs/Web/HTML/Element/button#Clicking_and_focus
+	 *
+	 * @param {boolean} nextIsEditingLink State toggle value to set.
+	 * @param {boolean} [setFocus=true]   Whether focus should be restored.
+	 */
+	function setIsEditingLinkWithFocus( nextIsEditingLink, setFocus = true ) {
+		isTogglingEditingWithFocusIntent.current = setFocus;
+
+		setIsEditingLink( nextIsEditingLink );
+	}
+
+	/**
+	 * Sets editing state and marks that focus may need to be restored after
+	 * the next render, if focus was within the wrapper while toggled.
+	 *
+	 * @param {boolean} nextIsEditingLink State toggle value to set.
+	 */
+	function setIsEditingLinkWithRetainedFocus( nextIsEditingLink ) {
+		const setFocus =
+			!! wrapperNode.current &&
+			wrapperNode.current.contains( document.activeElement );
+
+		setIsEditingLinkWithFocus( nextIsEditingLink, setFocus );
+	}
+
 	/**
 	 * onChange LinkControlSearchInput event handler
 	 *
@@ -321,18 +355,6 @@ function LinkControl( {
 			  } );
 	};
 
-	/**
-	 * 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 );
-	}
-
 	/**
 	 * Determines whether a given value could be a URL. Note this does not
 	 * guarantee the value is a URL only that it looks like it might be one. For
@@ -386,9 +408,9 @@ function LinkControl( {
 			// Only set link if request is resolved, otherwise enable edit mode.
 			if ( newSuggestion ) {
 				onChange( newSuggestion );
-				stopEditing();
+				setIsEditingLinkWithRetainedFocus( false );
 			} else {
-				setIsEditingLink( true );
+				setIsEditingLinkWithRetainedFocus( true );
 			}
 		} catch ( error ) {
 			if ( error && error.isCanceled ) {
@@ -402,12 +424,12 @@ function LinkControl( {
 					)
 			);
 			setIsResolvingLink( false );
-			setIsEditingLink( true );
+			setIsEditingLinkWithRetainedFocus( true );
 		}
 	};
 
 	const handleSelectSuggestion = ( suggestion, _value = {} ) => {
-		setIsEditingLink( false );
+		setIsEditingLinkWithRetainedFocus( false );
 		onChange( { ..._value, ...suggestion } );
 	};
 
@@ -514,7 +536,7 @@ function LinkControl( {
 								suggestion={ suggestion }
 								index={ index }
 								onClick={ () => {
-									stopEditing();
+									setIsEditingLinkWithFocus( false );
 									onChange( { ...value, ...suggestion } );
 								} }
 								isSelected={ index === selectedSuggestion }
@@ -555,7 +577,7 @@ function LinkControl( {
 							await handleOnCreate( inputValue );
 						} else {
 							handleSelectSuggestion( suggestion, value );
-							stopEditing();
+							setIsEditingLinkWithRetainedFocus( false );
 						}
 					} }
 					renderSuggestions={
@@ -595,7 +617,7 @@ function LinkControl( {
 
 						<Button
 							isSecondary
-							onClick={ () => setIsEditingLink( true ) }
+							onClick={ () => setIsEditingLinkWithFocus( true ) }
 							className="block-editor-link-control__search-item-action"
 						>
 							{ __( 'Edit' ) }

packages/block-editor/src/components/link-control/test/index.js

@@ -35,6 +35,28 @@ function eventLoopTick() {
 
 let container = null;
 
+beforeAll( () => {
+	// This is necessary because the implementation of `@wordpress/dom` focus
+	// utilities rely on CSSOM layout properties in order to detect an element
+	// as being focusable. These are not implemented by JSDOM, so are stubbed
+	// here instead. Ideally, this is either implemented or stubbed in JSDOM,
+	// mocked globally, or the implementation of focus is revised to not depend
+	// on these properties.
+	//
+	// See: https://github.com/jsdom/jsdom/issues/135
+	Object.defineProperties( window.HTMLElement.prototype, {
+		getClientRects: {
+			get: () => [ new window.DOMRect() ],
+		},
+		offsetHeight: {
+			get: () => 1,
+		},
+		offsetWidth: {
+			get: () => 1,
+		},
+	} );
+} );
+
 beforeEach( () => {
 	// setup a DOM element as a render target
 	container = document.createElement( 'div' );
@@ -1211,6 +1233,11 @@ describe( 'Selecting links', () => {
 				const searchInput = getURLInput();
 				const form = container.querySelector( 'form' );
 
+				// Focus the input via DOM. The simulated events below don't operate
+				// on the DOM, but for LinkControl's focus preservation to work (and
+				// to verify it below), focus must be set in the real DOM.
+				searchInput.focus();
+
 				// Simulate searching for a term
 				act( () => {
 					Simulate.change( searchInput, {

packages/block-editor/src/components/url-input/README.md

@@ -131,9 +131,11 @@ Renders the URL input field used by the `URLInputButton` component. It can be us
 
 ### `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.
+*Optional.* Whether the input should receive focus as soon as it is rendered. Defaults to `false`.
 
-If you are not conditionally rendering this component set this property to `false`.
+Note that it is usually inadvisable to use `autoFocus`, since it can be a disorienting experience for keyboard usage and for users of assistive technology. It should typically only be used when the input is displayed temporarily and it is the intended user experience that the input should be focused immediately upon being shown (e.g. showing the input only after activating a toggle button).
+
+If you are using `URLInput` in the context of a dialog, you should consider instead to use the `focusOnMount` prop of the [`Popover`](https://github.com/WordPress/gutenberg/blob/master/packages/components/src/popover/README.md#focusonmount) or [`Modal`](https://github.com/WordPress/gutenberg/blob/master/packages/components/src/modal/README.md#focusonmount) components instead, as they achieve the same effective behavior.
 
 ### `className: String`
 

packages/block-editor/src/components/url-input/button.js

@@ -35,6 +35,11 @@ class URLInputButton extends Component {
 		const { expanded } = this.state;
 		const buttonLabel = url ? __( 'Edit link' ) : __( 'Insert link' );
 
+		// Disable reason: The rendered URLInput is toggled in response to a
+		// click on the button, and it is expected that toggling it to be
+		// visible should shift focus from the button into the input.
+
+		/* eslint-disable jsx-a11y/no-autofocus */
 		return (
 			<div className="block-editor-url-input__button">
 				<Button
@@ -57,6 +62,7 @@ class URLInputButton extends Component {
 								onClick={ this.toggle }
 							/>
 							<URLInput
+								autoFocus
 								value={ url || '' }
 								onChange={ onChange }
 							/>
@@ -70,6 +76,7 @@ class URLInputButton extends Component {
 				) }
 			</div>
 		);
+		/* eslint-enable jsx-a11y/no-autofocus */
 	}
 }
 

packages/block-editor/src/components/url-input/index.js

@@ -383,7 +383,7 @@ class URLInput extends Component {
 			__experimentalRenderSuggestions: renderSuggestions,
 			placeholder = __( 'Paste URL or type to search' ),
 			value = '',
-			autoFocus = true,
+			autoFocus = false,
 			__experimentalShowInitialSuggestions = false,
 		} = this.props;