From 0d3c2e32a4e2e8b3ad2117ecb7dd5545ec7e3649 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Wed, 9 Jun 2021 11:17:29 +0100 Subject: [PATCH 1/7] Adds initial Unlink button --- .../src/components/link-control/index.js | 24 +++++++++++++++---- .../src/components/link-control/style.scss | 15 +++++++++++- packages/format-library/src/link/inline.js | 10 ++++++++ 3 files changed, 43 insertions(+), 6 deletions(-) diff --git a/packages/block-editor/src/components/link-control/index.js b/packages/block-editor/src/components/link-control/index.js index b9a63ceb71da3c..e8332e10231f0d 100644 --- a/packages/block-editor/src/components/link-control/index.js +++ b/packages/block-editor/src/components/link-control/index.js @@ -260,11 +260,25 @@ function LinkControl( { /> ) } - +
+ + { value && ! isEditingLink && ! isCreatingPage && ( + + ) } +
); } diff --git a/packages/block-editor/src/components/link-control/style.scss b/packages/block-editor/src/components/link-control/style.scss index b1b6958dbe5f8c..3d5d5941bb7749 100644 --- a/packages/block-editor/src/components/link-control/style.scss +++ b/packages/block-editor/src/components/link-control/style.scss @@ -380,10 +380,23 @@ $preview-image-height: 140px; padding: 10px; } -.block-editor-link-control__settings { +.block-editor-link-control__tools { + display: flex; + align-items: center; border-top: $border-width solid $gray-300; margin: 0; padding: $grid-unit-20 $grid-unit-30; +} + +.block-editor-link-control__unlink { + padding-left: $grid-unit-20; + padding-right: $grid-unit-20; +} + +.block-editor-link-control__settings { + flex: 1; + margin: 0; + :last-child { margin-bottom: 0; diff --git a/packages/format-library/src/link/inline.js b/packages/format-library/src/link/inline.js index d015267a8e38f7..c0098b1393758d 100644 --- a/packages/format-library/src/link/inline.js +++ b/packages/format-library/src/link/inline.js @@ -11,6 +11,7 @@ import { isCollapsed, applyFormat, useAnchorRef, + removeFormat, } from '@wordpress/rich-text'; import { __experimentalLinkControl as LinkControl } from '@wordpress/block-editor'; @@ -49,6 +50,15 @@ function InlineLinkUI( { }; function onChangeLink( nextValue ) { + // null values trigger removal of link format. + if ( null === nextValue ) { + const newValue = removeFormat( value, 'core/link' ); + onChange( newValue ); + stopAddingLink(); + speak( __( 'Link removed.' ), 'assertive' ); + return; + } + // 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. From cd648452de723d085d0525458e680b8e87611811 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Tue, 29 Jun 2021 11:23:58 +0100 Subject: [PATCH 2/7] Add dedicated opt in prop for removing links --- .../src/components/link-control/index.js | 29 ++++++++++--------- packages/format-library/src/link/inline.js | 15 ++++++---- 2 files changed, 25 insertions(+), 19 deletions(-) diff --git a/packages/block-editor/src/components/link-control/index.js b/packages/block-editor/src/components/link-control/index.js index e8332e10231f0d..df1a35e23e6fe5 100644 --- a/packages/block-editor/src/components/link-control/index.js +++ b/packages/block-editor/src/components/link-control/index.js @@ -1,7 +1,7 @@ /** * External dependencies */ -import { noop } from 'lodash'; +import { noop, isFunction } from 'lodash'; /** * WordPress dependencies @@ -106,6 +106,7 @@ function LinkControl( { value, settings, onChange = noop, + onRemove, noDirectEntry = false, showSuggestions = true, showInitialSuggestions, @@ -266,18 +267,20 @@ function LinkControl( { settings={ settings } onChange={ onChange } /> - { value && ! isEditingLink && ! isCreatingPage && ( - - ) } + { onRemove && + isFunction( onRemove ) && + value && + ! isEditingLink && + ! isCreatingPage && ( + + ) } ); diff --git a/packages/format-library/src/link/inline.js b/packages/format-library/src/link/inline.js index c0098b1393758d..ffd2820c5a27f6 100644 --- a/packages/format-library/src/link/inline.js +++ b/packages/format-library/src/link/inline.js @@ -49,14 +49,16 @@ function InlineLinkUI( { ...nextLinkValue, }; + function removeLink() { + const newValue = removeFormat( value, 'core/link' ); + onChange( newValue ); + stopAddingLink(); + speak( __( 'Link removed.' ), 'assertive' ); + } + function onChangeLink( nextValue ) { - // null values trigger removal of link format. if ( null === nextValue ) { - const newValue = removeFormat( value, 'core/link' ); - onChange( newValue ); - stopAddingLink(); - speak( __( 'Link removed.' ), 'assertive' ); - return; + return removeLink(); } // Merge with values from state, both for the purpose of assigning the @@ -149,6 +151,7 @@ function InlineLinkUI( { From 478c2aefd8c7bface26d5bfab8abcbb29cf4e8c3 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Tue, 29 Jun 2021 11:24:34 +0100 Subject: [PATCH 3/7] Add tests --- .../src/components/link-control/test/index.js | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) 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 27e1665d869ff2..7f445f70c76f6e 100644 --- a/packages/block-editor/src/components/link-control/test/index.js +++ b/packages/block-editor/src/components/link-control/test/index.js @@ -227,6 +227,43 @@ describe( 'Basic rendering', () => { expect( isEditing() ).toBe( false ); } ); } ); + + describe( 'Unlinking', () => { + it( 'should not show "Unlink" button if no onRemove handler is provided', () => { + act( () => { + render( + , + container + ); + } ); + + const unLinkButton = queryByText( container, 'Unlink' ); + + expect( unLinkButton ).toBeNull(); + } ); + + it( 'should show "Unlink" button if a onRemove handler is provided', () => { + const mockOnRemove = jest.fn(); + act( () => { + render( + , + container + ); + } ); + + const unLinkButton = queryByText( container, 'Unlink' ); + expect( unLinkButton ).toBeTruthy(); + + act( () => { + Simulate.click( unLinkButton ); + } ); + + expect( mockOnRemove ).toHaveBeenCalled(); + } ); + } ); } ); describe( 'Searching for a link', () => { From 550b4f01bfd41a6118e908d71f061b06213de596 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Tue, 29 Jun 2021 11:34:12 +0100 Subject: [PATCH 4/7] Update docs --- .../block-editor/src/components/link-control/README.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/block-editor/src/components/link-control/README.md b/packages/block-editor/src/components/link-control/README.md index d143b7b5fcb3ed..45da4a723c07c2 100644 --- a/packages/block-editor/src/components/link-control/README.md +++ b/packages/block-editor/src/components/link-control/README.md @@ -127,6 +127,14 @@ This `suggestion` will then be _automatically_ passed to the `onChange` handler As a result of the above, this prop is often used to allow on the fly creation of new entities (eg: `Posts`, `Pages`) based on the text the user has entered into the link search UI. As an example, the Navigation Block uses `createSuggestion` to create Pages on the fly from within the Block itself. +### onRemove + +- Type: `Function` +- Required: No +- Default: null + +An optional handler, which when passed will trigger the display of an `Unlink` UI within the control. This handler is expected to remove the current `value` of the control thus resetting it back to a default state. The key use case for this is allowing users to remove a link from the control without relying on there being an "unlink" control in the block toolbar. + #### Search `suggestion` values A `suggestion` should have the following shape: From 441744ec806992707a6df3ab6341c6fcb61ecb81 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Tue, 29 Jun 2021 12:42:53 +0100 Subject: [PATCH 5/7] Focus PR by removing unneeded onChange handler change --- packages/format-library/src/link/inline.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/format-library/src/link/inline.js b/packages/format-library/src/link/inline.js index ffd2820c5a27f6..8a9e7efae2fa1e 100644 --- a/packages/format-library/src/link/inline.js +++ b/packages/format-library/src/link/inline.js @@ -57,10 +57,6 @@ function InlineLinkUI( { } function onChangeLink( nextValue ) { - if ( null === nextValue ) { - return removeLink(); - } - // 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. From fde237c8bede31f02b0748f276ae783860dcba2e Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Tue, 29 Jun 2021 12:47:21 +0100 Subject: [PATCH 6/7] Use more accessible query for button Resolves https://github.com/WordPress/gutenberg/pull/32541#discussion_r660516876 --- .../src/components/link-control/test/index.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) 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 7f445f70c76f6e..250e4656d3708d 100644 --- a/packages/block-editor/src/components/link-control/test/index.js +++ b/packages/block-editor/src/components/link-control/test/index.js @@ -237,9 +237,12 @@ describe( 'Basic rendering', () => { ); } ); - const unLinkButton = queryByText( container, 'Unlink' ); + const unLinkButton = queryByRole( container, 'button', { + name: 'Unlink', + } ); expect( unLinkButton ).toBeNull(); + expect( unLinkButton ).not.toBeInTheDocument(); } ); it( 'should show "Unlink" button if a onRemove handler is provided', () => { @@ -254,8 +257,11 @@ describe( 'Basic rendering', () => { ); } ); - const unLinkButton = queryByText( container, 'Unlink' ); + const unLinkButton = queryByRole( container, 'button', { + name: 'Unlink', + } ); expect( unLinkButton ).toBeTruthy(); + expect( unLinkButton ).toBeInTheDocument(); act( () => { Simulate.click( unLinkButton ); From 545ace82352fe0532557274cc26ceef9903d9363 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Wed, 30 Jun 2021 12:06:35 +0100 Subject: [PATCH 7/7] Remove checking if onRemove is a function. --- .../src/components/link-control/index.js | 26 ++++++++----------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/packages/block-editor/src/components/link-control/index.js b/packages/block-editor/src/components/link-control/index.js index df1a35e23e6fe5..db6b005ac79fab 100644 --- a/packages/block-editor/src/components/link-control/index.js +++ b/packages/block-editor/src/components/link-control/index.js @@ -1,7 +1,7 @@ /** * External dependencies */ -import { noop, isFunction } from 'lodash'; +import { noop } from 'lodash'; /** * WordPress dependencies @@ -267,20 +267,16 @@ function LinkControl( { settings={ settings } onChange={ onChange } /> - { onRemove && - isFunction( onRemove ) && - value && - ! isEditingLink && - ! isCreatingPage && ( - - ) } + { onRemove && value && ! isEditingLink && ! isCreatingPage && ( + + ) } );