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

NavigationMenu: <LinkControl /> integration. #18062

Merged
merged 39 commits into from
Nov 6, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
9eaae16
link-control: doc
retrofox Oct 28, 2019
b93b07a
navigation-menu-item: replace URLPopover by LinkControl
retrofox Oct 28, 2019
c52d92f
navigation-menu: do not set link when onClose
retrofox Oct 28, 2019
33baef0
navigation-menu-item: ensuring hide popover once it closes
retrofox Oct 28, 2019
1cc27a2
navigation-menu-item: anchor LinkControl at TextControl
retrofox Oct 28, 2019
28d446e
navigation-menu-item: render external link
retrofox Oct 28, 2019
d32f027
navigation-menu-item: remove link object from attrs
retrofox Oct 28, 2019
5eb1366
navigation-menu-item: handing close link popover
retrofox Oct 29, 2019
9fa1562
navigation-menu: set text color to external link
retrofox Oct 29, 2019
9616d76
navigation-menu: apply colors correctly in edition mode
retrofox Oct 29, 2019
35e88e8
navigation-menu-item: apply text color to external link
retrofox Oct 30, 2019
2550320
fixing eslint errors/warnings
retrofox Oct 30, 2019
fbfcec6
fix camelcase warning flawlessly
retrofox Oct 30, 2019
81bf11a
navigation-menu-item: set current link as null when it changes
retrofox Oct 30, 2019
0d9637d
link-control: doc fixes
retrofox Oct 30, 2019
154d2c7
Updates setting change handler to test for specific setting and value
getdave Oct 31, 2019
ccccb5c
Remove hyperlinks from nav item labels
getdave Oct 31, 2019
9a2d2fb
Update attribute name
getdave Oct 31, 2019
c712717
Fix save output to handle new tab attribute
getdave Oct 31, 2019
56536a1
Update to simplify link update with default args
getdave Oct 31, 2019
0c965d8
Remove content accidentally re-added in merge of rebase
getdave Oct 31, 2019
80cb5e8
navigation-menu-item: set title as label when it's empty
retrofox Oct 31, 2019
b9d2195
navigation-menu-item: open LinkControl when new item
retrofox Oct 31, 2019
293bfb3
navigation-menu-item: new item workflow
retrofox Oct 31, 2019
f6ebb92
navigation-menu: rename state hook
retrofox Oct 31, 2019
3b4a3ed
navigation-menu-item: use camelCase for link ID
retrofox Oct 31, 2019
468eff8
navigation-menu-item: set edit mode style
retrofox Oct 31, 2019
e85cf06
navigation-menu-item: remove fake placeholder stuff
retrofox Oct 31, 2019
7044c24
navigation-menu-item: no use effect to Initialize is open state
retrofox Nov 1, 2019
2dfa557
navigation-menu-item: remove unneeded setting state
retrofox Nov 1, 2019
c063034
navigation-menu-item: move link control callback outside of the compo…
retrofox Nov 1, 2019
e1b64e4
navigation-menu-item: simplify closing linkcontrol
retrofox Nov 4, 2019
b77c663
navigation-menu: fixing eslint errors
retrofox Nov 4, 2019
b4ca8ca
Update packages/block-library/src/navigation-menu-item/edit.js
getdave Nov 5, 2019
cf3d2c8
Update packages/block-library/src/navigation-menu-item/edit.js
getdave Nov 5, 2019
2c7b597
Fixes bug where Link UI was visually divorced from target Menu Item
getdave Nov 5, 2019
15bf1b2
Updates to remove unwanted prop being passed to LinkControl
getdave Nov 5, 2019
f60ae05
navigation-menu: remove `destination` attr
retrofox Nov 5, 2019
c34be01
navigation-menu-item: cleanup timer once unmount
retrofox Nov 5, 2019
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
13 changes: 13 additions & 0 deletions packages/block-editor/src/components/link-control/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,19 @@ through of its function parameter.
- Type: `Function`
- Required: No

Use this callback to take an action after a user set or updated a link.
The function callback will receive the selected item, or Null.

