From 40ad7724721c758c6b62ea784efb8fa01d3996b4 Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Wed, 22 Jan 2020 09:39:17 -0600 Subject: [PATCH 1/9] Focus link text when link url changes and LinkControl popover is open --- .../block-library/src/navigation-link/edit.js | 37 ++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/packages/block-library/src/navigation-link/edit.js b/packages/block-library/src/navigation-link/edit.js index a65d61319e590..db030d2df54f2 100644 --- a/packages/block-library/src/navigation-link/edit.js +++ b/packages/block-library/src/navigation-link/edit.js @@ -30,7 +30,7 @@ import { RichText, __experimentalLinkControl as LinkControl, } from '@wordpress/block-editor'; -import { Fragment, useState, useEffect } from '@wordpress/element'; +import { Fragment, useState, useEffect, useRef } from '@wordpress/element'; /** * Internal dependencies @@ -54,6 +54,7 @@ function NavigationLinkEdit( { }; const [ isLinkOpen, setIsLinkOpen ] = useState( false ); const itemLabelPlaceholder = __( 'Add link…' ); + const ref = useRef(); // Show the LinkControl on mount if the URL is empty // ( When adding a new menu item) @@ -65,6 +66,39 @@ function NavigationLinkEdit( { } }, [] ); + /** + * The hook shouldn't be necessary but due to a focus loss happening + * when selecting a suggestion in the link popover, we force close on block unselection. + */ + useEffect( () => { + if ( ! isSelected ) { + setIsLinkOpen( false ); + } + }, [ isSelected ] ); + + // If the LinkControl popover is open and the URL has changed, close the LinkControl and focus the label. + useEffect( () => { + if ( isLinkOpen && url ) { + // close the link + setIsLinkOpen( false ); + // focus the label + selectLabelText(); + } + }, [ url ] ); + + /** + * Focus the navigation link label text and select it. + */ + function selectLabelText( ) { + ref.current.focus(); + const selection = window.getSelection(); + const range = document.createRange(); + // Get the range of the current ref contents so we can add this range to the selection. + range.selectNodeContents( ref.current ); + selection.removeAllRanges(); + selection.addRange( range ); + } + return ( @@ -141,6 +175,7 @@ function NavigationLinkEdit( { >
Date: Wed, 22 Jan 2020 11:15:56 -0600 Subject: [PATCH 2/9] Bunk change to rerun travis --- packages/block-library/src/navigation-link/edit.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-library/src/navigation-link/edit.js b/packages/block-library/src/navigation-link/edit.js index db030d2df54f2..c27e6ef3aed3b 100644 --- a/packages/block-library/src/navigation-link/edit.js +++ b/packages/block-library/src/navigation-link/edit.js @@ -76,7 +76,7 @@ function NavigationLinkEdit( { } }, [ isSelected ] ); - // If the LinkControl popover is open and the URL has changed, close the LinkControl and focus the label. + // If the LinkControl popover is open and the URL has changed, close the LinkControl and focus the label text. useEffect( () => { if ( isLinkOpen && url ) { // close the link From e942f073e99710837f1fcf7dde44629449a7e4cc Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Wed, 22 Jan 2020 14:08:10 -0600 Subject: [PATCH 3/9] Fixed tests by removing the click and select part, as we're already moving focus and selecting now. --- .../e2e-tests/specs/editor/blocks/navigation.test.js | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/packages/e2e-tests/specs/editor/blocks/navigation.test.js b/packages/e2e-tests/specs/editor/blocks/navigation.test.js index be39b50a56a37..aadab90d21bc1 100644 --- a/packages/e2e-tests/specs/editor/blocks/navigation.test.js +++ b/packages/e2e-tests/specs/editor/blocks/navigation.test.js @@ -6,7 +6,6 @@ import { createNewPost, getEditedPostContent, insertBlock, - pressKeyWithModifier, setUpResponseMocking, clickBlockToolbarButton, } from '@wordpress/e2e-test-utils'; @@ -66,15 +65,14 @@ async function updateActiveNavigationLink( { url, label } ) { } if ( label ) { - await page.click( '.is-selected .wp-block-navigation-link__label' ); - + // With https://github.com/WordPress/gutenberg/pull/19686, we're auto-selecting the label so we don't need to manually select this here. Leaving this code in place though as we may not always do this. // Ideally this would be `await pressKeyWithModifier( 'primary', 'a' )` // to select all text like other tests do. // Unfortunately, these tests don't seem to pass on Travis CI when // using that approach, while using `Home` and `End` they do pass. - await page.keyboard.press( 'Home' ); - await pressKeyWithModifier( 'shift', 'End' ); - await page.keyboard.press( 'Backspace' ); + // await page.keyboard.press( 'Home' ); + // await pressKeyWithModifier( 'shift', 'End' ); + // await page.keyboard.press( 'Backspace' ); await page.keyboard.type( label ); } } From 2350697e0b142d35892023b9d6ee9c7b968e76fd Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Wed, 22 Jan 2020 15:36:43 -0600 Subject: [PATCH 4/9] Selects nav link label text if it's url-like. Focuses if not. Automatically adds url-like labels as the label. Put a few too many things together. --- packages/block-library/package.json | 2 + .../block-library/src/navigation-link/edit.js | 54 +++++++++++-------- 2 files changed, 33 insertions(+), 23 deletions(-) diff --git a/packages/block-library/package.json b/packages/block-library/package.json index fe1bb28e1f96e..1a99a38df771f 100644 --- a/packages/block-library/package.json +++ b/packages/block-library/package.json @@ -37,6 +37,8 @@ "@wordpress/data": "file:../data", "@wordpress/date": "file:../date", "@wordpress/deprecated": "file:../deprecated", + "@wordpress/dom": "file:../dom", + "@wordpress/editor": "file:../editor", "@wordpress/element": "file:../element", "@wordpress/escape-html": "file:../escape-html", "@wordpress/i18n": "file:../i18n", diff --git a/packages/block-library/src/navigation-link/edit.js b/packages/block-library/src/navigation-link/edit.js index c27e6ef3aed3b..e31b5770ee683 100644 --- a/packages/block-library/src/navigation-link/edit.js +++ b/packages/block-library/src/navigation-link/edit.js @@ -30,8 +30,9 @@ import { RichText, __experimentalLinkControl as LinkControl, } from '@wordpress/block-editor'; +import { isURL, prependHTTP } from '@wordpress/url'; import { Fragment, useState, useEffect, useRef } from '@wordpress/element'; - +import { placeCaretAtHorizontalEdge } from '@wordpress/dom'; /** * Internal dependencies */ @@ -79,10 +80,17 @@ function NavigationLinkEdit( { // If the LinkControl popover is open and the URL has changed, close the LinkControl and focus the label text. useEffect( () => { if ( isLinkOpen && url ) { - // close the link + // Close the link. setIsLinkOpen( false ); - // focus the label - selectLabelText(); + + // Does this look like a URL and have something TLD-ish? + if ( isURL( prependHTTP( label ) ) && /^.+\.[a-z]+/.test( label ) ) { + // Focus and select the label text. + selectLabelText(); + } else { + // Focus it (but do not select). + placeCaretAtHorizontalEdge( ref.current, true ); + } } }, [ url ] ); @@ -198,26 +206,26 @@ function NavigationLinkEdit( { url: newURL = '', opensInNewTab: newOpensInNewTab, id, - } = {} ) => - setAttributes( { - title: escape( newTitle ), - url: encodeURI( newURL ), - label: ( () => { - const normalizedTitle = newTitle.replace( /http(s?):\/\//gi, '' ); - const normalizedURL = newURL.replace( /http(s?):\/\//gi, '' ); - if ( - newTitle !== '' && - normalizedTitle !== normalizedURL && - label !== newTitle - ) { - return escape( newTitle ); - } + } = {} ) => setAttributes( { + title: escape( newTitle ), + url: encodeURI( newURL ), + label: ( () => { + const normalizedTitle = newTitle.replace( /http(s?):\/\//gi, '' ); + const normalizedURL = newURL.replace( /http(s?):\/\//gi, '' ); + if ( + newTitle !== '' && + normalizedTitle !== normalizedURL && + label !== newTitle ) { + return escape( newTitle ); + } else if ( label ) { return label; - } )(), - opensInNewTab: newOpensInNewTab, - id, - } ) - } + } + // If there's no label, add the URL. + return escape( normalizedURL ); + } )(), + opensInNewTab: newOpensInNewTab, + id, + } ) } /> ) } From d43c29859595a604a203a0536397ed121d6bacaf Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Thu, 23 Jan 2020 11:44:30 -0600 Subject: [PATCH 5/9] Fixed tests for navigation block. Test now selects the label and deletes it only if the url does not equal the label. --- .../specs/editor/blocks/navigation.test.js | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/packages/e2e-tests/specs/editor/blocks/navigation.test.js b/packages/e2e-tests/specs/editor/blocks/navigation.test.js index aadab90d21bc1..05a8bce4d4127 100644 --- a/packages/e2e-tests/specs/editor/blocks/navigation.test.js +++ b/packages/e2e-tests/specs/editor/blocks/navigation.test.js @@ -8,6 +8,7 @@ import { insertBlock, setUpResponseMocking, clickBlockToolbarButton, + pressKeyWithModifier, } from '@wordpress/e2e-test-utils'; async function mockPagesResponse( pages ) { @@ -65,14 +66,18 @@ async function updateActiveNavigationLink( { url, label } ) { } if ( label ) { - // With https://github.com/WordPress/gutenberg/pull/19686, we're auto-selecting the label so we don't need to manually select this here. Leaving this code in place though as we may not always do this. - // Ideally this would be `await pressKeyWithModifier( 'primary', 'a' )` - // to select all text like other tests do. - // Unfortunately, these tests don't seem to pass on Travis CI when - // using that approach, while using `Home` and `End` they do pass. - // await page.keyboard.press( 'Home' ); - // await pressKeyWithModifier( 'shift', 'End' ); - // await page.keyboard.press( 'Backspace' ); + // With https://github.com/WordPress/gutenberg/pull/19686, we're auto-selecting the label if the label is URL-ish. + // In this case, it means we have to select and delete the label if it's _not_ the url. + if ( label !== url ) { + // Ideally this would be `await pressKeyWithModifier( 'primary', 'a' )` + // to select all text like other tests do. + // Unfortunately, these tests don't seem to pass on Travis CI when + // using that approach, while using `Home` and `End` they do pass. + await page.keyboard.press( 'Home' ); + await pressKeyWithModifier( 'shift', 'End' ); + await page.keyboard.press( 'Backspace' ); + } + await page.keyboard.type( label ); } } From 081aacdfe59820b2e60f9a458f42a67ddede429e Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Thu, 23 Jan 2020 13:16:48 -0600 Subject: [PATCH 6/9] Adds @wordpress/dom to package-lock.json --- package-lock.json | 2 ++ 1 file changed, 2 insertions(+) diff --git a/package-lock.json b/package-lock.json index d4562580e2b47..1d5a493bf8e4e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10115,6 +10115,8 @@ "@wordpress/data": "file:packages/data", "@wordpress/date": "file:packages/date", "@wordpress/deprecated": "file:packages/deprecated", + "@wordpress/dom": "file:packages/dom", + "@wordpress/editor": "file:packages/editor", "@wordpress/element": "file:packages/element", "@wordpress/escape-html": "file:packages/escape-html", "@wordpress/i18n": "file:packages/i18n", From 997122641b0bd2ad77d6b443405fc7a435bb91fc Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Fri, 31 Jan 2020 11:06:15 -0800 Subject: [PATCH 7/9] Fixed code formatting after rebasing --- .../block-library/src/navigation-link/edit.js | 45 ++++++++++--------- 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/packages/block-library/src/navigation-link/edit.js b/packages/block-library/src/navigation-link/edit.js index e31b5770ee683..de8d0b4614581 100644 --- a/packages/block-library/src/navigation-link/edit.js +++ b/packages/block-library/src/navigation-link/edit.js @@ -97,7 +97,7 @@ function NavigationLinkEdit( { /** * Focus the navigation link label text and select it. */ - function selectLabelText( ) { + function selectLabelText() { ref.current.focus(); const selection = window.getSelection(); const range = document.createRange(); @@ -206,26 +206,29 @@ function NavigationLinkEdit( { url: newURL = '', opensInNewTab: newOpensInNewTab, id, - } = {} ) => setAttributes( { - title: escape( newTitle ), - url: encodeURI( newURL ), - label: ( () => { - const normalizedTitle = newTitle.replace( /http(s?):\/\//gi, '' ); - const normalizedURL = newURL.replace( /http(s?):\/\//gi, '' ); - if ( - newTitle !== '' && - normalizedTitle !== normalizedURL && - label !== newTitle ) { - return escape( newTitle ); - } else if ( label ) { - return label; - } - // If there's no label, add the URL. - return escape( normalizedURL ); - } )(), - opensInNewTab: newOpensInNewTab, - id, - } ) } + } = {} ) => + setAttributes( { + title: escape( newTitle ), + url: encodeURI( newURL ), + label: ( () => { + const normalizedTitle = newTitle.replace( /http(s?):\/\//gi, '' ); + const normalizedURL = newURL.replace( /http(s?):\/\//gi, '' ); + if ( + newTitle !== '' && + normalizedTitle !== normalizedURL && + label !== newTitle + ) { + return escape( newTitle ); + } else if ( label ) { + return label; + } + // If there's no label, add the URL. + return escape( normalizedURL ); + } )(), + opensInNewTab: newOpensInNewTab, + id, + } ) + } /> ) } From bb50b449d91ed9534ba2788a99c0f368f56911ab Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Fri, 31 Jan 2020 12:16:26 -0800 Subject: [PATCH 8/9] Removed test for awaiting for Link Control focus after pressing Enter, as the focus should now be on the navigation link text with the Link Control closed --- packages/e2e-tests/specs/editor/blocks/navigation.test.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/e2e-tests/specs/editor/blocks/navigation.test.js b/packages/e2e-tests/specs/editor/blocks/navigation.test.js index 05a8bce4d4127..970a1763b6f4f 100644 --- a/packages/e2e-tests/specs/editor/blocks/navigation.test.js +++ b/packages/e2e-tests/specs/editor/blocks/navigation.test.js @@ -60,9 +60,6 @@ async function updateActiveNavigationLink( { url, label } ) { await page.keyboard.press( 'ArrowDown' ); // Select the suggestion. await page.keyboard.press( 'Enter' ); - // Make sure that the dialog is still opened, and that focus is retained - // within (focusing on the link preview). - await page.waitForSelector( ':focus.block-editor-link-control__search-item-title' ); } if ( label ) { From ffa71fa78ce42643441517de8a996e0fb3c0b0b4 Mon Sep 17 00:00:00 2001 From: Jerry Jones Date: Fri, 31 Jan 2020 14:32:26 -0800 Subject: [PATCH 9/9] Linting fix --- packages/block-library/src/navigation-link/edit.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/block-library/src/navigation-link/edit.js b/packages/block-library/src/navigation-link/edit.js index ae2939a7dae75..ff55c5dabfecd 100644 --- a/packages/block-library/src/navigation-link/edit.js +++ b/packages/block-library/src/navigation-link/edit.js @@ -91,7 +91,10 @@ function NavigationLinkEdit( { setIsLinkOpen( false ); // Does this look like a URL and have something TLD-ish? - if ( isURL( prependHTTP( label ) ) && /^.+\.[a-z]+/.test( label ) ) { + if ( + isURL( prependHTTP( label ) ) && + /^.+\.[a-z]+/.test( label ) + ) { // Focus and select the label text. selectLabelText(); } else {