Pass block attributes with rendering with location

[pbking]

Description

When the Navigation block is rendering via the __unstableLocation attribute the attributes are now passed to the rendering function. The rendering function was already expecting 'block_attribute' this change just passes it along.

This fixes a theme issue for Quadrat (or any theme that uses _unstableLocation and also expects mobile styling which is achived by setting the isResponsive attribute).

How has this been tested?

Activate Quadrat (or any theme that uses both __unstableLocation and isResponsive attributes).
Observe that the responsive (mobile) styling works as expected.

Types of changes

Bug fix

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• My code follows the accessibility standards.
• I've tested my changes with keyboard and screen readers.
• My code has proper inline documentation.
• I've included developer documentation if appropriate.
• I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

[jasmussen]

My understanding of the inner workings of how location works with regards to the navigation block (compared to classic menus) is a bit superficial, but I have some high level concerns (also voiced in the past) around optimizing too much for the separate navigation screen being the long term place for menus to be handled and edited. Ultimately the navigation block is just a block, and like other blocks is edited in-place, in-context, in its final place in a full site editing theme.

However this seems like just a bug-fix to a feature already landed, so thank you for that! @draganescu or @getdave can you take a look?

[scruffian]

LGTM.

Thanks for the fix. This was in the original PR but I removed it as I didn't know what it did! Now I do!

[getdave]

Looks like a simple bug fix for a feature that's already there (if only in unstable form). Let's 🚢

[talldan]

Not sure I understand, why are the attributes needed? The handling of block attributes is only added by the Gutenberg plugin, so I don't think the theme will work properly with the plugin disabled:

gutenberg/lib/navigation.php

Lines 302 to 318 in 8a520e9

	$block_attributes = array();
	if ( isset( $args->block_attributes ) ) {
	$block_attributes = $args->block_attributes;
	}
	$navigation_block = array(
	'blockName' => 'core/navigation',
	'attrs' => $block_attributes,
	'innerBlocks' => gutenberg_convert_menu_items_to_blocks(
	isset( $menu_items_by_parent_id[0] )
	? $menu_items_by_parent_id[0]
	: array(),
	$menu_items_by_parent_id
	),
	);
	return render_block( $navigation_block );

I don't really know much about Quadrat, maybe that's ok if it's an experimental theme? My concern though is that this is making both the block and the theme rely on experimental features developed for the navigation screen, which could be changed or removed at short notice.

I personally wouldn't have anticipated that any changes I make for the navigation screen could cause a block in a theme's template to fail unless I'd seen this PR beforehand—so that should make clear why this is risky.

[scruffian]

At the moment I don't think the navigation block works with the plugin anyway, does it?

[draganescu]

I want to second @talldan and have explained my understanding here that this is not a good idea. I still think we should not send block attributes to the core function that renders classic menus. It makes a mishmash of things :)

IMO we should revert this 🙏🏻

[pbking]

The reason that the attributes must be passed to the rendering is so that things the 'responsive' flag can be controlled. Otherwise important capabilities of the block are out of reach. Even minor attributes like alignment are important to achieve the correct design. From the theme design standpoint how that block comes to be populated is irrelevant (be it blocks from the FSE or menu configuration data). Themes DO need a way to express the full gamut of the navigation block attributes via block templates in addition to the freedom to collect the user navigation preferences in both ways.

Regarding the code that this technique is dependent on, I believe that it has been discussed to relocate that from the editor to another location, either another library or to the block itself. I believe that it's important that there be a single location for this logic (to turn menu configuration into blocks) and there are a number of places that are becoming dependent on it.

@draganescu is it then your suggestion that this rendering logic currently in the editor be moved to the block instead? In this way the editor is dependent on the block but the block will not be dependent on the editor?

[draganescu]

is it then your suggestion that this rendering logic currently in the editor be moved to the block instead

Yes. #33244 solved this. Thanks 🙏🏻