Add Open in new tab control into Link Preview (#53566)

* Add opensInNewTab to Link Preview step * Remove settings state tracking from inline link handling * Ensure only opensInNewTab can be exposed in preview * Reinstate closing of Link UI * Do not make formats inactive when creating link * Reinstate auto-hide of UI after link submission as temp patch Bug identified. Decided to ship what we had. See #53566 (comment) * Fix bug only allowing opens in new tab in advanced settings * Augment test to assert that checking box will apply changes * Implement spacing See #53566 (comment)
WordPress · Aug 15, 2023 · 3d01a3f · 3d01a3f
1 parent d600d2d
commit 3d01a3f
Show file tree

Hide file tree

Showing 5 changed files with 104 additions and 38 deletions.
diff --git a/packages/block-editor/src/components/link-control/index.js b/packages/block-editor/src/components/link-control/index.js
@@ -408,6 +408,29 @@ function LinkControl( {
 					onEditClick={ () => setIsEditingLink( true ) }
 					hasRichPreviews={ hasRichPreviews }
 					hasUnlinkControl={ shownUnlinkControl }
+					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 (
+								<LinkSettings
+									value={ internalControlValue }
+									settings={ settings?.filter(
+										( { id } ) => id === 'opensInNewTab'
+									) }
+									onChange={ ( { opensInNewTab } ) => {
+										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
@@ -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( {
 					) }
 				</div>
 			) }
+
+			{ additionalControls && additionalControls() }
 		</div>
 	);
 }
diff --git 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 {

diff --git 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 (
+				<LinkControl
+					value={ link }
+					settings={ customSettings }
+					onChange={ ( newVal ) => {
+						mockOnChange( newVal );
+						setLink( newVal );
+					} }
+				/>
+			);
+		};
+
+		render( <LinkControlConsumer /> );
+
+		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',
 			},
 			{

diff --git 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( {
@@ -185,6 +160,8 @@ function InlineLinkUI( {
 			}
 
 			newValue.start = newValue.end;
+
+			// Hides the Link UI.
 			newValue.activeFormats = [];
 			onChange( newValue );
 		}