From b943258b7b19636292230c74be4768ca72517875 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 18 Jan 2024 21:05:14 +0000 Subject: [PATCH 01/31] Proof of concept --- packages/format-library/src/link/index.js | 41 +++++++++++++++++++++-- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/packages/format-library/src/link/index.js b/packages/format-library/src/link/index.js index ab287daff19a7..cb9cdb35beb02 100644 --- a/packages/format-library/src/link/index.js +++ b/packages/format-library/src/link/index.js @@ -2,7 +2,7 @@ * WordPress dependencies */ import { __ } from '@wordpress/i18n'; -import { useState } from '@wordpress/element'; +import { useState, useLayoutEffect } from '@wordpress/element'; import { getTextContent, applyFormat, @@ -11,6 +11,7 @@ import { isCollapsed, insert, create, + useAnchor, } from '@wordpress/rich-text'; import { isURL, isEmail } from '@wordpress/url'; import { @@ -39,6 +40,42 @@ function Edit( { contentRef, } ) { const [ addingLink, setAddingLink ] = useState( false ); + const [ clickedLink, setClickedLink ] = useState( false ); + + const anchorElement = useAnchor( { + editableContentElement: contentRef.current, + settings: link, + } ); + + function setClickedLinkTrue() { + setClickedLink( true ); + } + + function setClickedLinkFalse() { + setClickedLink( false ); + } + + useLayoutEffect( () => { + // log tagNAME of anchorElement + if ( anchorElement?.tagName?.toLowerCase() === 'a' ) { + // add an event listener to the anchorElement + // for a click event + + anchorElement?.addEventListener( 'click', setClickedLinkTrue ); + } + + return () => { + // remove the event listener from the anchorElement + // for a click event + if ( anchorElement instanceof window.Element ) { + anchorElement?.removeEventListener( + 'click', + setClickedLinkTrue + ); + } + setClickedLinkFalse(); + }; + }, [ anchorElement, isActive ] ); function addLink() { const text = getTextContent( slice( value ) ); @@ -108,7 +145,7 @@ function Edit( { aria-expanded={ addingLink || isActive } /> ) } - { ( addingLink || isActive ) && ( + { ( addingLink || ( isActive && clickedLink ) ) && ( Date: Thu, 18 Jan 2024 15:28:36 -0600 Subject: [PATCH 02/31] Replace Unlink button in toolbar with edit link button --- packages/format-library/src/link/index.js | 39 +++++++---------------- 1 file changed, 12 insertions(+), 27 deletions(-) diff --git a/packages/format-library/src/link/index.js b/packages/format-library/src/link/index.js index cb9cdb35beb02..11d2e64b4a5cb 100644 --- a/packages/format-library/src/link/index.js +++ b/packages/format-library/src/link/index.js @@ -19,7 +19,7 @@ import { RichTextShortcut, } from '@wordpress/block-editor'; import { decodeEntities } from '@wordpress/html-entities'; -import { link as linkIcon, linkOff } from '@wordpress/icons'; +import { link as linkIcon } from '@wordpress/icons'; import { speak } from '@wordpress/a11y'; /** @@ -119,32 +119,17 @@ function Edit( { character="k" onUse={ onRemoveFormat } /> - { isActive && ( - - ) } - { ! isActive && ( - - ) } + { ( addingLink || ( isActive && clickedLink ) ) && ( Date: Fri, 19 Jan 2024 14:48:53 +0000 Subject: [PATCH 03/31] Avoid need for anchor hook --- packages/format-library/src/link/index.js | 27 ++++++----------------- 1 file changed, 7 insertions(+), 20 deletions(-) diff --git a/packages/format-library/src/link/index.js b/packages/format-library/src/link/index.js index 11d2e64b4a5cb..e1aa17e78f8d0 100644 --- a/packages/format-library/src/link/index.js +++ b/packages/format-library/src/link/index.js @@ -11,7 +11,6 @@ import { isCollapsed, insert, create, - useAnchor, } from '@wordpress/rich-text'; import { isURL, isEmail } from '@wordpress/url'; import { @@ -42,11 +41,6 @@ function Edit( { const [ addingLink, setAddingLink ] = useState( false ); const [ clickedLink, setClickedLink ] = useState( false ); - const anchorElement = useAnchor( { - editableContentElement: contentRef.current, - settings: link, - } ); - function setClickedLinkTrue() { setClickedLink( true ); } @@ -56,26 +50,19 @@ function Edit( { } useLayoutEffect( () => { - // log tagNAME of anchorElement - if ( anchorElement?.tagName?.toLowerCase() === 'a' ) { - // add an event listener to the anchorElement - // for a click event + const editableContentElement = contentRef.current; - anchorElement?.addEventListener( 'click', setClickedLinkTrue ); + function handleClick() { + setClickedLinkTrue(); } + editableContentElement.addEventListener( 'click', handleClick ); + return () => { - // remove the event listener from the anchorElement - // for a click event - if ( anchorElement instanceof window.Element ) { - anchorElement?.removeEventListener( - 'click', - setClickedLinkTrue - ); - } + editableContentElement.removeEventListener( 'click', handleClick ); setClickedLinkFalse(); }; - }, [ anchorElement, isActive ] ); + }, [ contentRef, isActive ] ); function addLink() { const text = getTextContent( slice( value ) ); From bbde341364b838e5f333945ff7290d68f72284f2 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Fri, 19 Jan 2024 15:35:48 +0000 Subject: [PATCH 04/31] Activating a link via keyboard or toolbar goes to preview mode --- packages/format-library/src/link/inline.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/format-library/src/link/inline.js b/packages/format-library/src/link/inline.js index 79933be6a6ac5..37c4a0f38cc66 100644 --- a/packages/format-library/src/link/inline.js +++ b/packages/format-library/src/link/inline.js @@ -263,7 +263,6 @@ function InlineLinkUI( { value={ linkValue } onChange={ onChangeLink } onRemove={ removeLink } - forceIsEditingLink={ addingLink } hasRichPreviews createSuggestion={ createPageEntity && handleCreate } withCreateSuggestion={ userCanCreatePages } From 74a3e9ef2c06801608ac54627bd96e5dcd52f61a Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Mon, 22 Jan 2024 14:43:31 -0600 Subject: [PATCH 05/31] Remove clickedLink state --- packages/format-library/src/link/index.js | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/packages/format-library/src/link/index.js b/packages/format-library/src/link/index.js index e1aa17e78f8d0..4657cc093201c 100644 --- a/packages/format-library/src/link/index.js +++ b/packages/format-library/src/link/index.js @@ -39,28 +39,22 @@ function Edit( { contentRef, } ) { const [ addingLink, setAddingLink ] = useState( false ); - const [ clickedLink, setClickedLink ] = useState( false ); - - function setClickedLinkTrue() { - setClickedLink( true ); - } - - function setClickedLinkFalse() { - setClickedLink( false ); - } useLayoutEffect( () => { const editableContentElement = contentRef.current; - function handleClick() { - setClickedLinkTrue(); + function handleClick( event ) { + if ( event.target.tagName !== 'A' ) { + return; + } + setAddingLink( true ); } editableContentElement.addEventListener( 'click', handleClick ); return () => { editableContentElement.removeEventListener( 'click', handleClick ); - setClickedLinkFalse(); + setAddingLink( false ); }; }, [ contentRef, isActive ] ); @@ -117,7 +111,7 @@ function Edit( { aria-haspopup="true" aria-expanded={ addingLink } /> - { ( addingLink || ( isActive && clickedLink ) ) && ( + { addingLink && ( Date: Tue, 23 Jan 2024 14:26:18 -0600 Subject: [PATCH 06/31] Fix bug where no contentEditable would crash block --- packages/format-library/src/link/index.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/format-library/src/link/index.js b/packages/format-library/src/link/index.js index 4657cc093201c..4d918c5fd9b86 100644 --- a/packages/format-library/src/link/index.js +++ b/packages/format-library/src/link/index.js @@ -42,6 +42,9 @@ function Edit( { useLayoutEffect( () => { const editableContentElement = contentRef.current; + if ( ! editableContentElement ) { + return; + } function handleClick( event ) { if ( event.target.tagName !== 'A' ) { From 8fde809c384dd3cd48135f7eed2ec09a7124867c Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Tue, 23 Jan 2024 16:01:56 -0600 Subject: [PATCH 07/31] Fix some e2e tests --- test/e2e/specs/editor/blocks/links.spec.js | 54 ++++++++++++++-------- 1 file changed, 36 insertions(+), 18 deletions(-) diff --git a/test/e2e/specs/editor/blocks/links.spec.js b/test/e2e/specs/editor/blocks/links.spec.js index d1de2df1da7ff..222b278cbd0a2 100644 --- a/test/e2e/specs/editor/blocks/links.spec.js +++ b/test/e2e/specs/editor/blocks/links.spec.js @@ -229,7 +229,7 @@ test.describe( 'Links', () => { // Click on the Edit button. await page.getByRole( 'button', { name: 'Edit' } ).click(); - + await page.getByLabel( 'Edit', { exact: true } ).click(); // Change the URL. // getByPlaceholder required in order to handle Link Control component // managing focus onto other inputs within the control. @@ -250,8 +250,13 @@ test.describe( 'Links', () => { ] ); } ); - test( `can remove existing links`, async ( { editor, LinkUtils } ) => { + test( `can remove existing links`, async ( { + editor, + LinkUtils, + pageUtils, + } ) => { await LinkUtils.createAndReselectLink(); + await pageUtils.pressKeys( 'primary+k' ); const linkPopover = LinkUtils.getLinkPopover(); @@ -336,6 +341,7 @@ test.describe( 'Links', () => { // Make a collapsed selection inside the link. await pageUtils.pressKeys( 'ArrowLeft' ); await pageUtils.pressKeys( 'ArrowRight' ); + await pageUtils.pressKeys( 'primary+k' ); const linkPopover = LinkUtils.getLinkPopover(); await linkPopover.getByRole( 'button', { name: 'Edit' } ).click(); @@ -441,10 +447,10 @@ test.describe( 'Links', () => { await page.keyboard.type( 'This is Gutenberg' ); await pageUtils.pressKeys( 'shiftAlt+ArrowLeft' ); await pageUtils.pressKeys( 'primary+K' ); + const linkPopover = LinkUtils.getLinkPopover(); await page.keyboard.type( URL ); await pageUtils.pressKeys( 'Enter' ); - const linkPopover = LinkUtils.getLinkPopover(); await expect( linkPopover ).toBeVisible(); // Close the link control to return the caret to the canvas await pageUtils.pressKeys( 'Escape' ); @@ -455,13 +461,15 @@ test.describe( 'Links', () => { await expect( linkPopover ).toBeHidden(); // Move the caret back into the link text and the link popover - // should be displayed. + // should not be displayed. await pageUtils.pressKeys( 'ArrowLeft' ); - await expect( linkPopover ).toBeVisible(); + await expect( linkPopover ).toBeHidden(); // Switch the Link UI into "Edit" mode via keyboard shortcut // and check that the input has the correct value. await pageUtils.pressKeys( 'primary+K' ); + await pageUtils.pressKeys( 'Tab' ); + await pageUtils.pressKeys( 'Enter' ); await expect( linkPopover.getByRole( 'combobox', { @@ -477,16 +485,12 @@ test.describe( 'Links', () => { // Move back into the RichText. await pageUtils.pressKeys( 'Escape' ); - - // ...but the Link Popover should still be active because we are within the link. - await expect( linkPopover ).toBeVisible(); + // Link Popover should be hidden even though it's within the linked text. + await expect( linkPopover ).toBeHidden(); // Move outside of the link entirely. await pageUtils.pressKeys( 'ArrowRight' ); - // Link Popover should now disappear because we are no longer within the link. - await expect( linkPopover ).toBeHidden(); - // Append some text to the paragraph to assert that focus has been returned // to the correct location within the RichText. await page.keyboard.type( ' and more!' ); @@ -561,7 +565,9 @@ test.describe( 'Links', () => { await page.keyboard.press( 'ArrowLeft' ); // Edit link. - await page.getByRole( 'button', { name: 'Edit' } ).click(); + await pageUtils.pressKeys( 'primary+K' ); + await pageUtils.pressKeys( 'Tab' ); + await pageUtils.pressKeys( 'Enter' ); // Open settings. await page @@ -638,6 +644,8 @@ test.describe( 'Links', () => { // Edit link. await pageUtils.pressKeys( 'primary+k' ); + await pageUtils.pressKeys( 'Tab' ); + await pageUtils.pressKeys( 'Enter' ); // getByPlaceholder required in order to handle Link Control component // managing focus onto other inputs within the control. @@ -650,6 +658,14 @@ test.describe( 'Links', () => { // Navigate back to the link editing state inputs to verify appears as changed. await page.keyboard.press( 'Tab' ); await page.keyboard.press( 'Enter' ); + // Navigate back to the popover. + await page.keyboard.press( 'ArrowLeft' ); + await page.keyboard.press( 'ArrowLeft' ); + + // Navigate back to inputs to verify appears as changed. + await pageUtils.pressKeys( 'primary+k' ); + await pageUtils.pressKeys( 'Tab' ); + await pageUtils.pressKeys( 'Enter' ); expect( await page @@ -706,6 +722,8 @@ test.describe( 'Links', () => { // Move back into the link. await pageUtils.pressKeys( 'shiftAlt+ArrowLeft' ); await pageUtils.pressKeys( 'primary+k' ); + await pageUtils.pressKeys( 'Tab' ); + await pageUtils.pressKeys( 'Enter' ); // Toggle the Advanced settings to be open. // This should set the editor preference to persist this @@ -722,14 +740,13 @@ test.describe( 'Links', () => { // Move focus out of Link UI and into Paragraph block. await pageUtils.pressKeys( 'Escape' ); - // Move caret back into the "WordPress" link to trigger - // the Link UI for that link. - await pageUtils.pressKeys( 'Alt+ArrowRight' ); - await pageUtils.pressKeys( 'ArrowRight' ); - await pageUtils.pressKeys( 'ArrowRight' ); + // Move caret back into the "WordPress" link + await pageUtils.pressKeys( 'ArrowRight', { times: 3 } ); // Switch Link UI to "edit" mode. - await page.getByRole( 'button', { name: 'Edit' } ).click(); + await pageUtils.pressKeys( 'primary+k' ); + await pageUtils.pressKeys( 'Tab' ); + await pageUtils.pressKeys( 'Enter' ); // Check that the Advanced settings are still expanded/open // and I can see the open in new tab checkbox. This verifies @@ -926,6 +943,7 @@ test.describe( 'Links', () => { // collapsed RichTextValue that contains a link format. await pageUtils.pressKeys( 'ArrowLeft' ); await pageUtils.pressKeys( 'ArrowRight' ); + await pageUtils.pressKeys( 'primary+k' ); const linkPopover = LinkUtils.getLinkPopover(); From b7659ef70340029916fb7e2585f2af44cab66722 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Wed, 24 Jan 2024 15:27:49 +0000 Subject: [PATCH 08/31] Fix bug with collapsed selections at edge --- packages/format-library/src/link/index.js | 25 +++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/packages/format-library/src/link/index.js b/packages/format-library/src/link/index.js index 4d918c5fd9b86..9b905c5fd29af 100644 --- a/packages/format-library/src/link/index.js +++ b/packages/format-library/src/link/index.js @@ -40,6 +40,17 @@ function Edit( { } ) { const [ addingLink, setAddingLink ] = useState( false ); + const isSelectionCollapsed = isCollapsed( value ); + + const canMakeLink = ! isSelectionCollapsed && ! addingLink && ! isActive; + + // There is a situation whereby there is an existing link in the rich text + // and the user clicks on the leftmost edge of that link and fails to activate + // the link format, but the click event still fires on the `` element. + // This causes the `addingLink` state to be set to `true` and the link UI + // to be rendered in "creating" mode. + const collapsedSelectionInactiveLink = isSelectionCollapsed && ! isActive; + useLayoutEffect( () => { const editableContentElement = contentRef.current; if ( ! editableContentElement ) { @@ -97,12 +108,20 @@ function Edit( { return ( <> - + { canMakeLink && ( + + ) } + + - { addingLink && ( + + { addingLink && ! collapsedSelectionInactiveLink && ( Date: Wed, 24 Jan 2024 15:29:39 +0000 Subject: [PATCH 09/31] Revert unwanted stuff --- packages/format-library/src/link/index.js | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/packages/format-library/src/link/index.js b/packages/format-library/src/link/index.js index 9b905c5fd29af..b8f336ab26f01 100644 --- a/packages/format-library/src/link/index.js +++ b/packages/format-library/src/link/index.js @@ -42,8 +42,6 @@ function Edit( { const isSelectionCollapsed = isCollapsed( value ); - const canMakeLink = ! isSelectionCollapsed && ! addingLink && ! isActive; - // There is a situation whereby there is an existing link in the rich text // and the user clicks on the leftmost edge of that link and fails to activate // the link format, but the click event still fires on the `` element. @@ -108,13 +106,7 @@ function Edit( { return ( <> - { canMakeLink && ( - - ) } + Date: Wed, 24 Jan 2024 09:45:18 -0600 Subject: [PATCH 10/31] Revert "Revert unwanted stuff" This reverts commit 016a3ea3190c727120ddd1854678264f57bfc107. --- packages/format-library/src/link/index.js | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/format-library/src/link/index.js b/packages/format-library/src/link/index.js index b8f336ab26f01..9b905c5fd29af 100644 --- a/packages/format-library/src/link/index.js +++ b/packages/format-library/src/link/index.js @@ -42,6 +42,8 @@ function Edit( { const isSelectionCollapsed = isCollapsed( value ); + const canMakeLink = ! isSelectionCollapsed && ! addingLink && ! isActive; + // There is a situation whereby there is an existing link in the rich text // and the user clicks on the leftmost edge of that link and fails to activate // the link format, but the click event still fires on the `` element. @@ -106,7 +108,13 @@ function Edit( { return ( <> - + { canMakeLink && ( + + ) } Date: Wed, 24 Jan 2024 09:45:30 -0600 Subject: [PATCH 11/31] Revert "Fix bug with collapsed selections at edge" This reverts commit b7659ef70340029916fb7e2585f2af44cab66722. --- packages/format-library/src/link/index.js | 25 ++--------------------- 1 file changed, 2 insertions(+), 23 deletions(-) diff --git a/packages/format-library/src/link/index.js b/packages/format-library/src/link/index.js index 9b905c5fd29af..4d918c5fd9b86 100644 --- a/packages/format-library/src/link/index.js +++ b/packages/format-library/src/link/index.js @@ -40,17 +40,6 @@ function Edit( { } ) { const [ addingLink, setAddingLink ] = useState( false ); - const isSelectionCollapsed = isCollapsed( value ); - - const canMakeLink = ! isSelectionCollapsed && ! addingLink && ! isActive; - - // There is a situation whereby there is an existing link in the rich text - // and the user clicks on the leftmost edge of that link and fails to activate - // the link format, but the click event still fires on the `` element. - // This causes the `addingLink` state to be set to `true` and the link UI - // to be rendered in "creating" mode. - const collapsedSelectionInactiveLink = isSelectionCollapsed && ! isActive; - useLayoutEffect( () => { const editableContentElement = contentRef.current; if ( ! editableContentElement ) { @@ -108,20 +97,12 @@ function Edit( { return ( <> - { canMakeLink && ( - - ) } - + - - - { addingLink && ! collapsedSelectionInactiveLink && ( + { addingLink && ( Date: Wed, 24 Jan 2024 09:47:41 -0600 Subject: [PATCH 12/31] Check for isActive before opening popover on link click --- packages/format-library/src/link/index.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/format-library/src/link/index.js b/packages/format-library/src/link/index.js index 4d918c5fd9b86..76eee5117fa2e 100644 --- a/packages/format-library/src/link/index.js +++ b/packages/format-library/src/link/index.js @@ -47,7 +47,13 @@ function Edit( { } function handleClick( event ) { - if ( event.target.tagName !== 'A' ) { + // There is a situation whereby there is an existing link in the rich text + // and the user clicks on the leftmost edge of that link and fails to activate + // the link format, but the click event still fires on the `` element. + // This causes the `addingLink` state to be set to `true` and the link UI + // to be rendered in "creating" mode. We need to check isActive to see if + // we have an active link format. + if ( event.target.tagName !== 'A' || ! isActive ) { return; } setAddingLink( true ); From 2f6b91e1865f293ed2dae252229c37cf1fc119b9 Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Wed, 24 Jan 2024 13:09:29 -0600 Subject: [PATCH 13/31] Return focus to the element that opened the link control popover Focus should return to the area that triggered the popover when pressing escape. --- packages/format-library/src/link/index.js | 29 ++++++++++++++++++---- packages/format-library/src/link/inline.js | 18 +++----------- 2 files changed, 28 insertions(+), 19 deletions(-) diff --git a/packages/format-library/src/link/index.js b/packages/format-library/src/link/index.js index 76eee5117fa2e..2b4267b706227 100644 --- a/packages/format-library/src/link/index.js +++ b/packages/format-library/src/link/index.js @@ -39,6 +39,7 @@ function Edit( { contentRef, } ) { const [ addingLink, setAddingLink ] = useState( false ); + const [ openedBy, setOpenedBy ] = useState( null ); useLayoutEffect( () => { const editableContentElement = contentRef.current; @@ -56,6 +57,7 @@ function Edit( { if ( event.target.tagName !== 'A' || ! isActive ) { return; } + setAddingLink( true ); } @@ -89,11 +91,26 @@ function Edit( { } } - function stopAddingLink( returnFocus = true ) { + function onClose() { + // Don't let the click handler on the toolbar button trigger again. + + // There are two places for us to return focus to on Escape keypress: + // 1. The rich text field. + // 2. The toolbar button. + + // The toolbar button is the only one we need to handle returning focus to. + // Otherwise, we rely on the passed in onFocus to return focus to the rich text field. + + // Close the popover setAddingLink( false ); - if ( returnFocus ) { + // Return focus to the toolbar button or the rich text field + if ( openedBy?.tagName === 'BUTTON' ) { + openedBy.focus(); + } else { onFocus(); } + // Remove the openedBy state + setOpenedBy( null ); } function onRemoveFormat() { @@ -113,7 +130,10 @@ function Edit( { name="link" icon={ linkIcon } title={ isActive ? __( 'Edit Link' ) : title } - onClick={ addLink } + onClick={ ( event ) => { + setOpenedBy( event.currentTarget ); + setAddingLink( true ); + } } isActive={ isActive || addingLink } shortcutType="primary" shortcutCharacter="k" @@ -122,8 +142,7 @@ function Edit( { /> { addingLink && ( cachedRect; - // Focus should only be moved into the Popover when the Link is being created or edited. - // When the Link is in "preview" mode focus should remain on the rich text because at - // this point the Link dialog is informational only and thus the user should be able to - // continue editing the rich text. - // Ref used because the focusOnMount prop shouldn't evolve during render of a Popover - // otherwise it causes a render of the content. - const focusOnMount = useRef( addingLink ? 'firstElement' : false ); - async function handleCreate( pageTitle ) { const page = await createPageEntity( { title: pageTitle, @@ -252,9 +243,8 @@ function InlineLinkUI( { return ( stopAddingLink( false ) } + onFocusOutside={ onFocusOutside } placement="bottom" offset={ 10 } shift From 94e332a85a9c6729df90b520201928b517182335 Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Wed, 24 Jan 2024 13:26:41 -0600 Subject: [PATCH 14/31] Use onFocusOutside to prevent stealing focus --- packages/format-library/src/link/index.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/packages/format-library/src/link/index.js b/packages/format-library/src/link/index.js index 2b4267b706227..16571a1f4b383 100644 --- a/packages/format-library/src/link/index.js +++ b/packages/format-library/src/link/index.js @@ -113,6 +113,17 @@ function Edit( { setOpenedBy( null ); } + // Test for this: + // 1. Click on the link button + // 2. Click the Options button in the top right of header + // 3. Focus should be in the dropdown of the Options button + // 4. Press Escape + // 5. Focus should be on the Options button + function onFocusOutside() { + setAddingLink( false ); + setOpenedBy( null ); + } + function onRemoveFormat() { onChange( removeFormat( value, name ) ); speak( __( 'Link removed.' ), 'assertive' ); @@ -143,6 +154,7 @@ function Edit( { { addingLink && ( Date: Wed, 24 Jan 2024 13:56:59 -0600 Subject: [PATCH 15/31] Change name back to stopAddingLink --- packages/format-library/src/link/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/format-library/src/link/index.js b/packages/format-library/src/link/index.js index 16571a1f4b383..34d4d192a9100 100644 --- a/packages/format-library/src/link/index.js +++ b/packages/format-library/src/link/index.js @@ -91,7 +91,7 @@ function Edit( { } } - function onClose() { + function stopAddingLink() { // Don't let the click handler on the toolbar button trigger again. // There are two places for us to return focus to on Escape keypress: @@ -153,7 +153,7 @@ function Edit( { /> { addingLink && ( Date: Wed, 24 Jan 2024 14:01:12 -0600 Subject: [PATCH 16/31] Add comments --- packages/format-library/src/link/index.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/format-library/src/link/index.js b/packages/format-library/src/link/index.js index 34d4d192a9100..248615def15d3 100644 --- a/packages/format-library/src/link/index.js +++ b/packages/format-library/src/link/index.js @@ -39,6 +39,7 @@ function Edit( { contentRef, } ) { const [ addingLink, setAddingLink ] = useState( false ); + // 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 ); useLayoutEffect( () => { @@ -91,6 +92,10 @@ function Edit( { } } + /** + * Runs when the popover is closed via escape keypress, unlinking the selected text, + * but _not_ on a click outside the popover. onFocusOutside handles that. + */ function stopAddingLink() { // Don't let the click handler on the toolbar button trigger again. From 5091a0af23e599ed76dc8aa100899452d56217ff Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Wed, 24 Jan 2024 15:43:23 -0600 Subject: [PATCH 17/31] Only add link for email and urls if it's a new link --- packages/format-library/src/link/index.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/format-library/src/link/index.js b/packages/format-library/src/link/index.js index 248615def15d3..7483c8d95132d 100644 --- a/packages/format-library/src/link/index.js +++ b/packages/format-library/src/link/index.js @@ -70,17 +70,17 @@ function Edit( { }; }, [ contentRef, isActive ] ); - function addLink() { + function addLink( target ) { const text = getTextContent( slice( value ) ); - if ( text && isURL( text ) && isValidHref( text ) ) { + if ( ! isActive && text && isURL( text ) && isValidHref( text ) ) { onChange( applyFormat( value, { type: name, attributes: { url: text }, } ) ); - } else if ( text && isEmail( text ) ) { + } else if ( ! isActive && text && isEmail( text ) ) { onChange( applyFormat( value, { type: name, @@ -88,6 +88,9 @@ function Edit( { } ) ); } else { + if ( target ) { + setOpenedBy( target ); + } setAddingLink( true ); } } @@ -147,8 +150,7 @@ function Edit( { icon={ linkIcon } title={ isActive ? __( 'Edit Link' ) : title } onClick={ ( event ) => { - setOpenedBy( event.currentTarget ); - setAddingLink( true ); + addLink( event.currentTarget ); } } isActive={ isActive || addingLink } shortcutType="primary" From c006259a8b69907c52d192b0dd0490c30d0bc971 Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Wed, 24 Jan 2024 15:58:57 -0600 Subject: [PATCH 18/31] Update test for escape from link popover to return focus --- test/e2e/specs/editor/blocks/links.spec.js | 27 +++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/test/e2e/specs/editor/blocks/links.spec.js b/test/e2e/specs/editor/blocks/links.spec.js index 222b278cbd0a2..f83b9d9386735 100644 --- a/test/e2e/specs/editor/blocks/links.spec.js +++ b/test/e2e/specs/editor/blocks/links.spec.js @@ -390,7 +390,7 @@ test.describe( 'Links', () => { await pageUtils.pressKeys( 'shiftAlt+ArrowLeft' ); // Insert a link. - await editor.clickBlockToolbarButton( 'Link' ); + await pageUtils.pressKeys( 'primary+k' ); const urlInput = page.getByRole( 'combobox', { name: 'Link', @@ -430,6 +430,31 @@ test.describe( 'Links', () => { }, }, ] ); + + // Test pressing escape from the toolbar button should return focus to the toolbar button + // Insert a link. + await editor.clickBlockToolbarButton( 'Link' ); + + // Expect the "Link" combobox to be visible and focused + await expect( urlInput ).toBeVisible(); + await expect( urlInput ).toBeFocused(); + + await page.keyboard.press( 'Escape' ); + await expect( LinkUtils.getLinkPopover() ).toBeHidden(); + + // Focus should return to the Link Toolbar Button that opened the popover + await expect( + page.getByLabel( 'Link', { exact: true } ) + ).toBeFocused(); + + await expect.poll( editor.getBlocks ).toMatchObject( [ + { + name: 'core/paragraph', + attributes: { + content: 'This is Gutenberg and more!', + }, + }, + ] ); } ); test( `can be created and modified using only the keyboard`, async ( { From 76b89d3cddc2c86a96fdaae78e585e1d2886d8e3 Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Wed, 24 Jan 2024 16:10:48 -0600 Subject: [PATCH 19/31] Leave link UI open after initial creation --- packages/format-library/src/link/index.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/format-library/src/link/index.js b/packages/format-library/src/link/index.js index 7483c8d95132d..2cb38f2cca916 100644 --- a/packages/format-library/src/link/index.js +++ b/packages/format-library/src/link/index.js @@ -66,7 +66,6 @@ function Edit( { return () => { editableContentElement.removeEventListener( 'click', handleClick ); - setAddingLink( false ); }; }, [ contentRef, isActive ] ); From 196d888dee6c375390ec6c7b88b644bf35ecb6db Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Wed, 24 Jan 2024 16:15:17 -0600 Subject: [PATCH 20/31] Fix createAndReselect test util --- test/e2e/specs/editor/blocks/links.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/specs/editor/blocks/links.spec.js b/test/e2e/specs/editor/blocks/links.spec.js index f83b9d9386735..946bd7737e5c3 100644 --- a/test/e2e/specs/editor/blocks/links.spec.js +++ b/test/e2e/specs/editor/blocks/links.spec.js @@ -1300,7 +1300,7 @@ class LinkUtils { await this.pageUtils.pressKeys( 'shiftAlt+ArrowLeft' ); // Click on the Link button. - await this.page.getByRole( 'button', { name: 'Link' } ).click(); + await this.pageUtils.pressKeys( 'primary+k' ); // Type a URL. await this.page.keyboard.type( 'https://wordpress.org/gutenberg' ); From 8ff163cdd39a35110dd5c36a3b0b68d6816aedea Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 25 Jan 2024 15:05:44 +0000 Subject: [PATCH 21/31] Fix keyboard shortcut tests --- test/e2e/specs/editor/blocks/links.spec.js | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/test/e2e/specs/editor/blocks/links.spec.js b/test/e2e/specs/editor/blocks/links.spec.js index 946bd7737e5c3..1852d63e641bd 100644 --- a/test/e2e/specs/editor/blocks/links.spec.js +++ b/test/e2e/specs/editor/blocks/links.spec.js @@ -572,6 +572,11 @@ test.describe( 'Links', () => { // Press Cmd+K to insert a link. await pageUtils.pressKeys( 'primary+K' ); + const linkPopover = LinkUtils.getLinkPopover(); + + // Expect link popover to be visible + await expect( linkPopover ).toBeVisible(); + // Type a URL. await page.keyboard.type( 'https://wordpress.org/gutenberg' ); @@ -584,36 +589,31 @@ test.describe( 'Links', () => { }, ] ); + // Submit the link. await page.keyboard.press( 'Enter' ); - await page.keyboard.press( 'ArrowLeft' ); - await page.keyboard.press( 'ArrowLeft' ); + // Expect the Link UI to still be visible + await expect( linkPopover ).toBeVisible(); - // Edit link. - await pageUtils.pressKeys( 'primary+K' ); + // Tab to "Edit" button and enter edit mode again. await pageUtils.pressKeys( 'Tab' ); await pageUtils.pressKeys( 'Enter' ); // Open settings. - await page - .getByRole( 'region', { - name: 'Editor content', - } ) + await linkPopover .getByRole( 'button', { name: 'Advanced', } ) .click(); // Navigate to and toggle the "Open in new tab" checkbox. - const checkbox = page.getByLabel( 'Open in new tab' ); + const checkbox = linkPopover.getByLabel( 'Open in new tab' ); await checkbox.click(); // Toggle should still have focus and be checked. await expect( checkbox ).toBeChecked(); await expect( checkbox ).toBeFocused(); - const linkPopover = LinkUtils.getLinkPopover(); - // Tab back to the Submit and apply the link. await linkPopover.getByRole( 'button', { name: 'Save' } ).click(); From c3200f2eced15f8f380145984100c87a65da89cf Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 25 Jan 2024 15:11:47 +0000 Subject: [PATCH 22/31] Fix and simplify updating URL test --- test/e2e/specs/editor/blocks/links.spec.js | 57 +++++++++------------- 1 file changed, 23 insertions(+), 34 deletions(-) diff --git a/test/e2e/specs/editor/blocks/links.spec.js b/test/e2e/specs/editor/blocks/links.spec.js index 1852d63e641bd..4df947ca07dba 100644 --- a/test/e2e/specs/editor/blocks/links.spec.js +++ b/test/e2e/specs/editor/blocks/links.spec.js @@ -640,18 +640,22 @@ test.describe( 'Links', () => { name: 'core/paragraph', } ); await page.keyboard.type( 'This is WordPress' ); + // Select "WordPress". await pageUtils.pressKeys( 'shiftAlt+ArrowLeft' ); await pageUtils.pressKeys( 'primary+k' ); - await page.keyboard.type( 'w.org' ); - await page.keyboard.press( 'Enter' ); // Close the link control to return the caret to the canvas const linkPopover = LinkUtils.getLinkPopover(); + + await page.keyboard.type( 'w.org' ); + + // Submit the link + await page.keyboard.press( 'Enter' ); + + // Close the Link Popover. await pageUtils.pressKeys( 'Escape' ); - // Deselect the link text by moving the caret to the end of the line - // and the link popover should not be displayed. - await pageUtils.pressKeys( 'End' ); + await expect( linkPopover ).toBeHidden(); await expect.poll( editor.getBlocks ).toMatchObject( [ @@ -663,43 +667,28 @@ test.describe( 'Links', () => { }, ] ); - // Move caret back into the link. - await page.keyboard.press( 'ArrowLeft' ); - await page.keyboard.press( 'ArrowLeft' ); - - // Edit link. + // Edit the link again. await pageUtils.pressKeys( 'primary+k' ); - await pageUtils.pressKeys( 'Tab' ); - await pageUtils.pressKeys( 'Enter' ); - // getByPlaceholder required in order to handle Link Control component + // Expect Link UI to be visible again + await expect( linkPopover ).toBeVisible(); + + // Click on the `Edit` button. + await linkPopover.getByRole( 'button', { name: 'Edit' } ).click(); + + // Change the URL. + // Note: getByPlaceholder required in order to handle Link Control component // managing focus onto other inputs within the control. - await page.getByPlaceholder( 'Search or type url' ).fill( '' ); + await linkPopover.getByPlaceholder( 'Search or type url' ).fill( '' ); await page.keyboard.type( 'wordpress.org' ); - // Update the link. + // Save the link. await linkPopover.getByRole( 'button', { name: 'Save' } ).click(); - // Navigate back to the link editing state inputs to verify appears as changed. - await page.keyboard.press( 'Tab' ); - await page.keyboard.press( 'Enter' ); - // Navigate back to the popover. - await page.keyboard.press( 'ArrowLeft' ); - await page.keyboard.press( 'ArrowLeft' ); - - // Navigate back to inputs to verify appears as changed. - await pageUtils.pressKeys( 'primary+k' ); - await pageUtils.pressKeys( 'Tab' ); - await pageUtils.pressKeys( 'Enter' ); - - expect( - await page - .getByRole( 'combobox', { - name: 'Link', - } ) - .inputValue() - ).toContain( 'wordpress.org' ); + // Link UI should be closed. + await expect( linkPopover ).toBeHidden(); + // The link should have been updated. await expect.poll( editor.getBlocks ).toMatchObject( [ { name: 'core/paragraph', From 21a8b12239bf5709df8bd0b0396fb360c301ba1a Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 25 Jan 2024 15:24:33 +0000 Subject: [PATCH 23/31] Fix test for preserving state of advanced settings drawer --- test/e2e/specs/editor/blocks/links.spec.js | 55 ++++++++++++---------- 1 file changed, 30 insertions(+), 25 deletions(-) diff --git a/test/e2e/specs/editor/blocks/links.spec.js b/test/e2e/specs/editor/blocks/links.spec.js index 4df947ca07dba..86811eed2496e 100644 --- a/test/e2e/specs/editor/blocks/links.spec.js +++ b/test/e2e/specs/editor/blocks/links.spec.js @@ -704,6 +704,7 @@ test.describe( 'Links', () => { page, editor, pageUtils, + LinkUtils, } ) => { // Create a block with some text. await editor.insertBlock( { @@ -729,23 +730,18 @@ test.describe( 'Links', () => { // Create a link. await pageUtils.pressKeys( 'primary+k' ); + await page.keyboard.type( 'https://wordpress.org/plugins/gutenberg/' ); await page.keyboard.press( 'Enter' ); - await page.keyboard.press( 'Escape' ); - await pageUtils.pressKeys( 'End' ); - // Move back into the link. - await pageUtils.pressKeys( 'shiftAlt+ArrowLeft' ); - await pageUtils.pressKeys( 'primary+k' ); - await pageUtils.pressKeys( 'Tab' ); - await pageUtils.pressKeys( 'Enter' ); + + // Press the "Edit" button + const linkPopover = LinkUtils.getLinkPopover(); + await linkPopover.getByRole( 'button', { name: 'Edit' } ).click(); // Toggle the Advanced settings to be open. // This should set the editor preference to persist this // UI state. - await page - .getByRole( 'region', { - name: 'Editor content', - } ) + await linkPopover .getByRole( 'button', { name: 'Advanced', } ) @@ -754,24 +750,25 @@ test.describe( 'Links', () => { // Move focus out of Link UI and into Paragraph block. await pageUtils.pressKeys( 'Escape' ); - // Move caret back into the "WordPress" link - await pageUtils.pressKeys( 'ArrowRight', { times: 3 } ); + // Click on the "WordPress" link + await editor.canvas + .getByRole( 'link', { + name: 'WordPress', + } ) + .click(); - // Switch Link UI to "edit" mode. - await pageUtils.pressKeys( 'primary+k' ); - await pageUtils.pressKeys( 'Tab' ); - await pageUtils.pressKeys( 'Enter' ); + // press the "edit" button + await linkPopover.getByRole( 'button', { name: 'Edit' } ).click(); // Check that the Advanced settings are still expanded/open // and I can see the open in new tab checkbox. This verifies // that the editor preference was persisted. - await expect( page.getByLabel( 'Open in new tab' ) ).toBeVisible(); + await expect( + linkPopover.getByLabel( 'Open in new tab' ) + ).toBeVisible(); // Toggle the Advanced settings back to being closed. - await page - .getByRole( 'region', { - name: 'Editor content', - } ) + await linkPopover .getByRole( 'button', { name: 'Advanced', } ) @@ -782,12 +779,20 @@ test.describe( 'Links', () => { // Move caret back into the "Gutenberg" link and open // the Link UI for that link. - await pageUtils.pressKeys( 'shiftAlt+ArrowLeft' ); - await pageUtils.pressKeys( 'primary+k' ); + await editor.canvas + .getByRole( 'link', { + name: 'Gutenberg', + } ) + .click(); + + // Switch to "Edit" mode. + await linkPopover.getByRole( 'button', { name: 'Edit' } ).click(); // Check that the Advanced settings are still closed. // This verifies that the editor preference was persisted. - await expect( page.getByLabel( 'Open in new tab' ) ).toBeHidden(); + await expect( + linkPopover.getByLabel( 'Open in new tab' ) + ).toBeHidden(); } ); test( 'can toggle link settings and save', async ( { From 3ff54ff6ec0dc6938fd637d64e949b6e0e7ac800 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 25 Jan 2024 15:27:29 +0000 Subject: [PATCH 24/31] Fix test for toggling link settings --- test/e2e/specs/editor/blocks/links.spec.js | 49 ++++++++++++---------- 1 file changed, 27 insertions(+), 22 deletions(-) diff --git a/test/e2e/specs/editor/blocks/links.spec.js b/test/e2e/specs/editor/blocks/links.spec.js index 86811eed2496e..5a7341dd6c52a 100644 --- a/test/e2e/specs/editor/blocks/links.spec.js +++ b/test/e2e/specs/editor/blocks/links.spec.js @@ -796,9 +796,7 @@ test.describe( 'Links', () => { } ); test( 'can toggle link settings and save', async ( { - page, editor, - pageUtils, LinkUtils, } ) => { await editor.insertBlock( { @@ -809,31 +807,35 @@ test.describe( 'Links', () => { }, } ); - // Move caret into the link. - await pageUtils.pressKeys( 'ArrowRight' ); + // Click on "Gutenberg" link in the canvas + await editor.canvas + .getByRole( 'link', { + name: 'Gutenberg', + } ) + .click(); - // Switch Link UI to "edit" mode. - await page.getByRole( 'button', { name: 'Edit' } ).click(); + // Get the Link Popover using the LinkUtils helper + const linkPopover = LinkUtils.getLinkPopover(); + + // Switch to Edit the link + await linkPopover.getByRole( 'button', { name: 'Edit' } ).click(); // Open Advanced Settings - await page - .getByRole( 'region', { - name: 'Editor content', - } ) + await linkPopover .getByRole( 'button', { name: 'Advanced', } ) .click(); // expect settings for `Open in new tab` and `No follow` - await expect( page.getByLabel( 'Open in new tab' ) ).not.toBeChecked(); - await expect( page.getByLabel( 'nofollow' ) ).not.toBeChecked(); + await expect( + linkPopover.getByLabel( 'Open in new tab' ) + ).not.toBeChecked(); + await expect( linkPopover.getByLabel( 'nofollow' ) ).not.toBeChecked(); // Toggle both of the settings - await page.getByLabel( 'Open in new tab' ).click(); - await page.getByLabel( 'nofollow' ).click(); - - const linkPopover = LinkUtils.getLinkPopover(); + await linkPopover.getByLabel( 'Open in new tab' ).click(); + await linkPopover.getByLabel( 'nofollow' ).click(); // Save the link await linkPopover.getByRole( 'button', { name: 'Save' } ).click(); @@ -848,17 +850,20 @@ test.describe( 'Links', () => { }, ] ); - // Move caret back into the link. - await page.keyboard.press( 'ArrowRight' ); - await page.keyboard.press( 'ArrowRight' ); + // Click on "Gutenberg" link in the canvas again + await editor.canvas + .getByRole( 'link', { + name: 'Gutenberg', + } ) + .click(); // Edit the link - await page.getByRole( 'button', { name: 'Edit' } ).click(); + await linkPopover.getByRole( 'button', { name: 'Edit' } ).click(); // Toggle both the settings to be off. // Note: no need to toggle settings again because the open setting should be persisted. - await page.getByLabel( 'Open in new tab' ).click(); - await page.getByLabel( 'nofollow' ).click(); + await linkPopover.getByLabel( 'Open in new tab' ).click(); + await linkPopover.getByLabel( 'nofollow' ).click(); // Save the link await linkPopover.getByRole( 'button', { name: 'Save' } ).click(); From 2517f504ac8df5559fa7f49413eb51b5a37acb3b Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Thu, 25 Jan 2024 15:36:45 +0000 Subject: [PATCH 25/31] Fix link text change test --- test/e2e/specs/editor/blocks/links.spec.js | 37 ++++++++++------------ 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/test/e2e/specs/editor/blocks/links.spec.js b/test/e2e/specs/editor/blocks/links.spec.js index 5a7341dd6c52a..d5a5eefd994d2 100644 --- a/test/e2e/specs/editor/blocks/links.spec.js +++ b/test/e2e/specs/editor/blocks/links.spec.js @@ -225,7 +225,7 @@ test.describe( 'Links', () => { pageUtils, LinkUtils, } ) => { - await LinkUtils.createAndReselectLink(); + await LinkUtils.createLink(); // Click on the Edit button. await page.getByRole( 'button', { name: 'Edit' } ).click(); @@ -255,7 +255,7 @@ test.describe( 'Links', () => { LinkUtils, pageUtils, } ) => { - await LinkUtils.createAndReselectLink(); + await LinkUtils.createLink(); await pageUtils.pressKeys( 'primary+k' ); const linkPopover = LinkUtils.getLinkPopover(); @@ -337,7 +337,7 @@ test.describe( 'Links', () => { LinkUtils, pageUtils, } ) => { - await LinkUtils.createAndReselectLink(); + await LinkUtils.createLink(); // Make a collapsed selection inside the link. await pageUtils.pressKeys( 'ArrowLeft' ); await pageUtils.pressKeys( 'ArrowRight' ); @@ -886,20 +886,16 @@ test.describe( 'Links', () => { editor, LinkUtils, } ) => { - await LinkUtils.createAndReselectLink(); + await LinkUtils.createLink(); const originalLinkText = 'Gutenberg'; const changedLinkText = ' link text that was modified via the Link UI to include spaces '; - // Make a collapsed selection inside the link. This is used - // as a stress test to ensure we can find the link text from a - // collapsed RichTextValue that contains a link format. - await pageUtils.pressKeys( 'ArrowLeft' ); - await pageUtils.pressKeys( 'ArrowRight' ); + // Get the LinkPopover using the LinkUtils + const linkPopover = LinkUtils.getLinkPopover(); - await editor.showBlockToolbar(); - await page.getByRole( 'button', { name: 'Edit' } ).click(); + await linkPopover.getByRole( 'button', { name: 'Edit' } ).click(); const textInput = page.getByLabel( 'Text', { exact: true } ); @@ -960,7 +956,7 @@ test.describe( 'Links', () => { pageUtils, LinkUtils, } ) => { - await LinkUtils.createAndReselectLink(); + await LinkUtils.createLink(); // Make a collapsed selection inside the link. This is used // as a stress test to ensure we can find the link text from a @@ -1032,7 +1028,7 @@ test.describe( 'Links', () => { } ) => { const originalLinkText = 'Gutenberg'; - await LinkUtils.createAndReselectLink(); + await LinkUtils.createLink(); // Make a collapsed selection inside the link in order // to activate the Link UI. @@ -1288,7 +1284,7 @@ class LinkUtils { }, isFixed ); } - async createAndReselectLink() { + async createLink() { // Create a block with some text. await this.editor.insertBlock( { name: 'core/paragraph', @@ -1301,16 +1297,17 @@ class LinkUtils { // Click on the Link button. await this.pageUtils.pressKeys( 'primary+k' ); + // get the link popover + const linkPopover = this.getLinkPopover(); + + // Expect link popover to be visible + await expect( linkPopover ).toBeVisible(); + // Type a URL. await this.page.keyboard.type( 'https://wordpress.org/gutenberg' ); - // Click on the Submit button. + // Submit the link. await this.pageUtils.pressKeys( 'Enter' ); - await this.pageUtils.pressKeys( 'Escape' ); - await this.pageUtils.pressKeys( 'End' ); - - // Reselect the link. - await this.pageUtils.pressKeys( 'shiftAlt+ArrowLeft' ); } /** From 7ab61d1b9ec31e240fed1f383ae6f1b49f773ad5 Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Thu, 25 Jan 2024 09:48:46 -0600 Subject: [PATCH 26/31] Update Edit Link -> Edit link --- packages/format-library/src/link/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/format-library/src/link/index.js b/packages/format-library/src/link/index.js index 2cb38f2cca916..5c18b752c2059 100644 --- a/packages/format-library/src/link/index.js +++ b/packages/format-library/src/link/index.js @@ -147,7 +147,7 @@ function Edit( { { addLink( event.currentTarget ); } } From 6b878f00cf03c83432b64215669afa2400ffbbd1 Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Thu, 25 Jan 2024 10:35:20 -0600 Subject: [PATCH 27/31] Change Edit link to Link --- packages/format-library/src/link/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/format-library/src/link/index.js b/packages/format-library/src/link/index.js index 5c18b752c2059..61b6f974cdce3 100644 --- a/packages/format-library/src/link/index.js +++ b/packages/format-library/src/link/index.js @@ -147,7 +147,7 @@ function Edit( { { addLink( event.currentTarget ); } } From ba6704431b73834cc460f9eff1b855c5b91f632a Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Thu, 25 Jan 2024 10:46:35 -0600 Subject: [PATCH 28/31] Fix some tests --- test/e2e/specs/editor/blocks/links.spec.js | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/test/e2e/specs/editor/blocks/links.spec.js b/test/e2e/specs/editor/blocks/links.spec.js index d5a5eefd994d2..cd67f2236d098 100644 --- a/test/e2e/specs/editor/blocks/links.spec.js +++ b/test/e2e/specs/editor/blocks/links.spec.js @@ -228,8 +228,8 @@ test.describe( 'Links', () => { await LinkUtils.createLink(); // Click on the Edit button. - await page.getByRole( 'button', { name: 'Edit' } ).click(); - await page.getByLabel( 'Edit', { exact: true } ).click(); + await page.getByRole( 'button', { name: 'Edit', exact: true } ).click(); + // Change the URL. // getByPlaceholder required in order to handle Link Control component // managing focus onto other inputs within the control. @@ -250,13 +250,8 @@ test.describe( 'Links', () => { ] ); } ); - test( `can remove existing links`, async ( { - editor, - LinkUtils, - pageUtils, - } ) => { + test( `can remove existing links`, async ( { editor, LinkUtils } ) => { await LinkUtils.createLink(); - await pageUtils.pressKeys( 'primary+k' ); const linkPopover = LinkUtils.getLinkPopover(); @@ -338,6 +333,7 @@ test.describe( 'Links', () => { pageUtils, } ) => { await LinkUtils.createLink(); + await pageUtils.pressKeys( 'Escape' ); // Make a collapsed selection inside the link. await pageUtils.pressKeys( 'ArrowLeft' ); await pageUtils.pressKeys( 'ArrowRight' ); @@ -957,6 +953,7 @@ test.describe( 'Links', () => { LinkUtils, } ) => { await LinkUtils.createLink(); + await pageUtils.pressKeys( 'Escape' ); // Make a collapsed selection inside the link. This is used // as a stress test to ensure we can find the link text from a From e75f2b19f27a8f336375401eb25a322e692e9703 Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Thu, 25 Jan 2024 10:49:22 -0600 Subject: [PATCH 29/31] Remove test no longer relevant The state being tested is no longer possible with the new ux. --- test/e2e/specs/editor/blocks/links.spec.js | 42 ---------------------- 1 file changed, 42 deletions(-) diff --git a/test/e2e/specs/editor/blocks/links.spec.js b/test/e2e/specs/editor/blocks/links.spec.js index cd67f2236d098..3c7d24cf49c06 100644 --- a/test/e2e/specs/editor/blocks/links.spec.js +++ b/test/e2e/specs/editor/blocks/links.spec.js @@ -1017,48 +1017,6 @@ test.describe( 'Links', () => { }, ] ); } ); - - test( 'should display (capture the) text from the currently active link even if there is a rich text selection', async ( { - editor, - pageUtils, - LinkUtils, - } ) => { - const originalLinkText = 'Gutenberg'; - - await LinkUtils.createLink(); - - // Make a collapsed selection inside the link in order - // to activate the Link UI. - await pageUtils.pressKeys( 'ArrowLeft' ); - await pageUtils.pressKeys( 'ArrowRight' ); - - const linkPopover = LinkUtils.getLinkPopover(); - - await linkPopover.getByRole( 'button', { name: 'Edit' } ).click(); - - // Place cursor within the underling RichText link (not the Link UI). - await editor.canvas - .getByRole( 'document', { - name: 'Block: Paragraph', - } ) - .getByRole( 'link', { - name: 'Gutenberg', - } ) - .click(); - - // Make a selection within the RichText. - await pageUtils.pressKeys( 'shift+ArrowRight', { - times: 3, - } ); - - // Making a selection within the link text whilst the Link UI - // is open should not alter the value in the Link UI's "Text" - // field. It should remain as the full text of the currently - // focused link format. - await expect( - linkPopover.getByLabel( 'Text', { exact: true } ) - ).toHaveValue( originalLinkText ); - } ); } ); test.describe( 'Disabling Link UI active state', () => { From 783a37823ade226f18f00dbc42cc51f980c35fbf Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Thu, 25 Jan 2024 11:04:07 -0600 Subject: [PATCH 30/31] Remove unused test. Add test for onFocusOutside --- test/e2e/specs/editor/blocks/links.spec.js | 137 +++------------------ 1 file changed, 15 insertions(+), 122 deletions(-) diff --git a/test/e2e/specs/editor/blocks/links.spec.js b/test/e2e/specs/editor/blocks/links.spec.js index 3c7d24cf49c06..a80c2304ac7fc 100644 --- a/test/e2e/specs/editor/blocks/links.spec.js +++ b/test/e2e/specs/editor/blocks/links.spec.js @@ -1020,141 +1020,34 @@ test.describe( 'Links', () => { } ); test.describe( 'Disabling Link UI active state', () => { - test( 'should not show the Link UI when selection extends beyond link boundary', async ( { + test( 'should correctly move focus when link control closes on click outside', async ( { page, pageUtils, - editor, LinkUtils, } ) => { - const linkedText = `Gutenberg`; - const textBeyondLinkedText = ` and more text.`; - - // Create a block with some text. - await editor.insertBlock( { - name: 'core/paragraph', - } ); - await page.keyboard.type( - `This is ${ linkedText }${ textBeyondLinkedText }` - ); - - // Move cursor next to end of `linkedText`. - await pageUtils.pressKeys( 'ArrowLeft', { - times: textBeyondLinkedText.length, - } ); - - // Select the linkedText. - await pageUtils.pressKeys( 'shiftAlt+ArrowLeft' ); - - // Click on the Link button. - await editor.clickBlockToolbarButton( 'Link' ); - - // Type a URL. - await page.keyboard.type( 'https://wordpress.org/gutenberg' ); - - // Update the link. - await pageUtils.pressKeys( 'Enter' ); - await pageUtils.pressKeys( 'Escape' ); - await pageUtils.pressKeys( 'ArrowRight' ); - - // Reactivate the link. - await pageUtils.pressKeys( 'ArrowLeft' ); - await pageUtils.pressKeys( 'ArrowLeft' ); + await LinkUtils.createLink(); const linkPopover = LinkUtils.getLinkPopover(); await expect( linkPopover ).toBeVisible(); - // Make selection starting within the link and moving beyond boundary to the left. - await pageUtils.pressKeys( 'shiftAlt+ArrowLeft', { - times: linkedText.length, - } ); - - // The Link UI should have disappeared (i.e. be inactive). - await expect( linkPopover ).toBeHidden(); - - // Cancel selection and move back within the Link. - await pageUtils.pressKeys( 'ArrowRight' ); - - // We should see the Link UI displayed again. - await expect( linkPopover ).toBeVisible(); + const optionsButton = page + .getByRole( 'region', { name: 'Editor top bar' } ) + .getByRole( 'button', { name: 'Options' } ); - // Make selection starting within the link and moving beyond boundary to the right. - await pageUtils.pressKeys( 'shift+ArrowRight', { - times: 3, - } ); + await optionsButton.click(); - // The Link UI should have disappeared (i.e. be inactive). await expect( linkPopover ).toBeHidden(); - } ); - - test( 'should not show the Link UI when selection extends into another link', async ( { - page, - pageUtils, - editor, - LinkUtils, - } ) => { - const linkedTextOne = `Gutenberg`; - const linkedTextTwo = `Block Editor`; - const linkOneURL = 'https://wordpress.org'; - const linkTwoURL = 'https://wordpress.org/gutenberg'; - - // Create a block with some text. - await editor.insertBlock( { - name: 'core/paragraph', - } ); - await page.keyboard.type( - `This is the ${ linkedTextOne }${ linkedTextTwo }` - ); - - // Select the linkedTextTwo. - await pageUtils.pressKeys( 'shift+ArrowLeft', { - times: linkedTextTwo.length, - } ); - - // Click on the Link button. - await editor.clickBlockToolbarButton( 'Link' ); - - // Type a URL. - await page.keyboard.type( linkTwoURL ); - - // Update the link. - await pageUtils.pressKeys( 'Enter' ); - await pageUtils.pressKeys( 'Escape' ); - - // Move cursor next to the **end** of `linkTextOne` - await pageUtils.pressKeys( 'ArrowLeft' ); - - // Select `linkTextOne` - await pageUtils.pressKeys( 'shiftAlt+ArrowLeft' ); - - // Click on the Link button. - await editor.clickBlockToolbarButton( 'Link' ); - - // Type a URL. - await page.keyboard.type( linkOneURL ); - - // Update the link. - await pageUtils.pressKeys( 'Enter' ); + // Expect focus on Top toolbar button within dropdown + await expect( + page.getByRole( 'menuitemcheckbox', { + name: 'Top toolbar Access all block and document tools in a single place', + } ) + ).toBeFocused(); + // Press Escape await pageUtils.pressKeys( 'Escape' ); - await pageUtils.pressKeys( 'ArrowRight' ); - - // Move cursor within `linkTextOne` - await pageUtils.pressKeys( 'ArrowLeft', { - times: 3, - } ); - - const linkPopover = LinkUtils.getLinkPopover(); - - // Link UI should activate for `linkTextOne` - await expect( linkPopover ).toBeVisible(); - - // Expand selection so that it overlaps with `linkTextTwo` - await pageUtils.pressKeys( 'Shift+ArrowRight', { - times: 6, - } ); - - // Link UI should be inactive. - await expect( linkPopover ).toBeHidden(); + // Expect focus on Top toolbar Options button + await expect( optionsButton ).toBeFocused(); } ); // Based on issue reported in https://github.com/WordPress/gutenberg/issues/41771/. From abf7fccfc1f5f8c3658732f704f99a0f9caae017 Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Thu, 25 Jan 2024 12:12:35 -0600 Subject: [PATCH 31/31] Hopefully fix test that is passing locally but failing on github --- test/e2e/specs/editor/blocks/links.spec.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/e2e/specs/editor/blocks/links.spec.js b/test/e2e/specs/editor/blocks/links.spec.js index a80c2304ac7fc..2e6681c2d0701 100644 --- a/test/e2e/specs/editor/blocks/links.spec.js +++ b/test/e2e/specs/editor/blocks/links.spec.js @@ -718,11 +718,10 @@ test.describe( 'Links', () => { await page.keyboard.press( 'Escape' ); // Move to edge of text "Gutenberg". - await pageUtils.pressKeys( 'shiftAlt+ArrowLeft' ); // If you just use Alt here it won't work on windows. await pageUtils.pressKeys( 'ArrowLeft' ); - + await pageUtils.pressKeys( 'ArrowLeft' ); // Select "Gutenberg". - await pageUtils.pressKeys( 'shiftAlt+ArrowRight' ); + await pageUtils.pressKeys( 'shiftAlt+ArrowLeft' ); // If you just use Alt here it won't work on windows. // Create a link. await pageUtils.pressKeys( 'primary+k' );