Navigation: Surface menu name in the List View next to the Navigation block

[yogeshbhutkar]

Fixes Point 2 of #68177

What, Why and How?

This PR enhances the Navigation block by displaying the menu name alongside the Navigation text. It achieves this by introducing a new menuTitle attribute, initialized during block rendering. The label is displayed using the __experimentalLabel attribute, passed to the initBlock method for rendering.

Testing Instructions

1. Create a Navigation Block.
2. Try renaming it.
3. Notice the name change reflected in the List View (left side) next to the Navigation text.

Screencast

Screenshots

Before	After

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: yogeshbhutkar <yogeshbhutkar@git.wordpress.org>
Co-authored-by: fabiankaegy <fabiankaegy@git.wordpress.org>
Co-authored-by: t-hamano <wildworks@git.wordpress.org>
Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[yogeshbhutkar]

Hi @getdave, can you please review this PR when you get a moment? Thanks!

[fabiankaegy]

Thank you for working on this :) The ens result you have is great and really useful. However the way that it is currently implemented is not ideal.

Instead of storing the title of the menu in an attribute we should instead use the actual title of the post connected to the navigation block via the ref. That way if someone renames the navigation the title stays accurate and the value cannot get out of sync.

[fabiankaegy]

Wow! That was quick :) Thanks for the update. Left 2 more small notes :)

[fabiankaegy]

From my perspective, this looks great and is a nice enhancement. I'll wait for @getdave @Mamaduka or @t-hamano to do another pass at the review though 👍

[yogeshbhutkar]

Thanks for the review @fabiankaegy.

[t-hamano]

Thanks for the PR! I think this is a fantastic improvement.

My suggestion is to simply display only the name of the navigation menu. In other words, {navigationName} instead of Navigation ({navigationName}). What do you think?

This is because in the initial installation of WordPress it is displayed as follows:

Also, the Template Part block only displays the name of the template part.

Additionally, we need to decide whether the new name should apply only to the List View. In this PR, the new name is also applied to the block toolbar:

If we want the new name to apply only to the List View, we need to check the context prop (Example).

[t-hamano]

The block title may have changed. Example code:

function change_navigation_block_name( $args, $block_type ) {
	if ( 'core/navigation' !== $block_type ) {
		return $args;
	}
	$args['title'] = __( 'My Navigation!' );
	return $args;
}
add_filter( "register_block_type_args", "change_navigation_block_name", 10, 2 );

Should this possibility be considered?

[Mamaduka]

You can return undefined here. This will signal the block editor to display the block's default title.

See the Template Part block for a similar example:

gutenberg/packages/block-library/src/template-part/index.js

Lines 31 to 33 in e746a95

	if ( ! slug ) {
	return;
	}

[t-hamano]

My suggestion is to simply display only the name of the navigation menu. In other words, {navigationName} instead of Navigation ({navigationName}). What do you think?

Another suggestion on this:

• If the block title and navigation name are the same, display Navigation
• If not, display Navigation ({navigationName})

[Mamaduka]

Same here. Let's return undefined when Navigation has no title. You can use early return to make things more readable, IMO.

[Mamaduka]

The navigation.title === metadata.title condition will never be true. The Navigation block has its renaming system and disabled renaming support.

[yogeshbhutkar]

If the block title and navigation name are the same, display Navigation.

I was attempting to achieve this. It works fine when both the texts are Navigation but if the label is changed using the following snippet, it fails:

function change_navigation_block_name( $args, $block_type ) {
	if ( 'core/navigation' !== $block_type ) {
		return $args;
	}
	$args['title'] = __( 'My Navigation!' );
	return $args;
}
add_filter( "register_block_type_args", "change_navigation_block_name", 10, 2 );

[fabiankaegy]

@Mamaduka I think this is for the case when the Navigation menu is named Navigation (which is the default name).

Because we don't want the label to read Navigation (Navigation)

So that is what this comparison is for

[yogeshbhutkar]

I feel we can use the runtime title for navigation block:

const runtimeBlockTitle =
	select( blocksStore ).getBlockType( 'core/navigation' )?.title;

This way, even if the filter updates the title, we can accommodate for the changes.

Like, if the title becomes My Navigation, then My Navigation (My Navigation) => My Navigation case will be covered. However, the above condition works just fine if we want to check Navigation itself, as the name is relatively static. I'm not sure if we should accommodate for the title changes on the fly (via filter) for this one. If so, this is the way to go.

What are your thoughts?

[Mamaduka]

I think we should just drop the block title from here and only display the navigation title. No other block (Template Part, Synced Pattern) does this. Let's keep the behavior consistent.

@t-hamano, also suggested the same: #68446 (comment).

Let's just use this logic. What do you think?

return decodeEntities( navigation.title );

[yogeshbhutkar]

Looks good @Mamaduka. The icon can convey the meaning of the block IMO.

[Mamaduka]

The icon can convey the meaning of the block IMO.

That's the current convention in the List View, and the Block Card in the right sidebar displays a badge with the original block title.

[Mamaduka]

Thanks for the follow ups, @yogeshbhutkar!

I’ll try to do another round of testing when I get some time and merge the update.

[t-hamano]

This PR mostly works, but there are visual changes in the list view.

Add the following code to gutenberg.php:

function change_navigation_block_name( $args, $block_type ) {
	if ( 'core/navigation' !== $block_type ) {
		return $args;
	}
	$args['title'] = __( 'My PHP Navigation!' );
	return $args;
}
add_filter( "register_block_type_args", "change_navigation_block_name", 10, 2 );

trunk: All block titles will be updated.

This PR: Only list view block titles will not be updated.

Is this an acceptable edge case?

[Mamaduka]

Let’s see how Template Part and Sync Pattern handle this. Navigation block should do the same. I think that means removing context limitation.

P.S. I think renaming blocks via PHP definition is super rare case anyways.

[t-hamano]

Let’s see how Template Part and Sync Pattern handle this. Navigation block should do the same. I think that means removing context limitation.

Template parts, Sync pattern, the entity title are displayed consistently in all the following places.

• List View
• Block Card
• Breadcrumb
• Block Toolbar

So it might be a good idea to remove the context in the Navigation block as well.

[fabiankaegy]

Also with the change in #65641 it makes sense to remove the block name from the custom label when we display a custom label :)