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

Navigation: Improve UX on adding links #19686

Merged
merged 10 commits into from
Jan 31, 2020
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'
);
jeryj marked this conversation as resolved.
Show resolved Hide resolved
}

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