From 3318717c80fd62cfa7a1fd7cf9c5ec33470b45b0 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Wed, 24 Jan 2024 09:40:26 +0000 Subject: [PATCH] Keep Link UI open upon initial link creation when used in RichText (#57726) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Don’t clear activeFormats on submit * Expose API to allow triggering the anchor to be recomputed * Correctly position Popover on link creation and avoid focus shifts due to forced remounts. * Add privateApis to RichText package and export useAnchorWithUpdate hook * Consume new private API in Link UI * Fix docs * Revert "Add privateApis to RichText package and export useAnchorWithUpdate hook" This reverts commit cbcbf41da512b93dc4271d4428faab34aafd389b. # Conflicts: # packages/rich-text/README.md # packages/rich-text/src/private-apis.js * Fix anchor position using standardised method * Revert all changes to useAnchor hook * Remove changing selection when commiting new links * Add more detail to comment * Fix e2e test: can be created and modified using only the keyboard * Test e2e fix: can update the url of an existing link * Fix e2e test: toggle state of advanced link settings is preserved across editing links * Test e2e fix createAndReselectLink util * Fix e2e test: should not show the Link UI when selection extends beyond link boundary * e2e test fix: should not show the Link UI when selection extends into another link --------- Co-authored-by: Jerry Jones --- packages/format-library/src/link/inline.js | 42 ++++++++++----------- test/e2e/specs/editor/blocks/links.spec.js | 44 ++++++++++++++-------- 2 files changed, 48 insertions(+), 38 deletions(-) diff --git a/packages/format-library/src/link/inline.js b/packages/format-library/src/link/inline.js index 39fcc0f3b0734..79933be6a6ac5 100644 --- a/packages/format-library/src/link/inline.js +++ b/packages/format-library/src/link/inline.js @@ -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'; @@ -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, @@ -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, @@ -178,17 +175,16 @@ function InlineLinkUI( { newValue = concat( valBefore, newValAfter ); } - newValue.start = newValue.end; - - // 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 ) ) { @@ -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 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 @@ -261,7 +260,6 @@ function InlineLinkUI( { shift > { 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. @@ -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( [ { @@ -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 @@ -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' ); @@ -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' ); @@ -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' ); @@ -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', { @@ -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. @@ -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' );