Add clear Apply and Cancel buttons to Link Control (#46933)

* Add initial Apply and Cancel buttons and refactor styles

* Add improved cancellation logic

* Update tests

* Add unit tests for Cancellation

* Fix Media flow tests

* Add ability to supply callback to be triggered on cancelling

* Fix input padding and spinner positioning

* Add test coverage to check optional onCancel is not called if undefined

* Update button text string in e2e test

* Fix tab stops in test

* Don't propgate the click back to the parent blocks

* Fix more tab stops in tests

* Fix another test tab stops

* Fix snapshot missing open in new tab attributes

* reset the URL to undefined rather than an empty string so that the popover doesn't reopen

* Ensure correctly resetting all “link” attributes to true default state

Co-authored-by: Ben Dwyer <ben@escruffian.com>

packages/block-editor/src/components/link-control/index.js

@@ -6,8 +6,13 @@ import classnames from 'classnames';
 /**
  * WordPress dependencies
  */
-import { Button, Spinner, Notice, TextControl } from '@wordpress/components';
-import { keyboardReturn } from '@wordpress/icons';
+import {
+	Button,
+	ButtonGroup,
+	Spinner,
+	Notice,
+	TextControl,
+} from '@wordpress/components';
 import { __ } from '@wordpress/i18n';
 import { useRef, useState, useEffect } from '@wordpress/element';
 import { focus } from '@wordpress/dom';
@@ -113,6 +118,7 @@ function LinkControl( {
 	settings = DEFAULT_LINK_SETTINGS,
 	onChange = noop,
 	onRemove,
+	onCancel,
 	noDirectEntry = false,
 	showSuggestions = true,
 	showInitialSuggestions,
@@ -190,6 +196,8 @@ function LinkControl( {
 		isEndingEditWithFocus.current = false;
 	}, [ isEditingLink, isCreatingPage ] );
 
+	const hasLinkValue = value?.url?.trim()?.length > 0;
+
 	/**
 	 * Cancels editing state and marks that focus may need to be restored after
 	 * the next render, if focus was within the wrapper when editing finished.
@@ -235,6 +243,29 @@ function LinkControl( {
 		}
 	};
 
+	const resetInternalValues = () => {
+		setInternalUrlInputValue( value?.url );
+		setInternalTextInputValue( value?.title );
+	};
+
+	const handleCancel = ( event ) => {
+		event.preventDefault();
+		event.stopPropagation();
+
+		// Ensure that any unsubmitted input changes are reset.
+		resetInternalValues();
+
+		if ( hasLinkValue ) {
+			// If there is a link then exist editing mode and show preview.
+			stopEditing();
+		} else {
+			// If there is no link value, then remove the link entirely.
+			onRemove?.();
+		}
+
+		onCancel?.();
+	};
+
 	const currentUrlInputValue = propInputValue || internalUrlInputValue;
 
 	const currentInputIsEmpty = ! currentUrlInputValue?.trim()?.length;
@@ -247,7 +278,7 @@ function LinkControl( {
 	// Only show text control once a URL value has been committed
 	// and it isn't just empty whitespace.
 	// See https://github.com/WordPress/gutenberg/pull/33849/#issuecomment-932194927.
-	const showTextControl = value?.url?.trim()?.length > 0 && hasTextControl;
+	const showTextControl = hasLinkValue && hasTextControl;
 
 	return (
 		<div
@@ -299,17 +330,7 @@ function LinkControl( {
 								createSuggestionButtonText
 							}
 							useLabel={ showTextControl }
-						>
-							<div className="block-editor-link-control__search-actions">
-								<Button
-									onClick={ handleSubmit }
-									label={ __( 'Submit' ) }
-									icon={ keyboardReturn }
-									className="block-editor-link-control__search-submit"
-									disabled={ currentInputIsEmpty } // Disallow submitting empty values.
-								/>
-							</div>
-						</LinkControlSearchInput>
+						/>
 					</div>
 					{ errorMessage && (
 						<Notice
@@ -320,6 +341,19 @@ function LinkControl( {
 							{ errorMessage }
 						</Notice>
 					) }
+					<ButtonGroup className="block-editor-link-control__search-actions">
+						<Button
+							variant="primary"
+							onClick={ handleSubmit }
+							className="xblock-editor-link-control__search-submit"
+							disabled={ currentInputIsEmpty } // Disallow submitting empty values.
+						>
+							{ __( 'Apply' ) }
+						</Button>
+						<Button variant="secondary" onClick={ handleCancel }>
+							{ __( 'Cancel' ) }
+						</Button>
+					</ButtonGroup>
 				</>
 			) }
 

packages/block-editor/src/components/link-control/style.scss

@@ -64,7 +64,6 @@ $preview-image-height: 140px;
 		width: calc(100% - #{$grid-unit-20 * 2});
 		display: block;
 		padding: 11px $grid-unit-20;
-		padding-right: ( $button-size * $block-editor-link-control-number-of-actions ); // width of reset and submit buttons
 		margin: 0;
 		position: relative;
 		border: 1px solid $gray-300;
@@ -77,20 +76,11 @@ $preview-image-height: 140px;
 }
 
 .block-editor-link-control__search-actions {
-	position: absolute;
-	/*
-	 * Actions must be positioned on top of URLInput, since the input will grow
-	 * when suggestions are rendered.
-	 *
-	 * Compensate for:
-	 *  - Border (1px)
-	 *  - Vertically, for the difference in height between the input (40px) and
-	 *    the icon buttons.
-	 *  - Horizontally, pad to the minimum of: default input padding, or the
-	 *    equivalent of the vertical padding.
-	 */
-	top: 1px + ( ( 40px - $button-size ) * 0.5 );
-	right: $grid-unit-20 + 1px + min($grid-unit-10, ( 40px - $button-size ) * 0.5);
+	display: flex;
+	flex-direction: row-reverse; // put "Cancel" on the left but retain DOM order.
+	justify-content: flex-start;
+	margin: $grid-unit-20; // allow margin collapse for vertical spacing.
+	gap: $grid-unit-10;
 }
 
 .components-button .block-editor-link-control__search-submit .has-icon {
@@ -479,14 +469,8 @@ $preview-image-height: 140px;
 		position: absolute;
 		left: auto;
 		bottom: auto;
-		/*
-		 * Position spinner to the left of the actions.
-		 *
-		 * Compensate for:
-		 *  - Input padding right ($button-size)
-		 */
 		top: calc(50% - #{$spinner-size} / 2);
-		right: $button-size;
+		right: $grid-unit-20;
 	}
 }
 

packages/block-editor/src/components/link-control/test/index.js

@@ -537,7 +537,7 @@ describe( 'Manual link entry', () => {
 				} );
 
 				let submitButton = screen.getByRole( 'button', {
-					name: 'Submit',
+					name: 'Apply',
 				} );
 
 				expect( submitButton ).toBeDisabled();
@@ -555,7 +555,7 @@ describe( 'Manual link entry', () => {
 				await user.keyboard( '[Enter]' );
 
 				submitButton = screen.getByRole( 'button', {
-					name: 'Submit',
+					name: 'Apply',
 				} );
 
 				// Verify the UI hasn't allowed submission.
@@ -578,7 +578,7 @@ describe( 'Manual link entry', () => {
 				} );
 
 				let submitButton = screen.queryByRole( 'button', {
-					name: 'Submit',
+					name: 'Apply',
 				} );
 
 				expect( submitButton ).toBeDisabled();
@@ -597,7 +597,7 @@ describe( 'Manual link entry', () => {
 				await user.click( submitButton );
 
 				submitButton = screen.queryByRole( 'button', {
-					name: 'Submit',
+					name: 'Apply',
 				} );
 
 				// Verify the UI hasn't allowed submission.
@@ -608,6 +608,135 @@ describe( 'Manual link entry', () => {
 		);
 	} );
 
+	describe( 'Handling cancellation', () => {
+		it( 'should allow cancellation of the link creation process and reset any entered values', async () => {
+			const user = userEvent.setup();
+			const mockOnRemove = jest.fn();
+			const mockOnCancel = jest.fn();
+
+			render( <LinkControl onRemove={ mockOnRemove } /> );
+
+			// Search Input UI.
+			const searchInput = screen.getByRole( 'combobox', {
+				name: 'URL',
+			} );
+
+			const cancelButton = screen.queryByRole( 'button', {
+				name: 'Cancel',
+			} );
+
+			expect( cancelButton ).toBeEnabled();
+			expect( cancelButton ).toBeVisible();
+
+			// Simulate adding a link for a term.
+			await user.type( searchInput, 'https://www.wordpress.org' );
+
+			// Attempt to submit the empty search value in the input.
+			await user.click( cancelButton );
+
+			// Verify the consumer can handle the cancellation.
+			expect( mockOnRemove ).toHaveBeenCalled();
+
+			// Ensure optional callback is not called.
+			expect( mockOnCancel ).not.toHaveBeenCalled();
+
+			expect( searchInput ).toHaveValue( '' );
+		} );
+
+		it( 'should allow cancellation of the link editing process and reset any entered values', async () => {
+			const user = userEvent.setup();
+			const initialLink = fauxEntitySuggestions[ 0 ];
+
+			const LinkControlConsumer = () => {
+				const [ link, setLink ] = useState( initialLink );
+
+				return (
+					<LinkControl
+						value={ link }
+						onChange={ ( suggestion ) => {
+							setLink( suggestion );
+						} }
+						hasTextControl
+					/>
+				);
+			};
+
+			render( <LinkControlConsumer /> );
+
+			let linkPreview = screen.getByLabelText( 'Currently selected' );
+
+			expect( linkPreview ).toBeInTheDocument();
+
+			// Click the "Edit" button to trigger into the editing mode.
+			let editButton = screen.queryByRole( 'button', {
+				name: 'Edit',
+			} );
+
+			await user.click( editButton );
+
+			let searchInput = screen.getByRole( 'combobox', {
+				name: 'URL',
+			} );
+
+			let textInput = screen.getByRole( 'textbox', {
+				name: 'Text',
+			} );
+
+			// Make a change to the search input.
+			await user.type( searchInput, 'This URL value was changed!' );
+
+			// Make a change to the text input.
+			await user.type( textInput, 'This text value was changed!' );
+
+			const cancelButton = screen.queryByRole( 'button', {
+				name: 'Cancel',
+			} );
+
+			// Cancel the editing process.
+			await user.click( cancelButton );
+
+			linkPreview = screen.getByLabelText( 'Currently selected' );
+
+			expect( linkPreview ).toBeInTheDocument();
+
+			// Re-query the edit button as it's been replaced.
+			editButton = screen.queryByRole( 'button', {
+				name: 'Edit',
+			} );
+
+			await user.click( editButton );
+
+			// Re-query the inputs as they have been replaced.
+			searchInput = screen.getByRole( 'combobox', {
+				name: 'URL',
+			} );
+
+			textInput = screen.getByRole( 'textbox', {
+				name: 'Text',
+			} );
+
+			// Expect to see the original link values and **not** the changed values.
+			expect( searchInput ).toHaveValue( initialLink.url );
+			expect( textInput ).toHaveValue( initialLink.text );
+		} );
+
+		it( 'should call onCancel callback when cancelling if provided', async () => {
+			const user = userEvent.setup();
+			const mockOnCancel = jest.fn();
+
+			render( <LinkControl onCancel={ mockOnCancel } /> );
+
+			const cancelButton = screen.queryByRole( 'button', {
+				name: 'Cancel',
+			} );
+
+			await user.click( cancelButton );
+
+			// Verify the consumer can handle the cancellation.
+			expect( mockOnCancel ).toHaveBeenCalled();
+		} );
+	} );
+
 	describe( 'Alternative link protocols and formats', () => {
 		it.each( [
 			[ 'mailto:example123456@wordpress.org', 'mailto' ],
@@ -1859,7 +1988,7 @@ describe( 'Controlling link title text', () => {
 		expect( textInput ).toHaveValue( textValue );
 
 		const submitButton = screen.queryByRole( 'button', {
-			name: 'Submit',
+			name: 'Apply',
 		} );
 
 		await user.click( submitButton );

packages/block-editor/src/components/media-replace-flow/test/index.js

@@ -137,7 +137,7 @@ describe( 'General media replace flow', () => {
 
 		await user.click(
 			screen.getByRole( 'button', {
-				name: 'Submit',
+				name: 'Apply',
 			} )
 		);
 

packages/block-library/src/navigation-link/edit.js

@@ -312,12 +312,17 @@ export default function NavigationLinkEdit( {
 	 */
 	function removeLink() {
 		// Reset all attributes that comprise the link.
+		// It is critical that all attributes are reset
+		// to their default values otherwise this may
+		// in advertently trigger side effects because
+		// the values will have "changed".
 		setAttributes( {
-			url: '',
-			label: '',
-			id: '',
-			kind: '',
-			type: '',
+			url: undefined,
+			label: undefined,
+			id: undefined,
+			kind: undefined,
+			type: undefined,
+			opensInNewTab: false,
 		} );
 
 		// Close the link editing UI.

packages/e2e-tests/specs/editor/various/__snapshots__/links.test.js.snap

@@ -68,6 +68,6 @@ exports[`Links should contain a label when it should open in a new tab 1`] = `
 
 exports[`Links should contain a label when it should open in a new tab 2`] = `
 "<!-- wp:paragraph -->
-<p>This is <a href=\\"http://wordpress.org\\">WordPress</a></p>
+<p>This is <a href=\\"http://wordpress.org\\" target=\\"_blank\\" rel=\\"noreferrer noopener\\">WordPress</a></p>
 <!-- /wp:paragraph -->"
 `;