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

New block: Custom Navigation Menu #5036

Closed
wants to merge 19 commits into from

Conversation

ryelle
Copy link
Contributor

@ryelle ryelle commented Feb 13, 2018

This PR adds a new block, "Navigation Menu". This is the basic version mocked up in this issue: #1466 (comment). The block itself is mostly done, only some questions (marked ⚠️in this issue) & tests remain. This also requires the patch on 40878 applied to your WP install (if this PR gets a 👍, perhaps we could patch these API changes into gutenberg for now).

This creates a dynamic block for the front end of the site, so that items added to the selected menus are updated automatically.

Screenshots

When first added, the block inserts a placeholder with a menu dropdown:

initial

After selecting a menu, it populates the block with the items, in a vertical list or horizontal nav-bar style.

vertical

horizontal

⚠️We need different icons for vertical & horizontal (currently using the list/grid pair from latest posts).

If the list is empty, it says so in the editor, but doesn't display anything on the front end.

empty

⚠️The link in this placeholder should go to the site's customizer, but I'm not sure how to get that.

The menu select is also added to the inspector controls area, so that the menu can be updated once the block is added.

controls

On the front end of the site, the menu is displayed in the post using minimal styling.

front-end-vertical

front-end-horizontal

Testing

As I mentioned, no tests are written for this yet. To manually test, you can add a Navigation block to your posts with different menus: a nested menu, empty menu, long menu, add a menu then delete it, etc.

Other notes

⚠️We should add some JS to the front end when this block is present to enable the keyboard navigation. This should also be added to the editor, perhaps to the react item component itself.

⚠️Not done in this version, but could be later -- a toggle for showing the item description.

export const settings = {
title: __( 'Navigation Menu' ),

description: __( 'Display your site pages.' ),
Copy link
Member

Choose a reason for hiding this comment

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

Menus are more then just pages, so maybe 'Shows a list of Menu items'?


renderMenu() {
const { data, isLoading } = this.props.items;
const customizerUrl = '';
Copy link
Member

Choose a reason for hiding this comment

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

This could be just customize.php?autofocus[panel]=nav_menus, since Gutenberg is loaded at wp-admin/post.php, a relative link will go to the right place.

>
{ ! Array.isArray( data ) ?
<Spinner /> :
<Button href={ `${ customizerUrl }?autofocus%5Bpanel%5D=nav_menus` }>
Copy link
Member

Choose a reason for hiding this comment

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

This should probably also have target="_blank"?

icon="menu"
label={ __( 'Navigation Menu' ) }
>
{ ! Array.isArray( data ) ?
Copy link
Member

Choose a reason for hiding this comment

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

The link to edit menus would be useful even when there are menus already created. What if none of the existing menus are the ones you want?

<Spinner /> :
<Button href={ `${ customizerUrl }?autofocus%5Bpanel%5D=nav_menus` }>
{ __( 'No items found in this menu.' ) }
</Button>
Copy link
Member

Choose a reason for hiding this comment

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

Also, should there be a refresh link to re-fetch menus once a user has created one? This could be useful regardless of whether there were menus to begin with.

register_block_type( 'core/navigation-menu', array(
'attributes' => array(
'selected' => array(
'type' => 'string',
Copy link
Member

Choose a reason for hiding this comment

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

Should this not be an int? I see it is casted to an int above.

);

if ( !! selected ) {
displayBlock = this.renderMenu();
Copy link
Member

Choose a reason for hiding this comment

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

We could avoid having to implement renderMenu here which is a duplication of wp_nav_menu() if we used ServerSideRender proposed in #780.

@ryelle ryelle force-pushed the add/custom-menu-block branch from ead8b10 to 1bf805e Compare March 15, 2018 13:51
@ryelle
Copy link
Contributor Author

ryelle commented Mar 15, 2018

I've rebased & addressed the feedback so far, and will keep an eye on #5602 to use the server-side rendering 🙂

@mrleemon
Copy link
Contributor

mrleemon commented May 4, 2018

Just a question. Is it planned to include the patch on 40878 before WP 5.0?

@tofumatt
Copy link
Member

tofumatt commented Jul 5, 2018

The server-side render component landed in #5602… is that the only thing blocking this from getting merged? 😄

@JodiWarren
Copy link

I don't believe this can be merged before Trac #40878 is merged into core, as /wp/v2/menus will be a missing route until it's included.

@melchoyce
Copy link
Contributor

A lot's changed since @ryelle started working on this. Let's close and revisit navigation menus in Phase 2 of Gutenberg.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants