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.

Sign up for GitHub

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

Only allow links to be created for selected text and fix CMD + K shortcut #58179

Closed
wants to merge 2 commits into from
Closed

Only allow links to be created for selected text and fix CMD + K shortcut #58179

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5775038
Correct conditional
getdave Feb 7, 2024
8bba99b
Update e2e tests to match new behaviour
getdave Feb 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion packages/format-library/src/link/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ function Edit( {
// We only need to store the button element that opened the popover. We can ignore the other states, as they will be handled by the onFocus prop to return to the rich text field.
const [ openedBy, setOpenedBy ] = useState( null );

const canMakeLink = ! isCollapsed( value ) || addingLink || isActive;

useLayoutEffect( () => {
const editableContentElement = contentRef.current;
if ( ! editableContentElement ) {
Expand Down Expand Up @@ -138,7 +140,13 @@ function Edit( {

return (
<>
<RichTextShortcut type="primary" character="k" onUse={ addLink } />
{ canMakeLink && (
<RichTextShortcut
type="primary"
character="k"
onUse={ addLink }
/>
) }
<RichTextShortcut
type="primaryShift"
character="k"
Expand All @@ -156,6 +164,7 @@ function Edit( {
shortcutCharacter="k"
aria-haspopup="true"
aria-expanded={ addingLink }
disabled={ ! canMakeLink }
/>
{ addingLink && (
<InlineLinkUI
Expand Down
78 changes: 17 additions & 61 deletions test/e2e/specs/editor/blocks/links.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,50 +18,6 @@
},
} );

test( `will use Post title as link text if link to existing post is created without any text selected`, async ( {
admin,
page,
editor,
requestUtils,
} ) => {
const titleText = 'Post to create a link to';
const { id: postId } = await requestUtils.createPost( {
title: titleText,
status: 'publish',
} );

await admin.createNewPost();

// Now in a new post and try to create a link from an autocomplete suggestion using the keyboard.
await editor.insertBlock( {
name: 'core/paragraph',
} );
await page.keyboard.type( 'Here comes a link: ' );

// Insert a link deliberately not selecting any text.
await editor.clickBlockToolbarButton( 'Link' );

// Trigger the autocomplete suggestion list and select the first suggestion.
await page.keyboard.type( 'Post to create a' );
await page.getByRole( 'option', { name: titleText } ).click();

await expect.poll( editor.getBlocks ).toMatchObject( [
{
name: 'core/paragraph',
attributes: {
content:
'Here comes a link: <a href="http://localhost:8889/?p=' +
postId +
'" data-type="post" data-id="' +
postId +
'">' +
titleText +
'</a>',
},
},
] );
} );

test( `can be created by selecting text and clicking link insertion button in block toolbar`, async ( {
page,
editor,
Expand Down Expand Up @@ -121,36 +77,36 @@
).toHaveValue( '' );
} );

test( `can be created without any text selected`, async ( {
test( `cannot be created without any text selected`, async ( {
page,
editor,
pageUtils,
LinkUtils,
} ) => {
// Create a block with some text.
await editor.insertBlock( {
name: 'core/paragraph',
} );
await page.keyboard.type( 'This is Gutenberg: ' );

// Press Cmd+K to insert a link.
await pageUtils.pressKeys( 'primary+K' );
await editor.showBlockToolbar();

// Type a URL.
await page.keyboard.type( 'https://wordpress.org/gutenberg' );
// Check that the Link button is disabled.
await page
.getByRole( 'toolbar', {
name: 'Block tools',
} )
.getByRole( 'button', {
name: 'Link',
} )
.isDisabled();

// Press Enter to apply the link.
await pageUtils.pressKeys( 'Enter' );
// Try Cmd+K to insert a link.
await pageUtils.pressKeys( 'primary+K' );

// A link with the URL as its text should have been inserted.
await expect.poll( editor.getBlocks ).toMatchObject( [
{
name: 'core/paragraph',
attributes: {
content:
'This is Gutenberg: <a href="https://wordpress.org/gutenberg">https://wordpress.org/gutenberg</a>',
},
},
] );
// Expect the link popover not to be visible because
// there is no text selection.
await expect( LinkUtils.getLinkPopover() ).toBeHidden();
} );

test( `will automatically create a link if selected text is a valid HTTP based URL`, async ( {
Expand Down Expand Up @@ -940,7 +896,7 @@

// Check the Link UI is open before asserting on presence of text input
// within that control.
await expect( linkPopover ).toBeVisible();

Check failure on line 899 in test/e2e/specs/editor/blocks/links.spec.js

View workflow job for this annotation

GitHub Actions / Playwright - 1

[chromium] › editor/blocks/links.spec.js:880:3 › Links › Editing link text › should not display text input when initially creating the link 

  4) [chromium] › editor/blocks/links.spec.js:880:3 › Links › Editing link text › should not display text input when initially creating the link 
    Error: Timed out 5000ms waiting for expect(locator).toBeVisible()

    Locator: locator('.components-popover__content .block-editor-link-control')
    Expected: visible
    Received: hidden
    Call log:
      - expect.toBeVisible with timeout 5000ms
      - waiting for locator('.components-popover__content .block-editor-link-control')


      897 | 			// Check the Link UI is open before asserting on presence of text input
      898 | 			// within that control.
    > 899 | 			await expect( linkPopover ).toBeVisible();
          | 			                            ^
      900 |
      901 | 			// Let's check we've focused a text input.
      902 | 			const textInput = linkPopover.getByLabel( 'Text', { exact: true } );

        at /home/runner/work/gutenberg/gutenberg/test/e2e/specs/editor/blocks/links.spec.js:899:32

// Let's check we've focused a text input.
const textInput = linkPopover.getByLabel( 'Text', { exact: true } );
Expand Down
Loading