Skip to content

Commit

Permalink
Revert "Add clear Apply and Cancel buttons to Link Control (#46933)"
Browse files Browse the repository at this point in the history 
This reverts commit 875628f.

# Conflicts:
#	packages/block-editor/src/components/link-control/index.js
  • Loading branch information
@getdave @ntsekouras
getdave authored and ntsekouras committed Feb 23, 2023
1 parent 1bcbcf0 commit 009ff76
Show file tree
Hide file tree
Showing 8 changed files with 49 additions and 205 deletions.
62 changes: 14 additions & 48 deletions packages/block-editor/src/components/link-control/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,8 @@ import classnames from 'classnames';
/**
* WordPress dependencies
*/
import {
Button,
ButtonGroup,
Spinner,
Notice,
TextControl,
} from '@wordpress/components';
import { Button, Spinner, Notice, TextControl } from '@wordpress/components';
import { keyboardReturn } from '@wordpress/icons';
import { __ } from '@wordpress/i18n';
import { useRef, useState, useEffect } from '@wordpress/element';
import { focus } from '@wordpress/dom';
Expand Down Expand Up @@ -118,7 +113,6 @@ function LinkControl( {
settings = DEFAULT_LINK_SETTINGS,
onChange = noop,
onRemove,
onCancel,
noDirectEntry = false,
showSuggestions = true,
showInitialSuggestions,
Expand Down Expand Up @@ -196,8 +190,6 @@ 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 @@ -243,29 +235,6 @@ 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 @@ -278,7 +247,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 = hasLinkValue && hasTextControl;
const showTextControl = value?.url?.trim()?.length > 0 && hasTextControl;

return (
<div
Expand Down Expand Up @@ -330,7 +299,17 @@ 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 @@ -341,19 +320,6 @@ 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="tertiary" onClick={ handleCancel }>
{ __( 'Cancel' ) }
</Button>
</ButtonGroup>
</>
) }

Expand Down
28 changes: 22 additions & 6 deletions packages/block-editor/src/components/link-control/style.scss
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ $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 @@ -76,11 +77,20 @@ $preview-image-height: 140px;
}

.block-editor-link-control__search-actions {
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;
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);
}

.components-button .block-editor-link-control__search-submit .has-icon {
Expand Down Expand Up @@ -469,8 +479,14 @@ $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: $grid-unit-20;
right: $button-size;
}
}

Expand Down
139 changes: 5 additions & 134 deletions packages/block-editor/src/components/link-control/test/index.js
View file
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: 'Apply',
name: 'Submit',
} );

expect( submitButton ).toBeDisabled();
Expand All @@ -555,7 +555,7 @@ describe( 'Manual link entry', () => {
await user.keyboard( '[Enter]' );

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

// Verify the UI hasn't allowed submission.
Expand All @@ -578,7 +578,7 @@ describe( 'Manual link entry', () => {
} );

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

expect( submitButton ).toBeDisabled();
Expand All @@ -597,7 +597,7 @@ describe( 'Manual link entry', () => {
await user.click( submitButton );

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

// Verify the UI hasn't allowed submission.
Expand All @@ -608,135 +608,6 @@ 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' ],
Expand Down Expand Up @@ -1988,7 +1859,7 @@ describe( 'Controlling link title text', () => {
expect( textInput ).toHaveValue( textValue );

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

await user.click( submitButton );
Expand Down
2 changes: 1 addition & 1 deletion packages/block-editor/src/components/media-replace-flow/test/index.js
View file
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: 'Apply',
name: 'Submit',
} )
);

Expand Down
15 changes: 5 additions & 10 deletions packages/block-library/src/navigation-link/edit.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -315,17 +315,12 @@ 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: undefined,
label: undefined,
id: undefined,
kind: undefined,
type: undefined,
opensInNewTab: false,
url: '',
label: '',
id: '',
kind: '',
type: '',
} );

// Close the link editing UI.
Expand Down
2 changes: 1 addition & 1 deletion packages/e2e-tests/specs/editor/various/__snapshots__/links.test.js.snap
View file
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\\" target=\\"_blank\\" rel=\\"noreferrer noopener\\">WordPress</a></p>
<p>This is <a href=\\"http://wordpress.org\\">WordPress</a></p>
<!-- /wp:paragraph -->"
`;
Loading

0 comments on commit 009ff76

Please sign in to comment.