Skip to content

Commit

Permalink
Add clear Apply and Cancel buttons to Link Control (#46933)
Browse files Browse the repository at this point in the history
* 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 <[email protected]>
  • Loading branch information
getdave and Ben Dwyer committed Jan 19, 2023
1 parent ea61b1b commit 875628f
Show file tree
Hide file tree
Showing 8 changed files with 205 additions and 49 deletions.
62 changes: 48 additions & 14 deletions packages/block-editor/src/components/link-control/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -113,6 +118,7 @@ function LinkControl( {
settings = DEFAULT_LINK_SETTINGS,
onChange = noop,
onRemove,
onCancel,
noDirectEntry = false,
showSuggestions = true,
showInitialSuggestions,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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>
</>
) }

Expand Down
28 changes: 6 additions & 22 deletions packages/block-editor/src/components/link-control/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {
Expand Down Expand Up @@ -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;
}
}

Expand Down
139 changes: 134 additions & 5 deletions packages/block-editor/src/components/link-control/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,7 @@ describe( 'Manual link entry', () => {
} );

let submitButton = screen.getByRole( 'button', {
name: 'Submit',
name: 'Apply',
} );

expect( submitButton ).toBeDisabled();
Expand All @@ -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.
Expand All @@ -578,7 +578,7 @@ describe( 'Manual link entry', () => {
} );

let submitButton = screen.queryByRole( 'button', {
name: 'Submit',
name: 'Apply',
} );

expect( submitButton ).toBeDisabled();
Expand All @@ -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.
Expand All @@ -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:[email protected]', 'mailto' ],
Expand Down Expand Up @@ -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 );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ describe( 'General media replace flow', () => {

await user.click(
screen.getByRole( 'button', {
name: 'Submit',
name: 'Apply',
} )
);

Expand Down
15 changes: 10 additions & 5 deletions packages/block-library/src/navigation-link/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 -->"
`;
Loading

1 comment on commit 875628f

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flaky tests detected in 875628f.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/3955960072
📝 Reported issues:

Please sign in to comment.