Skip to content

Commit

Permalink
LinkControl - use consistent labels, remove additional settings, and …
Browse files Browse the repository at this point in the history 
…copySmall icon (#58183)

* Add copySmall icon

* Remove additionalControls + consistent labels

* remove test we don't need

* Remove classes/styles we don't need and tooltip, tidy snackbar

* Fix focus style

* Fix test for pattern overrides

* Fix e2e tests

* Add additionalControls back in. It can be removed in a separate PR if necessary.

* Rename Edit -> Edit link and Unlink -> Remove link to fix unit tests

* Add url to copy link button

* Use sprintf for copy link

---------

Co-authored-by: scruffian <ben@scruffian.com>
Co-authored-by: Jerry Jones <jones.jeremydavid@gmail.com>
Co-authored-by: Dave Smith <getdavemail@gmail.com>
  • Loading branch information
@richtabor @scruffian
@jeryj @getdave
4 people authored Feb 7, 2024
1 parent 5e683b7 commit bc3f367
Show file tree
Hide file tree
Showing 8 changed files with 60 additions and 126 deletions.
42 changes: 20 additions & 22 deletions packages/block-editor/src/components/link-control/link-preview.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,15 @@ import classnames from 'classnames';
/**
* WordPress dependencies
*/
import { __ } from '@wordpress/i18n';
import { __, sprintf } from '@wordpress/i18n';
import {
Button,
ExternalLink,
__experimentalTruncate as Truncate,
Tooltip,
} from '@wordpress/components';
import { useCopyToClipboard } from '@wordpress/compose';
import { filterURLForDisplay, safeDecodeURI } from '@wordpress/url';
import { Icon, globe, info, linkOff, edit, copy } from '@wordpress/icons';
import { Icon, globe, info, linkOff, edit, copySmall } from '@wordpress/icons';
import { __unstableStripHTML as stripHTML } from '@wordpress/dom';
import { useDispatch } from '@wordpress/data';
import { store as noticesStore } from '@wordpress/notices';
Expand Down Expand Up @@ -66,7 +65,7 @@ export default function LinkPreview( {

const { createNotice } = useDispatch( noticesStore );
const ref = useCopyToClipboard( value.url, () => {
createNotice( 'info', __( 'Copied URL to clipboard.' ), {
createNotice( 'info', __( 'Link copied to clipboard.' ), {
isDismissible: true,
type: 'snackbar',
} );
Expand Down Expand Up @@ -99,16 +98,14 @@ export default function LinkPreview( {
<span className="block-editor-link-control__search-item-details">
{ ! isEmptyURL ? (
<>
<Tooltip text={ value.url }>
<ExternalLink
className="block-editor-link-control__search-item-title"
href={ value.url }
>
<Truncate numberOfLines={ 1 }>
{ displayTitle }
</Truncate>
</ExternalLink>
</Tooltip>
<ExternalLink
className="block-editor-link-control__search-item-title"
href={ value.url }
>
<Truncate numberOfLines={ 1 }>
{ displayTitle }
</Truncate>
</ExternalLink>
{ value?.url && displayTitle !== displayURL && (
<span className="block-editor-link-control__search-item-info">
<Truncate numberOfLines={ 1 }>
Expand All @@ -124,27 +121,28 @@ export default function LinkPreview( {
) }
</span>
</span>

<Button
icon={ edit }
label={ __( 'Edit' ) }
className="block-editor-link-control__search-item-action"
label={ __( 'Edit link' ) }
onClick={ onEditClick }
size="compact"
/>
{ hasUnlinkControl && (
<Button
icon={ linkOff }
label={ __( 'Unlink' ) }
className="block-editor-link-control__search-item-action block-editor-link-control__unlink"
label={ __( 'Remove link' ) }
onClick={ onRemove }
size="compact"
/>
) }
<Button
icon={ copy }
label={ __( 'Copy URL' ) }
className="block-editor-link-control__search-item-action block-editor-link-control__copy"
icon={ copySmall }
label={ sprintf(
// Translators: %1$s is a placeholder for an optional colon, %2$s is a placeholder for the link URL (if present).
__( 'Copy link%1$s%2$s' ), // Ends up looking like "Copy link: https://example.com".
isEmptyURL ? '' : ': ',
value.url
) }
ref={ ref }
disabled={ isEmptyURL }
size="compact"
Expand Down
14 changes: 4 additions & 10 deletions packages/block-editor/src/components/link-control/style.scss
View file
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,10 @@ $block-editor-link-control-number-of-actions: 1;
border-radius: $radius-block-ui;
line-height: 1.1;

&:focus {
box-shadow: none;
}

&:focus-visible {
@include block-toolbar-button-style__focus();
text-decoration: none;
Expand Down Expand Up @@ -352,11 +356,6 @@ $block-editor-link-control-number-of-actions: 1;
position: relative;
}

.block-editor-link-control__unlink {
padding-left: $grid-unit-20;
padding-right: $grid-unit-20;
}

.block-editor-link-control__setting {
margin-bottom: 0;
flex: 1;
Expand Down Expand Up @@ -425,8 +424,3 @@ $block-editor-link-control-number-of-actions: 1;
top: calc(50% + #{$spinner-size} / 4); // Add top spacing because this input has a visual label.
right: $grid-unit-15;
}

.block-editor-link-control__search-item-action {
margin-left: auto; // push to far right hand side
flex-shrink: 0;
}
101 changes: 11 additions & 90 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 @@ -321,7 +321,7 @@ describe( 'Basic rendering', () => {

// Click the "Edit" button to trigger into the editing mode.
const editButton = screen.queryByRole( 'button', {
name: 'Edit',
name: 'Edit link',
} );

await user.click( editButton );
Expand Down Expand Up @@ -379,7 +379,7 @@ describe( 'Basic rendering', () => {
render( <LinkControl value={ { url: 'https://example.com' } } /> );

const unLinkButton = screen.queryByRole( 'button', {
name: 'Unlink',
name: 'Remove link',
} );

expect( unLinkButton ).not.toBeInTheDocument();
Expand All @@ -397,7 +397,7 @@ describe( 'Basic rendering', () => {
);

const unLinkButton = screen.queryByRole( 'button', {
name: 'Unlink',
name: 'Remove link',
} );
expect( unLinkButton ).toBeVisible();

Expand All @@ -418,7 +418,7 @@ describe( 'Basic rendering', () => {
);

const unLinkButton = screen.queryByRole( 'button', {
name: 'Unlink',
name: 'Remove link',
} );
expect( unLinkButton ).toBeVisible();

Expand Down Expand Up @@ -822,7 +822,7 @@ describe( 'Manual link entry', () => {

// Click the "Edit" button to trigger into the editing mode.
let editButton = screen.queryByRole( 'button', {
name: 'Edit',
name: 'Edit link',
} );

await user.click( editButton );
Expand Down Expand Up @@ -856,7 +856,7 @@ describe( 'Manual link entry', () => {

// Re-query the edit button as it's been replaced.
editButton = screen.queryByRole( 'button', {
name: 'Edit',
name: 'Edit link',
} );

await user.click( editButton );
Expand Down Expand Up @@ -1060,7 +1060,7 @@ describe( 'Default search suggestions', () => {
// shown.
const currentLinkUI = screen.getByLabelText( 'Currently selected' );
const currentLinkBtn = within( currentLinkUI ).getByRole( 'button', {
name: 'Edit',
name: 'Edit link',
} );
await user.click( currentLinkBtn );

Expand Down Expand Up @@ -1502,7 +1502,7 @@ describe( 'Selecting links', () => {

expect( currentLink ).toBeVisible();
expect(
screen.queryByRole( 'button', { name: 'Edit' } )
screen.queryByRole( 'button', { name: 'Edit link' } )
).toBeVisible();
expect( currentLinkAnchor ).toBeVisible();
} );
Expand All @@ -1527,7 +1527,7 @@ describe( 'Selecting links', () => {
// Required in order to select the button below.
let currentLinkUI = screen.getByLabelText( 'Currently selected' );
const currentLinkBtn = within( currentLinkUI ).getByRole( 'button', {
name: 'Edit',
name: 'Edit link',
} );

// Simulate searching for a term.
Expand Down Expand Up @@ -1597,7 +1597,7 @@ describe( 'Selecting links', () => {

// Check that this suggestion is now shown as selected.
expect(
screen.getByRole( 'button', { name: 'Edit' } )
screen.getByRole( 'button', { name: 'Edit link' } )
).toBeVisible();
expect( currentLinkAnchor ).toBeVisible();
}
Expand Down Expand Up @@ -1709,7 +1709,7 @@ describe( 'Selecting links', () => {

expect( currentLink ).toBeVisible();
expect(
screen.getByRole( 'button', { name: 'Edit' } )
screen.getByRole( 'button', { name: 'Edit link' } )
).toBeVisible();
expect( currentLinkAnchor ).toBeVisible();
}
Expand Down Expand Up @@ -1816,66 +1816,6 @@ describe( 'Selecting links', () => {
} );

describe( 'Addition Settings UI', () => {
it( 'should allow toggling the "Opens in new tab" setting control (only) on the link preview', async () => {
const user = userEvent.setup();
const selectedLink = fauxEntitySuggestions[ 0 ];
const mockOnChange = jest.fn();

const customSettings = [
{
id: 'opensInNewTab',
title: 'Open in new tab',
},
{
id: 'noFollow',
title: 'No follow',
},
];

const LinkControlConsumer = () => {
const [ link, setLink ] = useState( selectedLink );

return (
<LinkControl
value={ link }
settings={ customSettings }
onChange={ ( newVal ) => {
mockOnChange( newVal );
setLink( newVal );
} }
/>
);
};

render( <LinkControlConsumer /> );

const opensInNewTabField = screen.queryByRole( 'checkbox', {
name: 'Open in new tab',
checked: false,
} );

expect( opensInNewTabField ).toBeInTheDocument();

// No matter which settings are passed in only the `Opens in new tab`
// setting should be shown on the link preview (non-editing) state.
const noFollowField = screen.queryByRole( 'checkbox', {
name: 'No follow',
} );
expect( noFollowField ).not.toBeInTheDocument();

// Check that the link value is updated immediately upon checking
// the checkbox.
await user.click( opensInNewTabField );

expect( opensInNewTabField ).toBeChecked();

expect( mockOnChange ).toHaveBeenCalledTimes( 1 );
expect( mockOnChange ).toHaveBeenCalledWith( {
...selectedLink,
opensInNewTab: true,
} );
} );

it( 'should hide advanced link settings and toggle when not editing a link', async () => {
const selectedLink = fauxEntitySuggestions[ 0 ];

Expand Down Expand Up @@ -2056,25 +1996,6 @@ describe( 'Addition Settings UI', () => {
} )
);
} );

it( 'should show tooltip with full URL alongside filtered display', async () => {
const user = userEvent.setup();
const url =
'http://www.wordpress.org/wp-content/uploads/a-document.pdf';
render( <LinkControl value={ { url } } /> );

const link = screen.getByRole( 'link' );

expect( link ).toHaveTextContent( 'a-document.pdf' );

await user.hover( link );

expect( await screen.findByRole( 'tooltip' ) ).toHaveTextContent( url );

await user.unhover( link );

expect( screen.queryByRole( 'tooltip' ) ).not.toBeInTheDocument();
} );
} );

describe( 'Post types', () => {
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 @@ -105,7 +105,7 @@ describe( 'General media replace flow', () => {

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

Expand Down
1 change: 1 addition & 0 deletions packages/icons/src/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ export { default as color } from './library/color';
export { default as column } from './library/column';
export { default as columns } from './library/columns';
export { default as copy } from './library/copy';
export { default as copySmall } from './library/copy-small';
export { default as comment } from './library/comment';
export { default as commentAuthorAvatar } from './library/comment-author-avatar';
export { default as commentAuthorName } from './library/comment-author-name';
Expand Down
16 changes: 16 additions & 0 deletions packages/icons/src/library/copy-small.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/**
* WordPress dependencies
*/
import { SVG, Path } from '@wordpress/primitives';

const copySmall = (
<SVG xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24">
<Path
fillRule="evenodd"
clipRule="evenodd"
d="M5.625 5.5h9.75c.069 0 .125.056.125.125v9.75a.125.125 0 0 1-.125.125h-9.75a.125.125 0 0 1-.125-.125v-9.75c0-.069.056-.125.125-.125ZM4 5.625C4 4.728 4.728 4 5.625 4h9.75C16.273 4 17 4.728 17 5.625v9.75c0 .898-.727 1.625-1.625 1.625h-9.75A1.625 1.625 0 0 1 4 15.375v-9.75Zm14.5 11.656v-9H20v9C20 18.8 18.77 20 17.251 20H6.25v-1.5h11.001c.69 0 1.249-.528 1.249-1.219Z"
/>
</SVG>
);

export default copySmall;
8 changes: 6 additions & 2 deletions test/e2e/specs/editor/blocks/links.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,9 @@ test.describe( 'Links', () => {
await LinkUtils.createLink();

// Click on the Edit button.
await page.getByRole( 'button', { name: 'Edit', exact: true } ).click();
await page
.getByRole( 'button', { name: 'Edit link', exact: true } )
.click();

// Change the URL.
// getByPlaceholder required in order to handle Link Control component
Expand All @@ -255,7 +257,9 @@ test.describe( 'Links', () => {

const linkPopover = LinkUtils.getLinkPopover();

await linkPopover.getByRole( 'button', { name: 'Unlink' } ).click();
await linkPopover
.getByRole( 'button', { name: 'Remove link' } )
.click();

// The link should have been removed.
await expect.poll( editor.getBlocks ).toMatchObject( [
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/specs/editor/various/pattern-overrides.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ test.describe( 'Pattern Overrides', () => {
exact: true,
} );
const editLinkButton = page.getByRole( 'button', {
name: 'Edit',
name: 'Edit link',
exact: true,
} );
const saveLinkButton = page.getByRole( 'button', {
Expand Down

1 comment on commit bc3f367

@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 bc3f367.
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/7817954103
📝 Reported issues:

Please sign in to comment.