Navigation Editor: Navigation links added to menu only preserve url and label attributes

[gwwar]

I'm not sure if this is intentional or not, but we only preserve the label and url attributes of a navigation link, when we save this in the navigation editor.

This means saving in the navigation editor is a bit lossy. For example, draft links will render on the frontend because id and type is not available to check. This may also be an issue for any future functionality that's added to navigation links.

Post links and other types of links added in customer or nav-menu also lose information when saved in the navigation editor and get turned into custom links.

Enabling the Navigation Editor

To enable the Navigation Editor:

• Install and activate the Gutenberg plugin
• Visit experiments /wp-admin/admin.php?page=gutenberg-experiments
• Check "Enable Navigation screen" and save

Post links from customizer/nav menu turning into custom links

menu-3.mp4

• Add a post link in customizer (/wp-admin/customize.php) or menu nav (/wp-admin/nav-menus.php) and save
• Load the menu in the Navigation Editor (/wp-admin/admin.php?page=gutenberg-navigation) and Save
• Load it again in customizer or menu nav

Expected: Menu contains a single post link
Actual: Menu contains a custom link

Losing Properties When Loading a Menu into the Navigation Block

menu.mp4

As a quick example here's a navigation block loaded from a menu:

<!-- wp:navigation {"orientation":"horizontal"} -->
<!-- wp:navigation-link {"label":"Metal Cats","url":"http://wordpress.local/metal-cats/"} /-->
<!-- /wp:navigation -->

And here's one built in the post editor:

<!-- wp:navigation {"orientation":"horizontal"} -->
<!-- wp:navigation-link {"label":"Metal Cats","type":"post","id":43,"url":"http://wordpress.local/metal-cats/"} /-->
<!-- /wp:navigation -->

If I spotted this correctly serialization/hydration occurs in:

gutenberg/packages/edit-navigation/src/store/utils.js

Lines 132 to 147 in b7f3416

	function blockToRequestItem( block, parentId, position ) {
	const menuItem = omit( getMenuItemForBlock( block ), 'menus', 'meta' );
	let attributes;
	if ( block.name === 'core/navigation-link' ) {
	attributes = {
	type: 'custom',
	title: block.attributes?.label,
	original_title: '',
	url: block.attributes.url,
	description: block.attributes.description,
	xfn: block.attributes.rel?.split( ' ' ),
	classes: block.attributes.className?.split( ' ' ),
	attr_title: block.attributes.title,
	};

gutenberg/packages/edit-navigation/src/store/resolvers.js

Lines 146 to 155 in b7f3416

	const attributes = {
	label: menuItem.title.rendered,
	url: menuItem.url,
	title: menuItem.attr_title,
	className: menuItem.classes.join( ' ' ),
	description: menuItem.description,
	rel: menuItem.xfn.join( ' ' ),
	};
	return createBlock( 'core/navigation-link', attributes, innerBlocks );

We might also need server changes if menu items cannot save arbitrary properties.

[getdave]

@gwwar This is related. I found the same issue.

#30956

Perhaps we can try and persist the relevant fields from the block to those on the Nav Menu object.

https://core.trac.wordpress.org/browser/tags/5.7.1/src/wp-includes/nav-menu.php#L786

For example, object and object_id should be ideal for storing some of the data you refer to. Then when we hydrate the menu item back into a block form we can decide on the type of block based on the data we have available.

[getdave]

Just noticed that there is just a single core/navigation-link block and that has several variations. So that should make this easier 🤞

[getdave]

@gwwar I have a PR up which I believe will address this but I'd be interested in your advice.

Essentially we have two places in code where we are creating core/navigation-link blocks from menu objects stored in the database (as opposed to as serialized blocks as we do with standard blocks):

1. In the Navigation Editor:
   

   gutenberg/packages/edit-navigation/src/store/resolvers.js

   Lines 146 to 155 in b7f3416

   	const attributes = {
   	label: menuItem.title.rendered,
   	url: menuItem.url,
   	title: menuItem.attr_title,
   	className: menuItem.classes.join( ' ' ),
   	description: menuItem.description,
   	rel: menuItem.xfn.join( ' ' ),
   	};
   	return createBlock( 'core/navigation-link', attributes, innerBlocks );

2: In the Navigation Block itself:

gutenberg/packages/block-library/src/navigation/placeholder.js

Lines 40 to 83 in 1420935

	function mapMenuItemsToBlocks( menuItems ) {
	return menuItems.map( ( menuItem ) => {
	if ( menuItem.type === 'block' ) {
	const [ block ] = parse( menuItem.content.raw );
	if ( ! block ) {
	return createBlock( 'core/freeform', {
	content: menuItem.content,
	} );
	}
	return block;
	}
	const attributes = {
	label: ! menuItem.title.rendered
	? __( '(no title)' )
	: menuItem.title.rendered,
	opensInNewTab: menuItem.target === '_blank',
	};
	if ( menuItem.url ) {
	attributes.url = menuItem.url;
	}
	if ( menuItem.description ) {
	attributes.description = menuItem.description;
	}
	if ( menuItem.xfn?.length && some( menuItem.xfn ) ) {
	attributes.rel = menuItem.xfn.join( ' ' );
	}
	if ( menuItem.classes?.length && some( menuItem.classes ) ) {
	attributes.className = menuItem.classes.join( ' ' );
	}
	const innerBlocks = menuItem.children?.length
	? mapMenuItemsToBlocks( menuItem.children )
	: [];
	return createBlock( 'core/navigation-link', attributes, innerBlocks );
	} );
	}

There is some lack of consistency between these approaches and we are also missing a number of opportunities to parse all the available data from the menu objects in the database when we convert to blocks. That is what you noticed in your Issue report.

My PR addresses some of this. But I wonder whether you think a good follow up might be to create a single source of truth for the act of parsing database menu objects into blocks? The current code is not DRY and has proven to be a maintenance issue - therefore I believe it is a good candidate for a refactor.