Implementing new UX for invoking rich text Link UI (#57986)

* Caret on linked text does not open Link UI automatically * Click on linked text opens Link UI * Activating a link via command + k or toolbar goes to preview mode * Return focus to the element that opened the link control popover --------- Co-authored-by: Jerry Jones <[email protected]>
WordPress · Jan 25, 2024 · b85e87b · b85e87b · github-actions · Jan 25, 2024
1 parent c94f863
commit b85e87b
Show file tree

Hide file tree

Showing 3 changed files with 253 additions and 329 deletions.
diff --git a/packages/format-library/src/link/index.js b/packages/format-library/src/link/index.js
@@ -2,7 +2,7 @@
  * WordPress dependencies
  */
 import { __ } from '@wordpress/i18n';
-import { useState } from '@wordpress/element';
+import { useState, useLayoutEffect } from '@wordpress/element';
 import {
 	getTextContent,
 	applyFormat,
@@ -18,7 +18,7 @@ import {
 	RichTextShortcut,
 } from '@wordpress/block-editor';
 import { decodeEntities } from '@wordpress/html-entities';
-import { link as linkIcon, linkOff } from '@wordpress/icons';
+import { link as linkIcon } from '@wordpress/icons';
 import { speak } from '@wordpress/a11y';
 
 /**
@@ -39,34 +39,96 @@ function Edit( {
 	contentRef,
 } ) {
 	const [ addingLink, setAddingLink ] = useState( false );
+	// We only need to store the button element that opened the popover. We can ignore the other states, as they will be handled by the onFocus prop to return to the rich text field.
+	const [ openedBy, setOpenedBy ] = useState( null );
 
-	function addLink() {
+	useLayoutEffect( () => {
+		const editableContentElement = contentRef.current;
+		if ( ! editableContentElement ) {
+			return;
+		}
+
+		function handleClick( event ) {
+			// There is a situation whereby there is an existing link in the rich text
+			// and the user clicks on the leftmost edge of that link and fails to activate
+			// the link format, but the click event still fires on the `<a>` element.
+			// This causes the `addingLink` state to be set to `true` and the link UI
+			// to be rendered in "creating" mode. We need to check isActive to see if
+			// we have an active link format.
+			if ( event.target.tagName !== 'A' || ! isActive ) {
+				return;
+			}
+
+			setAddingLink( true );
+		}
+
+		editableContentElement.addEventListener( 'click', handleClick );
+
+		return () => {
+			editableContentElement.removeEventListener( 'click', handleClick );
+		};
+	}, [ contentRef, isActive ] );
+
+	function addLink( target ) {
 		const text = getTextContent( slice( value ) );
 
-		if ( text && isURL( text ) && isValidHref( text ) ) {
+		if ( ! isActive && text && isURL( text ) && isValidHref( text ) ) {
 			onChange(
 				applyFormat( value, {
 					type: name,
 					attributes: { url: text },
 				} )
 			);
-		} else if ( text && isEmail( text ) ) {
+		} else if ( ! isActive && text && isEmail( text ) ) {
 			onChange(
 				applyFormat( value, {
 					type: name,
 					attributes: { url: `mailto:${ text }` },
 				} )
 			);
 		} else {
+			if ( target ) {
+				setOpenedBy( target );
+			}
 			setAddingLink( true );
 		}
 	}
 
-	function stopAddingLink( returnFocus = true ) {
+	/**
+	 * Runs when the popover is closed via escape keypress, unlinking the selected text,
+	 * but _not_ on a click outside the popover. onFocusOutside handles that.
+	 */
+	function stopAddingLink() {
+		// Don't let the click handler on the toolbar button trigger again.
+
+		// There are two places for us to return focus to on Escape keypress:
+		// 1. The rich text field.
+		// 2. The toolbar button.
+
+		// The toolbar button is the only one we need to handle returning focus to.
+		// Otherwise, we rely on the passed in onFocus to return focus to the rich text field.
+
+		// Close the popover
 		setAddingLink( false );
-		if ( returnFocus ) {
+		// Return focus to the toolbar button or the rich text field
+		if ( openedBy?.tagName === 'BUTTON' ) {
+			openedBy.focus();
+		} else {
 			onFocus();
 		}
+		// Remove the openedBy state
+		setOpenedBy( null );
+	}
+
+	// Test for this:
+	// 1. Click on the link button
+	// 2. Click the Options button in the top right of header
+	// 3. Focus should be in the dropdown of the Options button
+	// 4. Press Escape
+	// 5. Focus should be on the Options button
+	function onFocusOutside() {
+		setAddingLink( false );
+		setOpenedBy( null );
 	}
 
 	function onRemoveFormat() {
@@ -82,36 +144,23 @@ function Edit( {
 				character="k"
 				onUse={ onRemoveFormat }
 			/>
-			{ isActive && (
-				<RichTextToolbarButton
-					name="link"
-					icon={ linkOff }
-					title={ __( 'Unlink' ) }
-					onClick={ onRemoveFormat }
-					isActive={ isActive }
-					shortcutType="primaryShift"
-					shortcutCharacter="k"
-					aria-haspopup="true"
-					aria-expanded={ addingLink || isActive }
-				/>
-			) }
-			{ ! isActive && (
-				<RichTextToolbarButton
-					name="link"
-					icon={ linkIcon }
-					title={ title }
-					onClick={ addLink }
-					isActive={ isActive }
-					shortcutType="primary"
-					shortcutCharacter="k"
-					aria-haspopup="true"
-					aria-expanded={ addingLink || isActive }
-				/>
-			) }
-			{ ( addingLink || isActive ) && (
+			<RichTextToolbarButton
+				name="link"
+				icon={ linkIcon }
+				title={ isActive ? __( 'Link' ) : title }
+				onClick={ ( event ) => {
+					addLink( event.currentTarget );
+				} }
+				isActive={ isActive || addingLink }
+				shortcutType="primary"
+				shortcutCharacter="k"
+				aria-haspopup="true"
+				aria-expanded={ addingLink }
+			/>
+			{ addingLink && (
 				<InlineLinkUI
-					addingLink={ addingLink }
 					stopAddingLink={ stopAddingLink }
+					onFocusOutside={ onFocusOutside }
 					isActive={ isActive }
 					activeAttributes={ activeAttributes }
 					value={ value }

diff --git a/packages/format-library/src/link/inline.js b/packages/format-library/src/link/inline.js
@@ -1,7 +1,7 @@
 /**
  * WordPress dependencies
  */
-import { useMemo, useRef, createInterpolateElement } from '@wordpress/element';
+import { useMemo, createInterpolateElement } from '@wordpress/element';
 import { __, sprintf } from '@wordpress/i18n';
 import { speak } from '@wordpress/a11y';
 import { Popover } from '@wordpress/components';
@@ -42,9 +42,9 @@ const LINK_SETTINGS = [
 function InlineLinkUI( {
 	isActive,
 	activeAttributes,
-	addingLink,
 	value,
 	onChange,
+	onFocusOutside,
 	stopAddingLink,
 	contentRef,
 } ) {
@@ -183,8 +183,7 @@ function InlineLinkUI( {
 		// Link UI because it should remain open for the user to modify the link they have
 		// just created.
 		if ( ! isNewLink ) {
-			const returnFocusToRichText = true;
-			stopAddingLink( returnFocusToRichText );
+			stopAddingLink();
 		}
 
 		if ( ! isValidHref( newUrl ) ) {
@@ -215,14 +214,6 @@ function InlineLinkUI( {
 	const cachedRect = useCachedTruthy( popoverAnchor.getBoundingClientRect() );
 	popoverAnchor.getBoundingClientRect = () => cachedRect;
 
-	// Focus should only be moved into the Popover when the Link is being created or edited.
-	// When the Link is in "preview" mode focus should remain on the rich text because at
-	// this point the Link dialog is informational only and thus the user should be able to
-	// continue editing the rich text.
-	// Ref used because 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 );
-
 	async function handleCreate( pageTitle ) {
 		const page = await createPageEntity( {
 			title: pageTitle,
@@ -252,9 +243,8 @@ function InlineLinkUI( {
 	return (
 		<Popover
 			anchor={ popoverAnchor }
-			focusOnMount={ focusOnMount.current }
 			onClose={ stopAddingLink }
-			onFocusOutside={ () => stopAddingLink( false ) }
+			onFocusOutside={ onFocusOutside }
 			placement="bottom"
 			offset={ 10 }
 			shift
@@ -263,7 +253,6 @@ function InlineLinkUI( {
 				value={ linkValue }
 				onChange={ onChangeLink }
 				onRemove={ removeLink }
-				forceIsEditingLink={ addingLink }
 				hasRichPreviews
 				createSuggestion={ createPageEntity && handleCreate }
 				withCreateSuggestion={ userCanCreatePages }