From 7c9c6bc0186e4f0500fc18224aba1257fa0041fc Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 11 Aug 2023 12:05:21 +0100 Subject: [PATCH 1/9] Add opensInNewTab to Link Preview step --- .../block-editor/src/components/link-control/index.js | 11 +++++++++++ .../src/components/link-control/link-preview.js | 3 +++ 2 files changed, 14 insertions(+) diff --git a/packages/block-editor/src/components/link-control/index.js b/packages/block-editor/src/components/link-control/index.js index 0de609f7c137d9..6ad00732587915 100644 --- a/packages/block-editor/src/components/link-control/index.js +++ b/packages/block-editor/src/components/link-control/index.js @@ -408,6 +408,17 @@ function LinkControl( { onEditClick={ () => setIsEditingLink( true ) } hasRichPreviews={ hasRichPreviews } hasUnlinkControl={ shownUnlinkControl } + additionalControls={ () => ( + { + onChange( { + opensInNewTab, + } ); + } } + /> + ) } onRemove={ () => { onRemove(); setIsEditingLink( true ); diff --git a/packages/block-editor/src/components/link-control/link-preview.js b/packages/block-editor/src/components/link-control/link-preview.js index a163968238ffe3..8272602cde908d 100644 --- a/packages/block-editor/src/components/link-control/link-preview.js +++ b/packages/block-editor/src/components/link-control/link-preview.js @@ -29,6 +29,7 @@ export default function LinkPreview( { hasRichPreviews = false, hasUnlinkControl = false, onRemove, + additionalControls, } ) { // Avoid fetching if rich previews are not desired. const showRichPreviews = hasRichPreviews ? value?.url : null; @@ -167,6 +168,8 @@ export default function LinkPreview( { ) } ) } + + { additionalControls && additionalControls() } ); } From 3cf173227f18eb48a0b058e2ab23bc3a2e7d8367 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 11 Aug 2023 12:06:33 +0100 Subject: [PATCH 2/9] Remove settings state tracking from inline link handling --- packages/format-library/src/link/inline.js | 45 +++++----------------- 1 file changed, 10 insertions(+), 35 deletions(-) diff --git a/packages/format-library/src/link/inline.js b/packages/format-library/src/link/inline.js index 171dd410f2bd81..8123514ad6bd7a 100644 --- a/packages/format-library/src/link/inline.js +++ b/packages/format-library/src/link/inline.js @@ -1,7 +1,7 @@ /** * WordPress dependencies */ -import { useState, useRef, createInterpolateElement } from '@wordpress/element'; +import { useRef, createInterpolateElement } from '@wordpress/element'; import { __, sprintf } from '@wordpress/i18n'; import { speak } from '@wordpress/a11y'; import { Popover } from '@wordpress/components'; @@ -45,16 +45,6 @@ function InlineLinkUI( { // Get the text content minus any HTML tags. const richTextText = richLinkTextValue.text; - /** - * Pending settings to be applied to the next link. When inserting a new - * link, toggle values cannot be applied immediately, because there is not - * yet a link for them to apply to. Thus, they are maintained in a state - * value until the time that the link can be inserted or edited. - * - * @type {[Object|undefined,Function]} - */ - const [ nextLinkValue, setNextLinkValue ] = useState(); - const { createPageEntity, userCanCreatePages } = useSelect( ( select ) => { const { getSettings } = select( blockEditorStore ); const _settings = getSettings(); @@ -71,7 +61,6 @@ function InlineLinkUI( { id: activeAttributes.id, opensInNewTab: activeAttributes.target === '_blank', title: richTextText, - ...nextLinkValue, }; function removeLink() { @@ -82,32 +71,18 @@ function InlineLinkUI( { } function onChangeLink( nextValue ) { - // Merge with values from state, both for the purpose of assigning the - // next state value, and for use in constructing the new link format if - // the link is ready to be applied. - nextValue = { - ...nextLinkValue, - ...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 && - linkValue.url === nextValue.url; - - // If change handler was called as a result of a settings change during - // link insertion, it must be held in state until the link is ready to - // be applied. - const didToggleSettingForNewLink = - didToggleSetting && nextValue.url === undefined; + nextValue.url === undefined; - // If link will be assigned, the state value can be considered flushed. - // Otherwise, persist the pending changes. - setNextLinkValue( didToggleSettingForNewLink ? nextValue : undefined ); - - if ( didToggleSettingForNewLink ) { - return; - } + // Merge the next value with the current link value. + nextValue = { + ...linkValue, + ...nextValue, + }; const newUrl = prependHTTP( nextValue.url ); const linkFormat = createLinkFormat( { @@ -192,7 +167,7 @@ function InlineLinkUI( { // Focus should only be shifted back to the formatted segment when the // URL is submitted. if ( ! didToggleSetting ) { - stopAddingLink(); + // stopAddingLink(); } if ( ! isValidHref( newUrl ) ) { From 6e9a5bd3425b963edc63fd6ed4bd502677ad48f8 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 11 Aug 2023 12:15:50 +0100 Subject: [PATCH 3/9] Ensure only opensInNewTab can be exposed in preview --- .../src/components/link-control/index.js | 36 ++++++++++++------- 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/packages/block-editor/src/components/link-control/index.js b/packages/block-editor/src/components/link-control/index.js index 6ad00732587915..bcc42ed25e2101 100644 --- a/packages/block-editor/src/components/link-control/index.js +++ b/packages/block-editor/src/components/link-control/index.js @@ -408,17 +408,27 @@ function LinkControl( { onEditClick={ () => setIsEditingLink( true ) } hasRichPreviews={ hasRichPreviews } hasUnlinkControl={ shownUnlinkControl } - additionalControls={ () => ( - { - onChange( { - opensInNewTab, - } ); - } } - /> - ) } + additionalControls={ () => { + // Expose the "Opens in new tab" settings in the preview + // as it is the most common setting to change. + if ( + settings?.find( + ( setting ) => setting.id === 'opensInNewTab' + ) + ) { + return ( + { + onChange( { + opensInNewTab, + } ); + } } + /> + ); + } + } } onRemove={ () => { onRemove(); setIsEditingLink( true ); @@ -435,7 +445,9 @@ function LinkControl( { > id === 'opensInNewTab' + ) } onChange={ createSetInternalSettingValueHandler( settingsKeys ) } From 4e0117c9a91905bfd10cbd32e06c8da7587ce1b9 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 11 Aug 2023 12:33:14 +0100 Subject: [PATCH 4/9] Reinstate closing of Link UI --- 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 8123514ad6bd7a..37e272239ceba3 100644 --- a/packages/format-library/src/link/inline.js +++ b/packages/format-library/src/link/inline.js @@ -167,7 +167,7 @@ function InlineLinkUI( { // Focus should only be shifted back to the formatted segment when the // URL is submitted. if ( ! didToggleSetting ) { - // stopAddingLink(); + stopAddingLink(); } if ( ! isValidHref( newUrl ) ) { From 50236bf004bff92a85c3d2b4fad5e725ab0da4a4 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 11 Aug 2023 12:57:49 +0100 Subject: [PATCH 5/9] Do not make formats inactive when creating link --- packages/format-library/src/link/inline.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/format-library/src/link/inline.js b/packages/format-library/src/link/inline.js index 37e272239ceba3..3a31c34142b6f8 100644 --- a/packages/format-library/src/link/inline.js +++ b/packages/format-library/src/link/inline.js @@ -160,7 +160,6 @@ function InlineLinkUI( { } newValue.start = newValue.end; - newValue.activeFormats = []; onChange( newValue ); } From 186ce583fb1c62ada48b08f79dfaabaf640527c2 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Tue, 15 Aug 2023 10:34:40 +0100 Subject: [PATCH 6/9] Reinstate auto-hide of UI after link submission as temp patch Bug identified. Decided to ship what we had. See https://github.com/WordPress/gutenberg/pull/53566#issuecomment-1678638946 --- packages/format-library/src/link/inline.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/format-library/src/link/inline.js b/packages/format-library/src/link/inline.js index 3a31c34142b6f8..29636a8d8b94be 100644 --- a/packages/format-library/src/link/inline.js +++ b/packages/format-library/src/link/inline.js @@ -160,6 +160,9 @@ function InlineLinkUI( { } newValue.start = newValue.end; + + // Hides the Link UI. + newValue.activeFormats = []; onChange( newValue ); } From 625cd2a2bb2a8cc5bdb75d6f52e4f1905637a2aa Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Tue, 15 Aug 2023 12:50:57 +0100 Subject: [PATCH 7/9] Fix bug only allowing opens in new tab in advanced settings --- .../block-editor/src/components/link-control/index.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/block-editor/src/components/link-control/index.js b/packages/block-editor/src/components/link-control/index.js index bcc42ed25e2101..9d9e6f4e188d42 100644 --- a/packages/block-editor/src/components/link-control/index.js +++ b/packages/block-editor/src/components/link-control/index.js @@ -419,7 +419,9 @@ function LinkControl( { return ( id === 'opensInNewTab' + ) } onChange={ ( { opensInNewTab } ) => { onChange( { opensInNewTab, @@ -445,9 +447,7 @@ function LinkControl( { > id === 'opensInNewTab' - ) } + settings={ settings } onChange={ createSetInternalSettingValueHandler( settingsKeys ) } From eb29835d7ee908926d7ab13bd6b9a8baf2df45dc Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Tue, 15 Aug 2023 13:20:51 +0100 Subject: [PATCH 8/9] Augment test to assert that checking box will apply changes --- .../src/components/link-control/test/index.js | 67 +++++++++++++++++-- 1 file changed, 63 insertions(+), 4 deletions(-) diff --git a/packages/block-editor/src/components/link-control/test/index.js b/packages/block-editor/src/components/link-control/test/index.js index b08be26cae0f24..4e3e9f45e4dea2 100644 --- a/packages/block-editor/src/components/link-control/test/index.js +++ b/packages/block-editor/src/components/link-control/test/index.js @@ -1734,7 +1734,66 @@ describe( 'Selecting links', () => { } ); describe( 'Addition Settings UI', () => { - it( 'should hide advanced link settings when not editing a link', async () => { + it( 'should allow toggling the "Opens in new tab" setting control (only) on the link preview', async () => { + const user = userEvent.setup(); + const selectedLink = fauxEntitySuggestions[ 0 ]; + const mockOnChange = jest.fn(); + + const customSettings = [ + { + id: 'opensInNewTab', + title: 'Open in new tab', + }, + { + id: 'noFollow', + title: 'No follow', + }, + ]; + + const LinkControlConsumer = () => { + const [ link, setLink ] = useState( selectedLink ); + + return ( + { + mockOnChange( newVal ); + setLink( newVal ); + } } + /> + ); + }; + + render( ); + + const opensInNewTabField = screen.queryByRole( 'checkbox', { + name: 'Open in new tab', + checked: false, + } ); + + expect( opensInNewTabField ).toBeInTheDocument(); + + // No matter which settings are passed in only the `Opens in new tab` + // setting should be shown on the link preview (non-editing) state. + const noFollowField = screen.queryByRole( 'checkbox', { + name: 'No follow', + } ); + expect( noFollowField ).not.toBeInTheDocument(); + + // Check that the link value is updated immediately upon checking + // the checkbox. + await user.click( opensInNewTabField ); + + expect( opensInNewTabField ).toBeChecked(); + + expect( mockOnChange ).toHaveBeenCalledTimes( 1 ); + expect( mockOnChange ).toHaveBeenCalledWith( { + opensInNewTab: true, + } ); + } ); + + it( 'should hide advanced link settings and toggle when not editing a link', async () => { const selectedLink = fauxEntitySuggestions[ 0 ]; const LinkControlConsumer = () => { @@ -1750,7 +1809,7 @@ describe( 'Addition Settings UI', () => { expect( settingsToggle ).not.toBeInTheDocument(); } ); - it( 'should provides a means to toggle the link settings', async () => { + it( 'should provides a means to toggle the advanced link settings when editing a link', async () => { const selectedLink = fauxEntitySuggestions[ 0 ]; const LinkControlConsumer = () => { @@ -1785,7 +1844,7 @@ describe( 'Addition Settings UI', () => { expect( newTabSettingInput ).not.toBeVisible(); } ); - it( 'should display "New Tab" setting (in "off" mode) by default when a link is selected', async () => { + it( 'should display "New Tab" setting (in "off" mode) by default when a link is edited', async () => { const selectedLink = fauxEntitySuggestions[ 0 ]; const expectedSettingText = 'Open in new tab'; @@ -1817,7 +1876,7 @@ describe( 'Addition Settings UI', () => { const customSettings = [ { - id: 'newTab', + id: 'opensInNewTab', title: 'Open in new tab', }, { From 00e5641ccc3e81ef31d85f49d9b8aa5a2930de54 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Tue, 15 Aug 2023 13:32:28 +0100 Subject: [PATCH 9/9] Implement spacing See https://github.com/WordPress/gutenberg/pull/53566#issuecomment-1674877353 --- packages/block-editor/src/components/link-control/style.scss | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/block-editor/src/components/link-control/style.scss b/packages/block-editor/src/components/link-control/style.scss index 930f9357946667..0ca13fa9167773 100644 --- a/packages/block-editor/src/components/link-control/style.scss +++ b/packages/block-editor/src/components/link-control/style.scss @@ -413,6 +413,10 @@ $preview-image-height: 140px; &.block-editor-link-control__setting:last-child { margin-bottom: 0; } + + .is-preview & { + padding: 20px $grid-unit-10 $grid-unit-10 0; + } } .block-editor-link-control__tools {