Skip to content

Commit

Permalink
Navigation: Change UX to move focus to navigation link label and clos…
Browse files Browse the repository at this point in the history
…e the LinkControl (#19686)

* When adding a link via the LinkControl, selects/highlights nav link label text if it's url-like. Focuses if not. Automatically adds url-like labels as the label.

* Adds @wordpress/dom to package-lock.json

* 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
  • Loading branch information
jeryj committed Jan 31, 2020
1 parent 362ab68 commit 06bd266
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 18 deletions.
2 changes: 2 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions packages/block-library/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
55 changes: 52 additions & 3 deletions packages/block-library/src/navigation-link/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,9 @@ import {
RichText,
__experimentalLinkControl as LinkControl,
} from '@wordpress/block-editor';
import { Fragment, useState, useEffect } from '@wordpress/element';

import { isURL, prependHTTP } from '@wordpress/url';
import { Fragment, useState, useEffect, useRef } from '@wordpress/element';
import { placeCaretAtHorizontalEdge } from '@wordpress/dom';
/**
* Internal dependencies
*/
Expand Down Expand Up @@ -61,6 +62,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)
Expand All @@ -72,6 +74,49 @@ 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 text.
useEffect( () => {
if ( isLinkOpen && url ) {
// Close the link.
setIsLinkOpen( false );

// 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 ] );

/**
* 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 (
<Fragment>
<BlockControls>
Expand Down Expand Up @@ -155,6 +200,7 @@ function NavigationLinkEdit( {
>
<div className="wp-block-navigation-link__content">
<RichText
ref={ ref }
tagName="span"
className="wp-block-navigation-link__label"
value={ label }
Expand Down Expand Up @@ -209,8 +255,11 @@ function NavigationLinkEdit( {
label !== newTitle
) {
return escape( newTitle );
} else if ( label ) {
return label;
}
return label;
// If there's no label, add the URL.
return escape( normalizedURL );
} )(),
opensInNewTab: newOpensInNewTab,
id,
Expand Down
28 changes: 13 additions & 15 deletions packages/e2e-tests/specs/editor/blocks/navigation.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ import {
createNewPost,
getEditedPostContent,
insertBlock,
pressKeyWithModifier,
setUpResponseMocking,
clickBlockToolbarButton,
pressKeyWithModifier,
} from '@wordpress/e2e-test-utils';

async function mockPagesResponse( pages ) {
Expand Down Expand Up @@ -65,23 +65,21 @@ 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 ) {
await page.click( '.is-selected .wp-block-navigation-link__label' );

// 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 );
}
}
Expand Down

0 comments on commit 06bd266

Please sign in to comment.