Refactor of navigation block rendering using location attribute#33244
Refactor of navigation block rendering using location attribute#33244draganescu merged 7 commits intotrunkfrom
Conversation
draganescu
left a comment
There was a problem hiding this comment.
Yes. This is:
a. moving things forward for the navigation block in terms of interop with classic menus
b. preserving the independence of the new navigation editor from the block and the other way around, so that we can safely experiment without breaking things
c. probably most important, provides for people building block themes today a way to use classic menus and a path for a later "migration" mechanism.
| function gutenberg_get_menu_items_at_location( $location ) { | ||
| if ( empty( $location ) ) { | ||
| return false; | ||
| return; |
There was a problem hiding this comment.
I think it might better to return an empty array when "bailing" early in this function. This way, we can have a single return type.
Mamaduka
left a comment
There was a problem hiding this comment.
I think like this direction and agree with @draganescu.
The wp_nav_menu can be filtered and have custom Nav Menu Walkers. It can be hard to account for all the scenarios/markup.
Tested with 2020, and it worked as expected 👍
One thing that was a little confusing is that the Navigation block displays a placeholder state. When __unstableLocation attribute is used.
* Return result straight from render_menu_from_location * Try converting menu items to WP_Blocks * Pass context * Refactor into separate functions * Add doc blocks * Fix typo * Return an empty string
Description
Alternative to #33224
The current way the navigation block renders content using its
__unstableLocationattribute has some issues:block-nav-menus. This is a theme option that controls the navigation screen. The block should ideally be portable and not have a dependency on how the experimental nav screen works.__unstableLocationshould just work without some other configuration.wp_nav_menu, which then eventually (because of the theme opt-in) calls back torender_block( $navigation_block ), meaning the same block rendering code is executed twice, and without some care infinite recursion can result (Remove the __unstableLocation attribute prior to rendering #33175)This PR is a bit of a refactor of how it works. The navigation block is instead in charge of its own rendering. It pulls data from a specified menu location and parses the items into blocks, replacing the normal block's innerBlocks with the parsed menu items instead.
It's just a first iteration, but I think the good this is that this could lead to changing how the navigation screen works. That screen can now just render a navigation block with an
__unstableLocationattribute when a theme has opted in, and instead have the block do all the work. I'll follow-up with a PR that does this.How has this been tested?
(I tested this using Twenty Twenty, which has a footer location and the specified colors)
appearance > menuscreen, set it to the footer locationExpected: The menu should render with the correct menu items and navigation block settings (color, typography etc) should work.
Screenshots
Types of changes
Refactor
Checklist:
*.native.jsfiles for terms that need renaming or removal).