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

Keep Link UI open upon initial link creation when used in RichText #57726

Merged
merged 17 commits into from
Jan 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
42 changes: 20 additions & 22 deletions packages/format-library/src/link/inline.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,17 @@ import {
insert,
isCollapsed,
applyFormat,
useAnchor,
removeFormat,
slice,
replace,
split,
concat,
useAnchor,
} from '@wordpress/rich-text';
import {
__experimentalLinkControl as LinkControl,
store as blockEditorStore,
useCachedTruthy,
} from '@wordpress/block-editor';
import { useSelect } from '@wordpress/data';

Expand All @@ -29,7 +30,6 @@ import { useSelect } from '@wordpress/data';
*/
import { createLinkFormat, isValidHref, getFormatBoundary } from './utils';
import { link as settings } from './index';
import useLinkInstanceKey from './use-link-instance-key';

const LINK_SETTINGS = [
...LinkControl.DEFAULT_LINK_SETTINGS,
Expand Down Expand Up @@ -90,12 +90,9 @@ function InlineLinkUI( {
}

function onChangeLink( nextValue ) {
// LinkControl calls `onChange` immediately upon the toggling a setting.
// Before merging the next value with the current link value, check if
// the setting was toggled.
const didToggleSetting =
linkValue.opensInNewTab !== nextValue.opensInNewTab &&
nextValue.url === undefined;
const hasLink = linkValue?.url;
const isNewLink = ! hasLink;

// Merge the next value with the current link value.
nextValue = {
...linkValue,
Expand Down Expand Up @@ -178,17 +175,16 @@ function InlineLinkUI( {
newValue = concat( valBefore, newValAfter );
}

newValue.start = newValue.end;
getdave marked this conversation as resolved.
Show resolved Hide resolved

// Hides the Link UI.
newValue.activeFormats = [];
onChange( newValue );
}

// Focus should only be shifted back to the formatted segment when the
// URL is submitted.
if ( ! didToggleSetting ) {
stopAddingLink();
// Focus should only be returned to the rich text on submit if this link is not
// being created for the first time. If it is then focus should remain within the
// 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 );
}

if ( ! isValidHref( newUrl ) ) {
Expand All @@ -210,11 +206,14 @@ function InlineLinkUI( {
settings,
} );

// Generate a string based key that is unique to this anchor reference.
// This is used to force re-mount the LinkControl component to avoid
// potential stale state bugs caused by the component not being remounted
// See https://github.com/WordPress/gutenberg/pull/34742.
const forceRemountKey = useLinkInstanceKey( popoverAnchor );
// As you change the link by interacting with the Link UI
// the return value of document.getSelection jumps to the field you're editing,
// not the highlighted text. Given that useAnchor uses document.getSelection,
// it will return null, since it can't find the <mark> element within the Link UI.
// This caches the last truthy value of the selection anchor reference.
// This ensures the Popover is positioned correctly on initial submission of the link.
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
Expand Down Expand Up @@ -260,7 +259,6 @@ function InlineLinkUI( {
shift
>
<LinkControl
key={ forceRemountKey }
value={ linkValue }
onChange={ onChangeLink }
onRemove={ removeLink }
Expand Down
44 changes: 28 additions & 16 deletions test/e2e/specs/editor/blocks/links.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,9 @@ test.describe( 'Links', () => {
await pageUtils.pressKeys( 'Enter' );

const linkPopover = LinkUtils.getLinkPopover();
await expect( linkPopover ).toBeVisible();
// Close the link control to return the caret to the canvas
await pageUtils.pressKeys( 'Escape' );

// Deselect the link text by moving the caret to the end of the line
// and the link popover should not be displayed.
Expand Down Expand Up @@ -612,6 +615,13 @@ test.describe( 'Links', () => {
await page.keyboard.type( 'w.org' );

await page.keyboard.press( 'Enter' );
// Close the link control to return the caret to the canvas
const linkPopover = LinkUtils.getLinkPopover();
await pageUtils.pressKeys( 'Escape' );
// Deselect the link text by moving the caret to the end of the line
// and the link popover should not be displayed.
await pageUtils.pressKeys( 'End' );
await expect( linkPopover ).toBeHidden();

await expect.poll( editor.getBlocks ).toMatchObject( [
{
Expand All @@ -634,17 +644,12 @@ test.describe( 'Links', () => {
await page.getByPlaceholder( 'Search or type url' ).fill( '' );
await page.keyboard.type( 'wordpress.org' );

const linkPopover = LinkUtils.getLinkPopover();

// Update the link.
await linkPopover.getByRole( 'button', { name: 'Save' } ).click();

// Navigate back to the popover.
await page.keyboard.press( 'ArrowLeft' );
await page.keyboard.press( 'ArrowLeft' );

// Navigate back to inputs to verify appears as changed.
await pageUtils.pressKeys( 'primary+k' );
// Navigate back to the link editing state inputs to verify appears as changed.
await page.keyboard.press( 'Tab' );
await page.keyboard.press( 'Enter' );

expect(
await page
Expand Down Expand Up @@ -683,20 +688,21 @@ test.describe( 'Links', () => {
await pageUtils.pressKeys( 'primary+k' );
await page.keyboard.type( 'w.org' );
await page.keyboard.press( 'Enter' );
await page.keyboard.press( 'Escape' );

// Move to edge of text "Gutenberg".
await pageUtils.pressKeys( 'shiftAlt+ArrowLeft' ); // If you just use Alt here it won't work on windows.
await pageUtils.pressKeys( 'ArrowLeft' );
await pageUtils.pressKeys( 'ArrowLeft' );

// Select "Gutenberg".
await pageUtils.pressKeys( 'shiftAlt+ArrowLeft' );
await pageUtils.pressKeys( 'shiftAlt+ArrowRight' );

// Create a link.
await pageUtils.pressKeys( 'primary+k' );
await page.keyboard.type( 'https://wordpress.org/plugins/gutenberg/' );
await page.keyboard.press( 'Enter' );

await page.keyboard.press( 'Escape' );
await pageUtils.pressKeys( 'End' );
// Move back into the link.
await pageUtils.pressKeys( 'shiftAlt+ArrowLeft' );
await pageUtils.pressKeys( 'primary+k' );
Expand Down Expand Up @@ -1054,6 +1060,9 @@ test.describe( 'Links', () => {

// Update the link.
await pageUtils.pressKeys( 'Enter' );
await pageUtils.pressKeys( 'Escape' );
await pageUtils.pressKeys( 'ArrowRight' );

// Reactivate the link.
await pageUtils.pressKeys( 'ArrowLeft' );
await pageUtils.pressKeys( 'ArrowLeft' );
Expand Down Expand Up @@ -1117,11 +1126,10 @@ test.describe( 'Links', () => {

// Update the link.
await pageUtils.pressKeys( 'Enter' );
await pageUtils.pressKeys( 'Escape' );

// Move cursor next to the **end** of `linkTextOne`
await pageUtils.pressKeys( 'ArrowLeft', {
times: linkedTextTwo.length,
} );
await pageUtils.pressKeys( 'ArrowLeft' );

// Select `linkTextOne`
await pageUtils.pressKeys( 'shiftAlt+ArrowLeft' );
Expand All @@ -1134,6 +1142,8 @@ test.describe( 'Links', () => {

// Update the link.
await pageUtils.pressKeys( 'Enter' );
await pageUtils.pressKeys( 'Escape' );
await pageUtils.pressKeys( 'ArrowRight' );

// Move cursor within `linkTextOne`
await pageUtils.pressKeys( 'ArrowLeft', {
Expand All @@ -1146,8 +1156,8 @@ test.describe( 'Links', () => {
await expect( linkPopover ).toBeVisible();

// Expand selection so that it overlaps with `linkTextTwo`
await pageUtils.pressKeys( 'ArrowRight', {
times: 3,
await pageUtils.pressKeys( 'Shift+ArrowRight', {
times: 6,
} );

// Link UI should be inactive.
Expand Down Expand Up @@ -1254,6 +1264,8 @@ class LinkUtils {

// Click on the Submit button.
await this.pageUtils.pressKeys( 'Enter' );
await this.pageUtils.pressKeys( 'Escape' );
await this.pageUtils.pressKeys( 'End' );

// Reselect the link.
await this.pageUtils.pressKeys( 'shiftAlt+ArrowLeft' );
Expand Down
Loading