diff --git a/packages/block-editor/src/components/link-control/index.js b/packages/block-editor/src/components/link-control/index.js index 2d0f0078a9e76..7ffa6b327029a 100644 --- a/packages/block-editor/src/components/link-control/index.js +++ b/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 (
-
-
- + />
{ errorMessage && ( ) } + + + + ) } diff --git a/packages/block-editor/src/components/link-control/style.scss b/packages/block-editor/src/components/link-control/style.scss index a34d050353f94..51682a45c7c20 100644 --- a/packages/block-editor/src/components/link-control/style.scss +++ b/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; } } 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 86eae7289f4f6..bf48d93305fca 100644 --- a/packages/block-editor/src/components/link-control/test/index.js +++ b/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( ); + + // 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 ( + { + setLink( suggestion ); + } } + hasTextControl + /> + ); + }; + + render( ); + + 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( ); + + 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 ); diff --git a/packages/block-editor/src/components/media-replace-flow/test/index.js b/packages/block-editor/src/components/media-replace-flow/test/index.js index 2d3a9f64fc198..9d1aef6df7620 100644 --- a/packages/block-editor/src/components/media-replace-flow/test/index.js +++ b/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', } ) ); diff --git a/packages/block-library/src/navigation-link/edit.js b/packages/block-library/src/navigation-link/edit.js index 87e9cc38afc7c..47797fa0a9ffb 100644 --- a/packages/block-library/src/navigation-link/edit.js +++ b/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. diff --git a/packages/e2e-tests/specs/editor/various/__snapshots__/links.test.js.snap b/packages/e2e-tests/specs/editor/various/__snapshots__/links.test.js.snap index 330dfbbe142b0..6ba1f2c014885 100644 --- a/packages/e2e-tests/specs/editor/various/__snapshots__/links.test.js.snap +++ b/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`] = ` " -

This is WordPress

+

This is WordPress

" `; diff --git a/packages/e2e-tests/specs/editor/various/links.test.js b/packages/e2e-tests/specs/editor/various/links.test.js index c82e0c78c1dc9..44018bae3226f 100644 --- a/packages/e2e-tests/specs/editor/various/links.test.js +++ b/packages/e2e-tests/specs/editor/various/links.test.js @@ -122,6 +122,7 @@ describe( 'Links', () => { // Navigate to and toggle the "Open in new tab" checkbox. await page.keyboard.press( 'Tab' ); await page.keyboard.press( 'Tab' ); + await page.keyboard.press( 'Tab' ); await page.keyboard.press( 'Space' ); // Toggle should still have focus and be checked. @@ -135,6 +136,7 @@ describe( 'Links', () => { // Tab back to the Submit and apply the link. await pressKeyWithModifier( 'shift', 'Tab' ); + await pressKeyWithModifier( 'shift', 'Tab' ); await page.keyboard.press( 'Enter' ); // The link should have been inserted. @@ -528,6 +530,7 @@ describe( 'Links', () => { // Navigate to and toggle the "Open in new tab" checkbox. await page.keyboard.press( 'Tab' ); await page.keyboard.press( 'Tab' ); + await page.keyboard.press( 'Tab' ); await page.keyboard.press( 'Space' ); // Confirm that focus was not prematurely returned to the paragraph on @@ -766,6 +769,7 @@ describe( 'Links', () => { await page.keyboard.press( 'Tab' ); await page.keyboard.press( 'Tab' ); await page.keyboard.press( 'Tab' ); + await page.keyboard.press( 'Tab' ); // Make a selection within the RichText. await pressKeyWithModifier( 'shift', 'ArrowRight' ); diff --git a/test/e2e/specs/editor/blocks/image.spec.js b/test/e2e/specs/editor/blocks/image.spec.js index 209511d2d1cfa..26db3cb0d4254 100644 --- a/test/e2e/specs/editor/blocks/image.spec.js +++ b/test/e2e/specs/editor/blocks/image.spec.js @@ -486,7 +486,7 @@ test.describe( 'Image', () => { await page.click( 'role=button[name="Edit"i]' ); // Replace the url. await page.fill( 'role=combobox[name="URL"i]', imageUrl ); - await page.click( 'role=button[name="Submit"i]' ); + await page.click( 'role=button[name="Apply"i]' ); const regex = new RegExp( `