From 485cc81d5f3efaf44edb175f2aa4f74afb05f8c5 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Wed, 10 Jan 2024 15:00:27 +0000 Subject: [PATCH 01/17] =?UTF-8?q?Don=E2=80=99t=20clear=20activeFormats=20o?= =?UTF-8?q?n=20submit?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/format-library/src/link/inline.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/format-library/src/link/inline.js b/packages/format-library/src/link/inline.js index 58da13eb51ab6b..48993e8975cca6 100644 --- a/packages/format-library/src/link/inline.js +++ b/packages/format-library/src/link/inline.js @@ -180,8 +180,6 @@ function InlineLinkUI( { newValue.start = newValue.end; - // Hides the Link UI. - newValue.activeFormats = []; onChange( newValue ); } From a457c4853b15b3290f4416e26445f0cab7b57d6c Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Wed, 10 Jan 2024 21:08:02 +0000 Subject: [PATCH 02/17] Expose API to allow triggering the anchor to be recomputed --- packages/rich-text/src/component/use-anchor.js | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/packages/rich-text/src/component/use-anchor.js b/packages/rich-text/src/component/use-anchor.js index 7ea3281cb60d3a..ce571858a9b99f 100644 --- a/packages/rich-text/src/component/use-anchor.js +++ b/packages/rich-text/src/component/use-anchor.js @@ -1,7 +1,7 @@ /** * WordPress dependencies */ -import { useState, useLayoutEffect } from '@wordpress/element'; +import { useState, useLayoutEffect, useCallback } from '@wordpress/element'; /** @typedef {import('../register-format-type').WPFormat} WPFormat */ /** @typedef {import('../types').RichTextValue} RichTextValue */ @@ -142,17 +142,15 @@ export function useAnchor( { editableContentElement, settings = {} } ) { getAnchor( editableContentElement, tagName, className ) ); + const callback = useCallback( () => { + setAnchor( getAnchor( editableContentElement, tagName, className ) ); + }, [ className, editableContentElement, tagName ] ); + useLayoutEffect( () => { if ( ! editableContentElement ) return; const { ownerDocument } = editableContentElement; - function callback() { - setAnchor( - getAnchor( editableContentElement, tagName, className ) - ); - } - function attach() { ownerDocument.addEventListener( 'selectionchange', callback ); } @@ -174,7 +172,8 @@ export function useAnchor( { editableContentElement, settings = {} } ) { editableContentElement.removeEventListener( 'focusin', attach ); editableContentElement.removeEventListener( 'focusout', detach ); }; - }, [ editableContentElement, tagName, className ] ); + }, [ editableContentElement, tagName, className, callback ] ); + anchor.update = callback; return anchor; } From 2b055be236d3ec3fd02a5624c2cca6d883263f60 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Wed, 10 Jan 2024 21:11:07 +0000 Subject: [PATCH 03/17] Correctly position Popover on link creation and avoid focus shifts due to forced remounts. --- packages/format-library/src/link/inline.js | 35 +++++++++++----------- 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/packages/format-library/src/link/inline.js b/packages/format-library/src/link/inline.js index 48993e8975cca6..f9467c1fc5ee23 100644 --- a/packages/format-library/src/link/inline.js +++ b/packages/format-library/src/link/inline.js @@ -29,7 +29,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 +89,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, @@ -181,12 +177,22 @@ function InlineLinkUI( { newValue.start = newValue.end; onChange( newValue ); + + // Force update the anchor reference to ensure the Popover is positioned correctly/ + // Incorrect positioning can happen on the initial creation of a link because the + // anchor changes from a rich text selection to a link format element (e.g. ). + // however the Popover is not repositioned to account for this change because the + // dependencies of `useAnchor` do not change. + popoverAnchor?.update(); } - // 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 ) ) { @@ -208,12 +214,6 @@ 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 ); - // 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 @@ -258,7 +258,6 @@ function InlineLinkUI( { shift > Date: Fri, 12 Jan 2024 09:54:53 +0000 Subject: [PATCH 04/17] Add privateApis to RichText package and export useAnchorWithUpdate hook --- package-lock.json | 2 + packages/private-apis/src/implementation.js | 1 + packages/rich-text/README.md | 4 ++ packages/rich-text/package.json | 1 + .../rich-text/src/component/use-anchor.js | 55 ++++++++++--------- packages/rich-text/src/index.ts | 2 + packages/rich-text/src/lock-unlock.js | 10 ++++ packages/rich-text/src/private-apis.js | 13 +++++ 8 files changed, 62 insertions(+), 26 deletions(-) create mode 100644 packages/rich-text/src/lock-unlock.js create mode 100644 packages/rich-text/src/private-apis.js diff --git a/package-lock.json b/package-lock.json index 2a2fc30b96099b..3faa5d6ab67aef 100644 --- a/package-lock.json +++ b/package-lock.json @@ -55930,6 +55930,7 @@ "@wordpress/escape-html": "file:../escape-html", "@wordpress/i18n": "file:../i18n", "@wordpress/keycodes": "file:../keycodes", + "@wordpress/private-apis": "file:../private-apis", "memize": "^2.1.0", "rememo": "^4.0.2" }, @@ -70507,6 +70508,7 @@ "@wordpress/escape-html": "file:../escape-html", "@wordpress/i18n": "file:../i18n", "@wordpress/keycodes": "file:../keycodes", + "@wordpress/private-apis": "file:../private-apis", "memize": "^2.1.0", "rememo": "^4.0.2" } diff --git a/packages/private-apis/src/implementation.js b/packages/private-apis/src/implementation.js index a31fd91ce094dd..9035bbfa7cd66c 100644 --- a/packages/private-apis/src/implementation.js +++ b/packages/private-apis/src/implementation.js @@ -29,6 +29,7 @@ const CORE_MODULES_USING_PRIVATE_APIS = [ '@wordpress/patterns', '@wordpress/preferences', '@wordpress/reusable-blocks', + '@wordpress/rich-text', '@wordpress/router', '@wordpress/dataviews', ]; diff --git a/packages/rich-text/README.md b/packages/rich-text/README.md index 90fd15e1c905c5..ac58e81fa5d84b 100644 --- a/packages/rich-text/README.md +++ b/packages/rich-text/README.md @@ -299,6 +299,10 @@ _Returns_ - `RichTextValue`: A new combined value. +### privateApis + +Private @wordpress/block-editor APIs. + ### registerFormatType Registers a new format provided a unique name and an object defining its behavior. diff --git a/packages/rich-text/package.json b/packages/rich-text/package.json index ef935eea5686fc..a60aff28bf0c4b 100644 --- a/packages/rich-text/package.json +++ b/packages/rich-text/package.json @@ -39,6 +39,7 @@ "@wordpress/escape-html": "file:../escape-html", "@wordpress/i18n": "file:../i18n", "@wordpress/keycodes": "file:../keycodes", + "@wordpress/private-apis": "file:../private-apis", "memize": "^2.1.0", "rememo": "^4.0.2" }, diff --git a/packages/rich-text/src/component/use-anchor.js b/packages/rich-text/src/component/use-anchor.js index ce571858a9b99f..8cfdedb6d1474f 100644 --- a/packages/rich-text/src/component/use-anchor.js +++ b/packages/rich-text/src/component/use-anchor.js @@ -86,18 +86,6 @@ function createVirtualAnchorElement( range, editableContentElement ) { }; } -/** - * Get the anchor: a format element if there is a matching one based on the - * tagName and className or a range otherwise. - * - * @param {HTMLElement} editableContentElement The editable wrapper. - * @param {string} tagName The tag name of the format - * element. - * @param {string} className The class name of the format - * element. - * - * @return {HTMLElement|VirtualAnchorElement|undefined} The anchor. - */ function getAnchor( editableContentElement, tagName, className ) { if ( ! editableContentElement ) return; @@ -124,19 +112,7 @@ function getAnchor( editableContentElement, tagName, className ) { return createVirtualAnchorElement( range, editableContentElement ); } -/** - * This hook, to be used in a format type's Edit component, returns the active - * element that is formatted, or a virtual element for the selection range if - * no format is active. The returned value is meant to be used for positioning - * UI, e.g. by passing it to the `Popover` component via the `anchor` prop. - * - * @param {Object} $1 Named parameters. - * @param {HTMLElement|null} $1.editableContentElement The element containing - * the editable content. - * @param {WPFormat=} $1.settings The format type's settings. - * @return {Element|VirtualAnchorElement|undefined|null} The active element or selection range. - */ -export function useAnchor( { editableContentElement, settings = {} } ) { +function useAnchorBase( { editableContentElement, settings = {} } ) { const { tagName, className } = settings; const [ anchor, setAnchor ] = useState( () => getAnchor( editableContentElement, tagName, className ) @@ -174,6 +150,33 @@ export function useAnchor( { editableContentElement, settings = {} } ) { }; }, [ editableContentElement, tagName, className, callback ] ); - anchor.update = callback; + return { + anchor, + update: callback, + }; +} + +/** + * This hook, to be used in a format type's Edit component, returns the active + * element that is formatted, or a virtual element for the selection range if + * no format is active. The returned value is meant to be used for positioning + * UI, e.g. by passing it to the `Popover` component via the `anchor` prop. + * + * @param {Object} $1 Named parameters. + * @param {HTMLElement|null} $1.editableContentElement The element containing + * the editable content. + * @param {WPFormat=} $1.settings The format type's settings. + * @return {Element|VirtualAnchorElement|undefined|null} The active element or selection range. + */ +export function useAnchor( { editableContentElement, settings = {} } ) { + const { anchor } = useAnchorBase( { editableContentElement, settings } ); + return anchor; } + +export function useAnchorWithUpdate( { + editableContentElement, + settings = {}, +} ) { + return useAnchorBase( { editableContentElement, settings } ); +} diff --git a/packages/rich-text/src/index.ts b/packages/rich-text/src/index.ts index f82317d81573d0..57f8bad762628b 100644 --- a/packages/rich-text/src/index.ts +++ b/packages/rich-text/src/index.ts @@ -36,3 +36,5 @@ export { * documentation for more information. */ export type { RichTextValue } from './types'; + +export { privateApis } from './private-apis'; diff --git a/packages/rich-text/src/lock-unlock.js b/packages/rich-text/src/lock-unlock.js new file mode 100644 index 00000000000000..fa8c56b7d3ff64 --- /dev/null +++ b/packages/rich-text/src/lock-unlock.js @@ -0,0 +1,10 @@ +/** + * WordPress dependencies + */ +import { __dangerousOptInToUnstableAPIsOnlyForCoreModules } from '@wordpress/private-apis'; + +export const { lock, unlock } = + __dangerousOptInToUnstableAPIsOnlyForCoreModules( + 'I know using unstable features means my theme or plugin will inevitably break in the next version of WordPress.', + '@wordpress/rich-text' + ); diff --git a/packages/rich-text/src/private-apis.js b/packages/rich-text/src/private-apis.js new file mode 100644 index 00000000000000..398ffb40651460 --- /dev/null +++ b/packages/rich-text/src/private-apis.js @@ -0,0 +1,13 @@ +/** + * Internal dependencies + */ +import { useAnchorWithUpdate } from './component/use-anchor'; +import { lock } from './lock-unlock'; + +/** + * Private @wordpress/block-editor APIs. + */ +export const privateApis = {}; +lock( privateApis, { + useAnchorWithUpdate, +} ); From 343c735ba245e73c4762f2f61895a71f36f68068 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 12 Jan 2024 09:55:09 +0000 Subject: [PATCH 05/17] Consume new private API in Link UI --- packages/format-library/src/link/inline.js | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/packages/format-library/src/link/inline.js b/packages/format-library/src/link/inline.js index f9467c1fc5ee23..1fd5dd2cc31727 100644 --- a/packages/format-library/src/link/inline.js +++ b/packages/format-library/src/link/inline.js @@ -11,12 +11,12 @@ import { insert, isCollapsed, applyFormat, - useAnchor, removeFormat, slice, replace, split, concat, + privateApis as richTextPrivateApis, } from '@wordpress/rich-text'; import { __experimentalLinkControl as LinkControl, @@ -29,6 +29,9 @@ import { useSelect } from '@wordpress/data'; */ import { createLinkFormat, isValidHref, getFormatBoundary } from './utils'; import { link as settings } from './index'; +import { unlock } from '../lock-unlock'; + +const { useAnchorWithUpdate } = unlock( richTextPrivateApis ); const LINK_SETTINGS = [ ...LinkControl.DEFAULT_LINK_SETTINGS, @@ -183,7 +186,7 @@ function InlineLinkUI( { // anchor changes from a rich text selection to a link format element (e.g. ). // however the Popover is not repositioned to account for this change because the // dependencies of `useAnchor` do not change. - popoverAnchor?.update(); + updatePopoverAnchor(); } // Focus should only be returned to the rich text on submit if this link is not @@ -209,10 +212,11 @@ function InlineLinkUI( { } } - const popoverAnchor = useAnchor( { - editableContentElement: contentRef.current, - settings, - } ); + const { anchor: popoverAnchor, update: updatePopoverAnchor } = + useAnchorWithUpdate( { + editableContentElement: contentRef.current, + settings, + } ); // 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 From 306bf385b6c478cf6edd2aae47c5d1e4618f289a Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 12 Jan 2024 09:57:02 +0000 Subject: [PATCH 06/17] Fix docs --- packages/rich-text/README.md | 2 +- packages/rich-text/src/private-apis.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/rich-text/README.md b/packages/rich-text/README.md index ac58e81fa5d84b..89d4ae5be946fa 100644 --- a/packages/rich-text/README.md +++ b/packages/rich-text/README.md @@ -301,7 +301,7 @@ _Returns_ ### privateApis -Private @wordpress/block-editor APIs. +Private @wordpress/rich-text APIs. ### registerFormatType diff --git a/packages/rich-text/src/private-apis.js b/packages/rich-text/src/private-apis.js index 398ffb40651460..01bb0653a5697f 100644 --- a/packages/rich-text/src/private-apis.js +++ b/packages/rich-text/src/private-apis.js @@ -5,7 +5,7 @@ import { useAnchorWithUpdate } from './component/use-anchor'; import { lock } from './lock-unlock'; /** - * Private @wordpress/block-editor APIs. + * Private @wordpress/rich-text APIs. */ export const privateApis = {}; lock( privateApis, { From 0f5202bc2d4977d60fff825147efa0bfb4f06839 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 12 Jan 2024 14:47:40 +0000 Subject: [PATCH 07/17] 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 --- package-lock.json | 2 - packages/private-apis/src/implementation.js | 1 - packages/rich-text/README.md | 4 -- packages/rich-text/package.json | 1 - .../rich-text/src/component/use-anchor.js | 55 +++++++++---------- packages/rich-text/src/index.ts | 2 - packages/rich-text/src/lock-unlock.js | 10 ---- packages/rich-text/src/private-apis.js | 13 ----- 8 files changed, 26 insertions(+), 62 deletions(-) delete mode 100644 packages/rich-text/src/lock-unlock.js delete mode 100644 packages/rich-text/src/private-apis.js diff --git a/package-lock.json b/package-lock.json index 3faa5d6ab67aef..2a2fc30b96099b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -55930,7 +55930,6 @@ "@wordpress/escape-html": "file:../escape-html", "@wordpress/i18n": "file:../i18n", "@wordpress/keycodes": "file:../keycodes", - "@wordpress/private-apis": "file:../private-apis", "memize": "^2.1.0", "rememo": "^4.0.2" }, @@ -70508,7 +70507,6 @@ "@wordpress/escape-html": "file:../escape-html", "@wordpress/i18n": "file:../i18n", "@wordpress/keycodes": "file:../keycodes", - "@wordpress/private-apis": "file:../private-apis", "memize": "^2.1.0", "rememo": "^4.0.2" } diff --git a/packages/private-apis/src/implementation.js b/packages/private-apis/src/implementation.js index 9035bbfa7cd66c..a31fd91ce094dd 100644 --- a/packages/private-apis/src/implementation.js +++ b/packages/private-apis/src/implementation.js @@ -29,7 +29,6 @@ const CORE_MODULES_USING_PRIVATE_APIS = [ '@wordpress/patterns', '@wordpress/preferences', '@wordpress/reusable-blocks', - '@wordpress/rich-text', '@wordpress/router', '@wordpress/dataviews', ]; diff --git a/packages/rich-text/README.md b/packages/rich-text/README.md index 89d4ae5be946fa..90fd15e1c905c5 100644 --- a/packages/rich-text/README.md +++ b/packages/rich-text/README.md @@ -299,10 +299,6 @@ _Returns_ - `RichTextValue`: A new combined value. -### privateApis - -Private @wordpress/rich-text APIs. - ### registerFormatType Registers a new format provided a unique name and an object defining its behavior. diff --git a/packages/rich-text/package.json b/packages/rich-text/package.json index a60aff28bf0c4b..ef935eea5686fc 100644 --- a/packages/rich-text/package.json +++ b/packages/rich-text/package.json @@ -39,7 +39,6 @@ "@wordpress/escape-html": "file:../escape-html", "@wordpress/i18n": "file:../i18n", "@wordpress/keycodes": "file:../keycodes", - "@wordpress/private-apis": "file:../private-apis", "memize": "^2.1.0", "rememo": "^4.0.2" }, diff --git a/packages/rich-text/src/component/use-anchor.js b/packages/rich-text/src/component/use-anchor.js index 8cfdedb6d1474f..ce571858a9b99f 100644 --- a/packages/rich-text/src/component/use-anchor.js +++ b/packages/rich-text/src/component/use-anchor.js @@ -86,6 +86,18 @@ function createVirtualAnchorElement( range, editableContentElement ) { }; } +/** + * Get the anchor: a format element if there is a matching one based on the + * tagName and className or a range otherwise. + * + * @param {HTMLElement} editableContentElement The editable wrapper. + * @param {string} tagName The tag name of the format + * element. + * @param {string} className The class name of the format + * element. + * + * @return {HTMLElement|VirtualAnchorElement|undefined} The anchor. + */ function getAnchor( editableContentElement, tagName, className ) { if ( ! editableContentElement ) return; @@ -112,7 +124,19 @@ function getAnchor( editableContentElement, tagName, className ) { return createVirtualAnchorElement( range, editableContentElement ); } -function useAnchorBase( { editableContentElement, settings = {} } ) { +/** + * This hook, to be used in a format type's Edit component, returns the active + * element that is formatted, or a virtual element for the selection range if + * no format is active. The returned value is meant to be used for positioning + * UI, e.g. by passing it to the `Popover` component via the `anchor` prop. + * + * @param {Object} $1 Named parameters. + * @param {HTMLElement|null} $1.editableContentElement The element containing + * the editable content. + * @param {WPFormat=} $1.settings The format type's settings. + * @return {Element|VirtualAnchorElement|undefined|null} The active element or selection range. + */ +export function useAnchor( { editableContentElement, settings = {} } ) { const { tagName, className } = settings; const [ anchor, setAnchor ] = useState( () => getAnchor( editableContentElement, tagName, className ) @@ -150,33 +174,6 @@ function useAnchorBase( { editableContentElement, settings = {} } ) { }; }, [ editableContentElement, tagName, className, callback ] ); - return { - anchor, - update: callback, - }; -} - -/** - * This hook, to be used in a format type's Edit component, returns the active - * element that is formatted, or a virtual element for the selection range if - * no format is active. The returned value is meant to be used for positioning - * UI, e.g. by passing it to the `Popover` component via the `anchor` prop. - * - * @param {Object} $1 Named parameters. - * @param {HTMLElement|null} $1.editableContentElement The element containing - * the editable content. - * @param {WPFormat=} $1.settings The format type's settings. - * @return {Element|VirtualAnchorElement|undefined|null} The active element or selection range. - */ -export function useAnchor( { editableContentElement, settings = {} } ) { - const { anchor } = useAnchorBase( { editableContentElement, settings } ); - + anchor.update = callback; return anchor; } - -export function useAnchorWithUpdate( { - editableContentElement, - settings = {}, -} ) { - return useAnchorBase( { editableContentElement, settings } ); -} diff --git a/packages/rich-text/src/index.ts b/packages/rich-text/src/index.ts index 57f8bad762628b..f82317d81573d0 100644 --- a/packages/rich-text/src/index.ts +++ b/packages/rich-text/src/index.ts @@ -36,5 +36,3 @@ export { * documentation for more information. */ export type { RichTextValue } from './types'; - -export { privateApis } from './private-apis'; diff --git a/packages/rich-text/src/lock-unlock.js b/packages/rich-text/src/lock-unlock.js deleted file mode 100644 index fa8c56b7d3ff64..00000000000000 --- a/packages/rich-text/src/lock-unlock.js +++ /dev/null @@ -1,10 +0,0 @@ -/** - * WordPress dependencies - */ -import { __dangerousOptInToUnstableAPIsOnlyForCoreModules } from '@wordpress/private-apis'; - -export const { lock, unlock } = - __dangerousOptInToUnstableAPIsOnlyForCoreModules( - 'I know using unstable features means my theme or plugin will inevitably break in the next version of WordPress.', - '@wordpress/rich-text' - ); diff --git a/packages/rich-text/src/private-apis.js b/packages/rich-text/src/private-apis.js deleted file mode 100644 index 01bb0653a5697f..00000000000000 --- a/packages/rich-text/src/private-apis.js +++ /dev/null @@ -1,13 +0,0 @@ -/** - * Internal dependencies - */ -import { useAnchorWithUpdate } from './component/use-anchor'; -import { lock } from './lock-unlock'; - -/** - * Private @wordpress/rich-text APIs. - */ -export const privateApis = {}; -lock( privateApis, { - useAnchorWithUpdate, -} ); From 25ffdf787a1af6ceb6b44ab432be42e8dec42ffc Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 12 Jan 2024 14:49:38 +0000 Subject: [PATCH 08/17] Fix anchor position using standardised method --- packages/format-library/src/link/inline.js | 31 +++++++++++----------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/packages/format-library/src/link/inline.js b/packages/format-library/src/link/inline.js index 1fd5dd2cc31727..9cc1e97e905bcf 100644 --- a/packages/format-library/src/link/inline.js +++ b/packages/format-library/src/link/inline.js @@ -16,11 +16,12 @@ import { replace, split, concat, - privateApis as richTextPrivateApis, + useAnchor, } from '@wordpress/rich-text'; import { __experimentalLinkControl as LinkControl, store as blockEditorStore, + useCachedTruthy, } from '@wordpress/block-editor'; import { useSelect } from '@wordpress/data'; @@ -29,9 +30,6 @@ import { useSelect } from '@wordpress/data'; */ import { createLinkFormat, isValidHref, getFormatBoundary } from './utils'; import { link as settings } from './index'; -import { unlock } from '../lock-unlock'; - -const { useAnchorWithUpdate } = unlock( richTextPrivateApis ); const LINK_SETTINGS = [ ...LinkControl.DEFAULT_LINK_SETTINGS, @@ -180,13 +178,6 @@ function InlineLinkUI( { newValue.start = newValue.end; onChange( newValue ); - - // Force update the anchor reference to ensure the Popover is positioned correctly/ - // Incorrect positioning can happen on the initial creation of a link because the - // anchor changes from a rich text selection to a link format element (e.g. ). - // however the Popover is not repositioned to account for this change because the - // dependencies of `useAnchor` do not change. - updatePopoverAnchor(); } // Focus should only be returned to the rich text on submit if this link is not @@ -212,11 +203,19 @@ function InlineLinkUI( { } } - const { anchor: popoverAnchor, update: updatePopoverAnchor } = - useAnchorWithUpdate( { - editableContentElement: contentRef.current, - settings, - } ); + const popoverAnchor = useAnchor( { + editableContentElement: contentRef.current, + settings, + } ); + + // 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. + // + 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 From 91a766a757e788382c8c27505e10c7f4d6aa0703 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 12 Jan 2024 14:52:22 +0000 Subject: [PATCH 09/17] Revert all changes to useAnchor hook --- packages/rich-text/src/component/use-anchor.js | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/packages/rich-text/src/component/use-anchor.js b/packages/rich-text/src/component/use-anchor.js index ce571858a9b99f..7ea3281cb60d3a 100644 --- a/packages/rich-text/src/component/use-anchor.js +++ b/packages/rich-text/src/component/use-anchor.js @@ -1,7 +1,7 @@ /** * WordPress dependencies */ -import { useState, useLayoutEffect, useCallback } from '@wordpress/element'; +import { useState, useLayoutEffect } from '@wordpress/element'; /** @typedef {import('../register-format-type').WPFormat} WPFormat */ /** @typedef {import('../types').RichTextValue} RichTextValue */ @@ -142,15 +142,17 @@ export function useAnchor( { editableContentElement, settings = {} } ) { getAnchor( editableContentElement, tagName, className ) ); - const callback = useCallback( () => { - setAnchor( getAnchor( editableContentElement, tagName, className ) ); - }, [ className, editableContentElement, tagName ] ); - useLayoutEffect( () => { if ( ! editableContentElement ) return; const { ownerDocument } = editableContentElement; + function callback() { + setAnchor( + getAnchor( editableContentElement, tagName, className ) + ); + } + function attach() { ownerDocument.addEventListener( 'selectionchange', callback ); } @@ -172,8 +174,7 @@ export function useAnchor( { editableContentElement, settings = {} } ) { editableContentElement.removeEventListener( 'focusin', attach ); editableContentElement.removeEventListener( 'focusout', detach ); }; - }, [ editableContentElement, tagName, className, callback ] ); + }, [ editableContentElement, tagName, className ] ); - anchor.update = callback; return anchor; } From 011fe3b3f0dac1f44eddab9baa698b98ed0f99fa Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 12 Jan 2024 14:59:25 +0000 Subject: [PATCH 10/17] Remove changing selection when commiting new links --- packages/format-library/src/link/inline.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/format-library/src/link/inline.js b/packages/format-library/src/link/inline.js index 9cc1e97e905bcf..a870ad32f35f5d 100644 --- a/packages/format-library/src/link/inline.js +++ b/packages/format-library/src/link/inline.js @@ -175,8 +175,6 @@ function InlineLinkUI( { newValue = concat( valBefore, newValAfter ); } - newValue.start = newValue.end; - onChange( newValue ); } From a20eaceadffd0bfc639fcc5721d19e7cdeacb54b Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Tue, 23 Jan 2024 14:14:42 +0000 Subject: [PATCH 11/17] Add more detail to comment --- packages/format-library/src/link/inline.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/format-library/src/link/inline.js b/packages/format-library/src/link/inline.js index a870ad32f35f5d..0189a75a122f7d 100644 --- a/packages/format-library/src/link/inline.js +++ b/packages/format-library/src/link/inline.js @@ -211,7 +211,7 @@ function InlineLinkUI( { // 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; From 10858fa427e1bd1adc9857f7f925e0831ceff079 Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Tue, 23 Jan 2024 09:03:09 -0600 Subject: [PATCH 12/17] Fix e2e test: can be created and modified using only the keyboard --- test/e2e/specs/editor/blocks/links.spec.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/e2e/specs/editor/blocks/links.spec.js b/test/e2e/specs/editor/blocks/links.spec.js index 9620b45fed9da8..79c08858cd11ce 100644 --- a/test/e2e/specs/editor/blocks/links.spec.js +++ b/test/e2e/specs/editor/blocks/links.spec.js @@ -445,11 +445,13 @@ 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. await pageUtils.pressKeys( 'End' ); - await expect( linkPopover ).toBeHidden(); // Move the caret back into the link text and the link popover // should be displayed. From fb8f5a9d96075154ad61b13f5b14265b4574e26d Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Tue, 23 Jan 2024 09:17:03 -0600 Subject: [PATCH 13/17] Test e2e fix: can update the url of an existing link --- test/e2e/specs/editor/blocks/links.spec.js | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/test/e2e/specs/editor/blocks/links.spec.js b/test/e2e/specs/editor/blocks/links.spec.js index 79c08858cd11ce..8284f635f222cf 100644 --- a/test/e2e/specs/editor/blocks/links.spec.js +++ b/test/e2e/specs/editor/blocks/links.spec.js @@ -452,6 +452,7 @@ test.describe( 'Links', () => { // 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(); // Move the caret back into the link text and the link popover // should be displayed. @@ -614,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( [ { @@ -636,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 From 48923844038505bc84982ae8d177ac32a0cab6cd Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Tue, 23 Jan 2024 09:30:24 -0600 Subject: [PATCH 14/17] Fix e2e test: toggle state of advanced link settings is preserved across editing links --- test/e2e/specs/editor/blocks/links.spec.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/e2e/specs/editor/blocks/links.spec.js b/test/e2e/specs/editor/blocks/links.spec.js index 8284f635f222cf..55457618c1480e 100644 --- a/test/e2e/specs/editor/blocks/links.spec.js +++ b/test/e2e/specs/editor/blocks/links.spec.js @@ -688,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' ); From 28062a38eb509f415d006a9b62a20ae899670f60 Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Tue, 23 Jan 2024 09:33:22 -0600 Subject: [PATCH 15/17] Test e2e fix createAndReselectLink util --- test/e2e/specs/editor/blocks/links.spec.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/e2e/specs/editor/blocks/links.spec.js b/test/e2e/specs/editor/blocks/links.spec.js index 55457618c1480e..01dac4abf489a8 100644 --- a/test/e2e/specs/editor/blocks/links.spec.js +++ b/test/e2e/specs/editor/blocks/links.spec.js @@ -1260,6 +1260,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' ); From b94bdec91bc54ebfdc77235798f9116e41881cf4 Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Tue, 23 Jan 2024 09:37:11 -0600 Subject: [PATCH 16/17] Fix e2e test: should not show the Link UI when selection extends beyond link boundary --- test/e2e/specs/editor/blocks/links.spec.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/e2e/specs/editor/blocks/links.spec.js b/test/e2e/specs/editor/blocks/links.spec.js index 01dac4abf489a8..32e3a25f54b949 100644 --- a/test/e2e/specs/editor/blocks/links.spec.js +++ b/test/e2e/specs/editor/blocks/links.spec.js @@ -1060,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' ); From ebfba2d7b21e9d0b584bc0b328660ae7d8835451 Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Tue, 23 Jan 2024 09:46:11 -0600 Subject: [PATCH 17/17] e2e test fix: should not show the Link UI when selection extends into another link --- test/e2e/specs/editor/blocks/links.spec.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/test/e2e/specs/editor/blocks/links.spec.js b/test/e2e/specs/editor/blocks/links.spec.js index 32e3a25f54b949..d1de2df1da7ff7 100644 --- a/test/e2e/specs/editor/blocks/links.spec.js +++ b/test/e2e/specs/editor/blocks/links.spec.js @@ -1126,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' ); @@ -1143,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', { @@ -1155,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.