From 06bd2662c8fafacfbd6832aa6017b91915375445 Mon Sep 17 00:00:00 2001
From: Jerry Jones <jones.jeremydavid@gmail.com>
Date: Fri, 31 Jan 2020 15:03:27 -0800
Subject: [PATCH] Navigation: Change UX to move focus to navigation link label
 and close the LinkControl (#19686)

* When adding a link via the LinkControl, selects/highlights nav link label text if it's url-like. Focuses if not. Automatically adds url-like labels as the label.

* Adds @wordpress/dom to package-lock.json

* Removed test for awaiting for Link Control focus after pressing Enter, as the focus should now be on the navigation link text with the Link Control closed
---
 package-lock.json                             |  2 +
 packages/block-library/package.json           |  2 +
 .../block-library/src/navigation-link/edit.js | 55 ++++++++++++++++++-
 .../specs/editor/blocks/navigation.test.js    | 28 +++++-----
 4 files changed, 69 insertions(+), 18 deletions(-)

diff --git a/package-lock.json b/package-lock.json
index d4562580e2b47..1d5a493bf8e4e 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -10115,6 +10115,8 @@
 				"@wordpress/data": "file:packages/data",
 				"@wordpress/date": "file:packages/date",
 				"@wordpress/deprecated": "file:packages/deprecated",
+				"@wordpress/dom": "file:packages/dom",
+				"@wordpress/editor": "file:packages/editor",
 				"@wordpress/element": "file:packages/element",
 				"@wordpress/escape-html": "file:packages/escape-html",
 				"@wordpress/i18n": "file:packages/i18n",
diff --git a/packages/block-library/package.json b/packages/block-library/package.json
index fe1bb28e1f96e..1a99a38df771f 100644
--- a/packages/block-library/package.json
+++ b/packages/block-library/package.json
@@ -37,6 +37,8 @@
 		"@wordpress/data": "file:../data",
 		"@wordpress/date": "file:../date",
 		"@wordpress/deprecated": "file:../deprecated",
+		"@wordpress/dom": "file:../dom",
+		"@wordpress/editor": "file:../editor",
 		"@wordpress/element": "file:../element",
 		"@wordpress/escape-html": "file:../escape-html",
 		"@wordpress/i18n": "file:../i18n",
diff --git a/packages/block-library/src/navigation-link/edit.js b/packages/block-library/src/navigation-link/edit.js
index d953ca8efeada..ff55c5dabfecd 100644
--- a/packages/block-library/src/navigation-link/edit.js
+++ b/packages/block-library/src/navigation-link/edit.js
@@ -30,8 +30,9 @@ import {
 	RichText,
 	__experimentalLinkControl as LinkControl,
 } from '@wordpress/block-editor';
-import { Fragment, useState, useEffect } from '@wordpress/element';
-
+import { isURL, prependHTTP } from '@wordpress/url';
+import { Fragment, useState, useEffect, useRef } from '@wordpress/element';
+import { placeCaretAtHorizontalEdge } from '@wordpress/dom';
 /**
  * Internal dependencies
  */
@@ -61,6 +62,7 @@ function NavigationLinkEdit( {
 	};
 	const [ isLinkOpen, setIsLinkOpen ] = useState( false );
 	const itemLabelPlaceholder = __( 'Add link…' );
+	const ref = useRef();
 
 	// Show the LinkControl on mount if the URL is empty
 	// ( When adding a new menu item)
@@ -72,6 +74,49 @@ 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 ] );
+
+	// If the LinkControl popover is open and the URL has changed, close the LinkControl and focus the label text.
+	useEffect( () => {
+		if ( isLinkOpen && url ) {
+			// Close the link.
+			setIsLinkOpen( false );
+
+			// Does this look like a URL and have something TLD-ish?
+			if (
+				isURL( prependHTTP( label ) ) &&
+				/^.+\.[a-z]+/.test( label )
+			) {
+				// Focus and select the label text.
+				selectLabelText();
+			} else {
+				// Focus it (but do not select).
+				placeCaretAtHorizontalEdge( ref.current, true );
+			}
+		}
+	}, [ url ] );
+
+	/**
+	 * Focus the navigation link label text and select it.
+	 */
+	function selectLabelText() {
+		ref.current.focus();
+		const selection = window.getSelection();
+		const range = document.createRange();
+		// Get the range of the current ref contents so we can add this range to the selection.
+		range.selectNodeContents( ref.current );
+		selection.removeAllRanges();
+		selection.addRange( range );
+	}
+
 	return (
 		<Fragment>
 			<BlockControls>
@@ -155,6 +200,7 @@ function NavigationLinkEdit( {
 			>
 				<div className="wp-block-navigation-link__content">
 					<RichText
+						ref={ ref }
 						tagName="span"
 						className="wp-block-navigation-link__label"
 						value={ label }
@@ -209,8 +255,11 @@ function NavigationLinkEdit( {
 												label !== newTitle
 											) {
 												return escape( newTitle );
+											} else if ( label ) {
+												return label;
 											}
-											return label;
+											// If there's no label, add the URL.
+											return escape( normalizedURL );
 										} )(),
 										opensInNewTab: newOpensInNewTab,
 										id,
diff --git a/packages/e2e-tests/specs/editor/blocks/navigation.test.js b/packages/e2e-tests/specs/editor/blocks/navigation.test.js
index 8a61834ac3b3c..fd3ab2202f636 100644
--- a/packages/e2e-tests/specs/editor/blocks/navigation.test.js
+++ b/packages/e2e-tests/specs/editor/blocks/navigation.test.js
@@ -6,9 +6,9 @@ import {
 	createNewPost,
 	getEditedPostContent,
 	insertBlock,
-	pressKeyWithModifier,
 	setUpResponseMocking,
 	clickBlockToolbarButton,
+	pressKeyWithModifier,
 } from '@wordpress/e2e-test-utils';
 
 async function mockPagesResponse( pages ) {
@@ -65,23 +65,21 @@ 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 ) {
-		await page.click( '.is-selected .wp-block-navigation-link__label' );
-
-		// Ideally this would be `await pressKeyWithModifier( 'primary', 'a' )`
-		// to select all text like other tests do.
-		// Unfortunately, these tests don't seem to pass on Travis CI when
-		// using that approach, while using `Home` and `End` they do pass.
-		await page.keyboard.press( 'Home' );
-		await pressKeyWithModifier( 'shift', 'End' );
-		await page.keyboard.press( 'Backspace' );
+		// With https://github.com/WordPress/gutenberg/pull/19686, we're auto-selecting the label if the label is URL-ish.
+		// In this case, it means we have to select and delete the label if it's _not_ the url.
+		if ( label !== url ) {
+			// Ideally this would be `await pressKeyWithModifier( 'primary', 'a' )`
+			// to select all text like other tests do.
+			// Unfortunately, these tests don't seem to pass on Travis CI when
+			// using that approach, while using `Home` and `End` they do pass.
+			await page.keyboard.press( 'Home' );
+			await pressKeyWithModifier( 'shift', 'End' );
+			await page.keyboard.press( 'Backspace' );
+		}
+
 		await page.keyboard.type( label );
 	}
 }