Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implementing new UX for invoking rich text Link UI #57986

Merged
merged 31 commits into from
Jan 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
b943258
Proof of concept
getdave Jan 18, 2024
b4bc075
Replace Unlink button in toolbar with edit link button
jeryj Jan 18, 2024
101723a
Avoid need for anchor hook
getdave Jan 19, 2024
bbde341
Activating a link via keyboard or toolbar goes to preview mode
getdave Jan 19, 2024
74a3e9e
Remove clickedLink state
jeryj Jan 22, 2024
cd51c57
Fix bug where no contentEditable would crash block
jeryj Jan 23, 2024
8fde809
Fix some e2e tests
jeryj Jan 23, 2024
b7659ef
Fix bug with collapsed selections at edge
getdave Jan 24, 2024
016a3ea
Revert unwanted stuff
getdave Jan 24, 2024
e7286f6
Revert "Revert unwanted stuff"
jeryj Jan 24, 2024
b7ba204
Revert "Fix bug with collapsed selections at edge"
jeryj Jan 24, 2024
ec501ea
Check for isActive before opening popover on link click
jeryj Jan 24, 2024
2f6b91e
Return focus to the element that opened the link control popover
jeryj Jan 24, 2024
94e332a
Use onFocusOutside to prevent stealing focus
jeryj Jan 24, 2024
9da61c7
Change name back to stopAddingLink
jeryj Jan 24, 2024
c59c5c0
Add comments
jeryj Jan 24, 2024
5091a0a
Only add link for email and urls if it's a new link
jeryj Jan 24, 2024
c006259
Update test for escape from link popover to return focus
jeryj Jan 24, 2024
76b89d3
Leave link UI open after initial creation
jeryj Jan 24, 2024
196d888
Fix createAndReselect test util
jeryj Jan 24, 2024
8ff163c
Fix keyboard shortcut tests
getdave Jan 25, 2024
c3200f2
Fix and simplify updating URL test
getdave Jan 25, 2024
21a8b12
Fix test for preserving state of advanced settings drawer
getdave Jan 25, 2024
3ff54ff
Fix test for toggling link settings
getdave Jan 25, 2024
2517f50
Fix link text change test
getdave Jan 25, 2024
7ab61d1
Update Edit Link -> Edit link
jeryj Jan 25, 2024
6b878f0
Change Edit link to Link
jeryj Jan 25, 2024
ba67044
Fix some tests
jeryj Jan 25, 2024
e75f2b1
Remove test no longer relevant
jeryj Jan 25, 2024
783a378
Remove unused test. Add test for onFocusOutside
jeryj Jan 25, 2024
abf7fcc
Hopefully fix test that is passing locally but failing on github
jeryj Jan 25, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 84 additions & 35 deletions packages/format-library/src/link/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* WordPress dependencies
*/
import { __ } from '@wordpress/i18n';
import { useState } from '@wordpress/element';
import { useState, useLayoutEffect } from '@wordpress/element';
import {
getTextContent,
applyFormat,
Expand All @@ -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';

/**
Expand All @@ -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 );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't want to add this openedBy state, as the popover has its own system for returning focus. For some reason, it's not working correctly with the Rich text element and split between iframe editor and toolbar in the header.


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() {
Expand All @@ -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 }
Expand Down
19 changes: 4 additions & 15 deletions packages/format-library/src/link/inline.js
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -42,9 +42,9 @@ const LINK_SETTINGS = [
function InlineLinkUI( {
isActive,
activeAttributes,
addingLink,
value,
onChange,
onFocusOutside,
stopAddingLink,
contentRef,
} ) {
Expand Down Expand Up @@ -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 ) ) {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -263,7 +253,6 @@ function InlineLinkUI( {
value={ linkValue }
onChange={ onChangeLink }
onRemove={ removeLink }
forceIsEditingLink={ addingLink }
hasRichPreviews
createSuggestion={ createPageEntity && handleCreate }
withCreateSuggestion={ userCanCreatePages }
Expand Down
Loading
Loading