```es6
<LinkControl
onLinkChange={ ( item ) => {
item
? console.log( `The item selected has the ${ item.id } id.` )
: console.warn( 'No Item selected.' );
}
/>
```

### onSettingChange

- Type: `Function`
Expand Down
6 changes: 6 additions & 0 deletions packages/block-library/src/navigation-menu-item/block.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,15 @@
"title": {
"type": "string"
},
"type": {
"type": "string"
},
"description": {
"type": "string"
},
"linkId": {
"type": "number"
},
"opensInNewTab": {
"type": "boolean",
"default": false
Expand Down
173 changes: 101 additions & 72 deletions packages/block-library/src/navigation-menu-item/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,41 @@ import {
BlockControls,
InnerBlocks,
InspectorControls,
URLPopover,
RichText,
__experimentalLinkControl as LinkControl,
} from '@wordpress/block-editor';
import {
Fragment,
useRef,
useState,
} from '@wordpress/element';
import { Fragment, useState, useEffect } from '@wordpress/element';

/**
* It updates the link attribute when the
* link settings changes.
*
* @param {Function} setter Setter attribute function.
*/
const updateLinkSetting = ( setter ) => ( setting, value ) => {
if ( setting === 'new-tab' ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR will become relevant here.

#18285

setter( { opensInNewTab: value } );
}
};

/**
* Updates the link attribute when it changes
* through of the `onLinkChange` LinkControl callback.
*
* @param {Function} setter Setter attribute function.
* @param {string} label ItemMenu link label.
*/
const updateLink = ( setter, label ) => ( { title: newTitle = '', url: newURL = '' } = {} ) => {
setter( {
title: newTitle,
url: newURL,
} );

// Set the item label as well if it isn't already defined.
if ( ! label ) {
setter( { label: newTitle } );
}
};

function NavigationMenuItemEdit( {
attributes,
Expand All @@ -50,40 +77,48 @@ function NavigationMenuItemEdit( {
setAttributes,
insertMenuItemBlock,
} ) {
const [ isLinkOpen, setIsLinkOpen ] = useState( false );
const [ isEditingLink, setIsEditingLink ] = useState( false );
const [ urlInput, setUrlInput ] = useState( null );
const { label, opensInNewTab, title, url } = attributes;
const link = title ? { title, url } : null;
getdave marked this conversation as resolved.
Show resolved Hide resolved
const [ isLinkOpen, setIsLinkOpen ] = useState( ! label && isSelected );

const inputValue = urlInput !== null ? urlInput : url;
let onCloseTimerId = null;

const onKeyDown = ( event ) => {
if ( [ LEFT, DOWN, RIGHT, UP, BACKSPACE, ENTER ].indexOf( event.keyCode ) > -1 ) {
// Stop the key event from propagating up to ObserveTyping.startTypingInTextField.
event.stopPropagation();
/**
* It's a kind of hack to handle closing the LinkControl popover
* clicking on the ToolbarButton link.
*/
useEffect( () => {
if ( ! isSelected ) {
setIsLinkOpen( false );
}
};

const closeURLPopover = () => {
setIsEditingLink( false );
setUrlInput( null );
setIsLinkOpen( false );
};
return () => {
// Clear LinkControl.OnClose timeout.
if ( onCloseTimerId ) {
clearTimeout( onCloseTimerId );
}
};
}, [ isSelected ] );

const autocompleteRef = useRef( null );
/**
* `onKeyDown` LinkControl handler.
* It takes over to stop the event propagation to make the
* navigation work, avoiding undesired behaviors.
* For instance, it will block to move between menu items
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess other use cases of LinkControl will face similar issues. Can we address this problem at the LinkControl level?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, it's a topic that has been quite discussed in the development of the LinkControl. At that moment we decided to delegate event handling to the component which consumes it.

However, it's a valid suggestion and we could consider delegating this handling to the LinkControl itself. We will need to analyze most scenarios as possible in order to avoid blocking some desired behaviors. For instance, how it should act in a paragraph context?

* when the LinkControl is focused.
*
* @param {Event} event
*/
const handleLinkControlOnKeyDown = ( event ) => {
const { keyCode } = event;

const onFocusOutside = ( event ) => {
const autocompleteElement = autocompleteRef.current;
if ( autocompleteElement && autocompleteElement.contains( event.target ) ) {
return;
if ( [ LEFT, DOWN, RIGHT, UP, BACKSPACE, ENTER ].indexOf( keyCode ) > -1 ) {
// Stop the key event from propagating up to ObserveTyping.startTypingInTextField.
event.stopPropagation();
}
closeURLPopover();
};

const stopPropagation = ( event ) => {
event.stopPropagation();
};

const { label, url } = attributes;
const itemLabelPlaceholder = __( 'Add item…' );

return (
<Fragment>
Expand All @@ -93,51 +128,29 @@ function NavigationMenuItemEdit( {
name="link"
icon="admin-links"
title={ __( 'Link' ) }
onClick={ () => setIsLinkOpen( ! isLinkOpen ) }
onClick={ () => {
if ( isLinkOpen ) {
return;
}
setIsLinkOpen( ! isLinkOpen );
} }
/>
{ <ToolbarButton
<ToolbarButton
name="submenu"
icon={ <SVG xmlns="http://www.w3.org/2000/svg" width="24" height="24"><Path d="M14 5h8v2h-8zm0 5.5h8v2h-8zm0 5.5h8v2h-8zM2 11.5C2 15.08 4.92 18 8.5 18H9v2l3-3-3-3v2h-.5C6.02 16 4 13.98 4 11.5S6.02 7 8.5 7H12V5H8.5C4.92 5 2 7.92 2 11.5z" /><Path fill="none" d="M0 0h24v24H0z" /></SVG> }
title={ __( 'Add submenu item' ) }
onClick={ insertMenuItemBlock }
/> }
/>
</Toolbar>
{ isLinkOpen &&
<>
<URLPopover
className="wp-block-navigation-menu-item__inline-link-input"
onClose={ closeURLPopover }
onFocusOutside={ onFocusOutside }
>
{ ( ! url || isEditingLink ) &&
<URLPopover.LinkEditor
value={ inputValue }
onChangeInputValue={ setUrlInput }
onKeyPress={ stopPropagation }
onKeyDown={ onKeyDown }
onSubmit={ ( event ) => event.preventDefault() }
autocompleteRef={ autocompleteRef }
/>
}
{ ( url && ! isEditingLink ) &&
<URLPopover.LinkViewer
onKeyPress={ stopPropagation }
url={ url }
/>
}

</URLPopover>
</>
}
</BlockControls>
<InspectorControls>
<PanelBody
title={ __( 'Menu Settings' ) }
>
<ToggleControl
checked={ attributes.opensInNewTab }
onChange={ ( opensInNewTab ) => {
setAttributes( { opensInNewTab } );
onChange={ ( newTab ) => {
setAttributes( { opensInNewTab: newTab } );
} }
label={ __( 'Open in new tab' ) }
/>
Expand All @@ -154,8 +167,8 @@ function NavigationMenuItemEdit( {
>
<TextControl
value={ attributes.title || '' }
onChange={ ( title ) => {
setAttributes( { title } );
onChange={ ( itemTitle ) => {
setAttributes( { title: itemTitle } );
} }
label={ __( 'Title Attribute' ) }
help={ __( 'Provide more context about where the link goes.' ) }
Expand Down Expand Up @@ -186,13 +199,29 @@ function NavigationMenuItemEdit( {
'is-selected': isSelected,
} ) }
>
<RichText
className="wp-block-navigation-menu-item__content"
value={ label }
onChange={ ( labelValue ) => setAttributes( { label: labelValue } ) }
placeholder={ __( 'Add item…' ) }
withoutInteractiveFormatting
/>
<div className="wp-block-navigation-menu-item__inner">
<RichText
className="wp-block-navigation-menu-item__content"
value={ label }
onChange={ ( labelValue ) => setAttributes( { label: labelValue } ) }
placeholder={ itemLabelPlaceholder }
withoutInteractiveFormatting
/>
{ isLinkOpen && (
<LinkControl
className="wp-block-navigation-menu-item__inline-link-input"
onKeyDown={ handleLinkControlOnKeyDown }
onKeyPress={ ( event ) => event.stopPropagation() }
currentLink={ link }
onLinkChange={ updateLink( setAttributes, label ) }
onClose={ () => {
onCloseTimerId = setTimeout( () => setIsLinkOpen( false ), 100 );
} }
currentSettings={ { 'new-tab': opensInNewTab } }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR will cause this to break

#18285

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries, it just happens. Let's see which ships first and then let's simply update the looser :-D

onSettingsChange={ updateLinkSetting( setAttributes ) }
/>
) }
</div>
<InnerBlocks
allowedBlocks={ [ 'core/navigation-menu-item' ] }
renderAppender={ hasDescendants ? InnerBlocks.ButtonBlockAppender : false }
Expand Down
33 changes: 28 additions & 5 deletions packages/block-library/src/navigation-menu-item/editor.scss
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,36 @@
}

.wp-block-navigation-menu-item {
font-family: inherit;
font-size: inherit;
background-color: var(--background-color-menu-link);
color: var(--color-menu-link);
&:not(.is-editing) {
font-family: inherit;
font-size: inherit;
background-color: var(--background-color-menu-link);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a specific problem form this PR, but I think we can not use CSS variables as they are not supported by IE11 https://caniuse.com/#feat=css-variables.
I guess given that it was an existing problem, this is not a blocker at all, but later we should apply some refactoring.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The decision for the nav menu block was to implement it using forward looking tech and simply gracefully degrade it for IE, in this case not showing the color controls at all.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually we've created a PR that removes the CSS vars.

color: var(--color-menu-link);
width: inherit;

.components-external-link {
color: var(--color-menu-link);
}
}

&:focus {
&.is-editing .components-text-control__input {
width: inherit;
background-color: var(--background-color-menu-link);
color: var(--color-menu-link);

.components-external-link {
color: var(--color-menu-link);
&:focus {
color: var(--color-menu-link);
}
}
}

&.is-editing,
&.is-selected {
box-shadow: 0 0 1px $light-gray-900 inset;
border-radius: 4px;
min-width: 30px;
}
}

Expand Down
16 changes: 11 additions & 5 deletions packages/block-library/src/navigation-menu/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,17 @@ function NavigationMenu( {
if ( ! pages ) {
return null;
}
return pages.map( ( page ) => {
return [ 'core/navigation-menu-item',
{ label: page.title.rendered, url: page.permalink_template },
];
} );

return pages.map( ( { title, type, link: url, id: linkId } ) => (
[ 'core/navigation-menu-item', {
label: title.rendered,
title: title.raw,
type,
linkId,
url,
opensInNewTab: false,
} ]
) );
},
[ pages ]
);
Expand Down
9 changes: 9 additions & 0 deletions packages/block-library/src/navigation-menu/index.php
Original file line number Diff line number Diff line change
Expand Up @@ -102,17 +102,26 @@ function build_navigation_menu_html( $block, $colors ) {
class="wp-block-navigation-menu-item__link ' . $colors['text_css_classes'] . '"
' . $colors['text_inline_styles'];

// Start appending HTML attributes to anchor tag.
if ( isset( $menu_item['attrs']['url'] ) ) {
$html .= ' href="' . $menu_item['attrs']['url'] . '"';
}
if ( isset( $menu_item['attrs']['title'] ) ) {
$html .= ' title="' . $menu_item['attrs']['title'] . '"';
}

if ( isset( $menu_item['attrs']['opensInNewTab'] ) && true === $menu_item['attrs']['opensInNewTab'] ) {
$html .= ' target="_blank" ';
}
// End appending HTML attributes to anchor tag.

// Start anchor tag content.
$html .= '>';
if ( isset( $menu_item['attrs']['label'] ) ) {
$html .= $menu_item['attrs']['label'];
}
$html .= '</a>';
// End anchor tag content.

if ( count( (array) $menu_item['innerBlocks'] ) > 0 ) {
$html .= build_navigation_menu_html( $menu_item, $colors );
Expand Down