Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use consistent labels, remove additional settings, and copySmall icon LinkControl #58183

Merged
merged 11 commits into from
Feb 7, 2024
42 changes: 20 additions & 22 deletions packages/block-editor/src/components/link-control/link-preview.js
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>
getdave marked this conversation as resolved.
Show resolved Hide resolved
{ 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
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;
}
getdave marked this conversation as resolved.
Show resolved Hide resolved

&: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
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
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
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
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
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
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
Loading