Make WordPress Core

Opened 3 years ago

Closed 2 years ago

Last modified 19 months ago

#54684 closed enhancement (reported-upstream)

Add ability to filter navigation block items

Reported by: afragen's profile afragen Owned by:
Milestone: Priority: normal
Severity: normal Version: 5.5
Component: Menus Keywords: has-patch
Focuses: administration Cc:

Description (last modified by afragen)

There currently doesn't seem to be a simple way to add/modify blocks that are produced by the class WP_Block_List. Blocks passed to this class are used in menus in wp-includes/blocks/navigation.php.

A filter in the constructor would be a nice addition.

Naming is hard.

UPDATED: Now adds filter to navigation block items after creation not during creation of the WP_Block_List.

Change History (32)

This ticket was mentioned in PR #2079 on WordPress/wordpress-develop by afragen.


3 years ago
#1

Add a filter to the WP_Block_List class constructor to enable modification of the passed blocks. This would allow simpler modification of block menu items from the navigation block.

Trac ticket: https://core.trac.wordpress.org/ticket/54684

#2 @afragen
3 years ago

  • Milestone changed from Awaiting Review to 5.9

#3 @mukesh27
3 years ago

Hi there!

The ticket milestone set to 5.9 so please update @since x.x.x tag in PR.

#4 @audrasjb
3 years ago

  • Keywords dev-feedback added

Hi, I tested the patch and it looks like it works fine since I'm able to override the default value using the following code:

<?php
add_filter( 'block_list_blocks', function( $blocks ) {
        wp_die( var_export( $blocks ) );
        return $blocks;
}, 10, 1 );

/* Returns:
array ( 
        0 => array (
                'blockName' => 'core/navigation-link',
                'attrs' => array (
                        'label' => 'Page d’exemple', 
                        'type' => 'page', 
                        'id' => 2, 
                        'url' => 'http://localhost:8888/552/wordpress-10/page-d-exemple/', 
                        'kind' => 'post-type', 
                        'isTopLevelLink' => true,
                ),
                'innerBlocks' => array ( ), 
                'innerHTML' => '', 
                'innerContent' => array ( ), 
        ),
)
*/

add_filter( 'block_list_blocks', function( $blocks ) {
        $blocks[0]['attrs'] = array(
                'label' => 'My page One',
                'type'  => 'page',
                'id'    => 3,
                'url'   => 'http://localhost:8888/552/wordpress-10/page-d-exemple/',
                'kind'  => 'post-type',
                'isTopLevelLink' => 1
        );
        wp_die( var_export( $blocks ) );
        return $blocks;
}, 10, 1 );

/* Returns:
array ( 
        0 => array ( 
                'blockName' => 'core/navigation-link', 
                'attrs' => array ( 
                        'label' => 'My page One', 
                        'type' => 'page', 
                        'id' => 3, 
                        'url' => 'http://localhost:8888/552/wordpress-10/page-d-exemple/', 
                        'kind' => 'post-type', 
                        'isTopLevelLink' => 1, 
                ), 
                'innerBlocks' => array ( ), 
                'innerHTML' => '', 
                'innerContent' => array ( ), 
        ), 
)
*/

However, this doesn't change anything in the editor. What is the expected behavior? Shouldn’t it override the navigation block’s links? It does nothing on my side.

#5 @afragen
3 years ago

  • Keywords dev-feedback removed

The filter is only designed to modify the array of $blocks that are passed as a menu. It doesn't effect any visible component in the editor.

In addition to changing the passed array you can add to it. I have a small plugin that adds a Log in / Log out item to any displayed menu or navigation menu block. Provided I got the conditional correct 😉

https://github.com/afragen/login-logout-primary-menu-item

Last edited 3 years ago by afragen (previous) (diff)

#6 follow-up: @afragen
3 years ago

There doesn't seem to be a equivalent filter in the navigation block as wp_nav_menu_items.

Last edited 3 years ago by afragen (previous) (diff)

This ticket was mentioned in Slack in #core by afragen. View the logs.


3 years ago

This ticket was mentioned in Slack in #core by marybaum. View the logs.


3 years ago

This ticket was mentioned in Slack in #core by afragen. View the logs.


3 years ago

#10 @hellofromTonya
3 years ago

This ticket is an enhancement, though 5.9 is in feature freeze. Is this an enhancement or a defect?

The class and its constructor were introduced in 5.5.0, not in 5.9. What issue introduced in 5.9 does the new proposed filter resolve?

(The answers help to determine if the patch can be committed in 5.9 or if it should be moved to Gutenberg and 6.0 for continued discussion.)

#11 @hellofromTonya
3 years ago

  • Version changed from trunk to 5.5

Marking the Version as 5.5.0 as the constructor was introduced in 5.5.

#12 @afragen
3 years ago

  • Type changed from enhancement to defect (bug)

This ticket really fixes a regression as there doesn't seem to be any method of filtering the menu items of a block theme.

#13 in reply to: ↑ 6 @hellofromTonya
3 years ago

  • Type changed from defect (bug) to enhancement

Replying to afragen:

There doesn't seem to be a equivalent filter in the navigation block as wp_nav_menu_items.

Looking at the code in render_block_core_navigation() within blocks/navigation.php (here in Gutenberg and here in Core), WP_Block_List() is invoked 3 times.

@afragen if the goal is to gain access to the nav menu blocks, then wouldn't that goal be better served at the end of the render_block_core_navigation() function. Why? To get access to all of the nav blocks after processing and just before rendering?

Else, the filter in WP_Block_List::__construct() will run each time its instantiated, whether it's a nav block or not. That adds time and memory, i.e. additional performance hits.

#14 @hellofromTonya
3 years ago

I don't think introducing a filter in the constructor of WP_Block_List is the right approach to gain access to the nav blocks. Why?

  • This class has existed since 5.0.
  • It is not exclusively for nav blocks, but rather all blocks use it.
  • Adding a filter to this constructor will increase processing time and memory, meaning the introduction of this filter will cause a performance hit.

If the goal is to gain access to a specific type of block, then filtering could be done within the specific block's published library files. But looking at other blocks, I don't see a similar filter. There are filters available in WP_Block to gain access to each block.

Kind of thinking this ticket needs to move upstream to Gutenberg. If it is a regression, regressions can be considered during RC. But likely the addition of a filter would be introduced upstream first and then backported.

@afragen Can you open an issue upstream please?

#15 follow-up: @afragen
3 years ago

The only place this class is invoked is in the navigation.php block.

Do you mean make a ticket in Gutenberg?

#16 in reply to: ↑ 15 @hellofromTonya
3 years ago

Replying to afragen:

The only place this class is invoked is in the navigation.php block.

Do you mean make a ticket in Gutenberg?

Within Core, it's also instantiated in the WP_Block::__construct() for WP_Block::$inner_blocks. And it's a public class available for plugins to use too.

Oh sorry, yes, an issue in the Gutenberg repository https://github.com/WordPress/gutenberg/issues.

#17 @hellofromTonya
3 years ago

Hey @afragen,

In the meantime, could you use the 'block_type_metadata_setting' filter which is called when register_block_core_navigation() which then invokes register_block_type_from_metadata() which fires that filter. The WP_Block_Type::$render_callback is registered to the render_block_core_navigation(). Did a little a test to see what the data looks like with that filter https://gist.github.com/hellofromtonya/3fc80da6d76146e08921a38ffaad0e7e.

Not sure if it gives you exactly what you need, but might open a possibility to discover other available filters as a stop gap.

#18 @hellofromTonya
3 years ago

  • Milestone changed from 5.9 to 5.9.1

With RC1 today, moving this to 5.9.1.

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


3 years ago

#20 @hellofromTonya
3 years ago

  • Milestone changed from 5.9.1 to 6.0

Whoops, as it's adding a new filter, i.e. enhancement, moving to 6.0.

#21 @afragen
3 years ago

@hellofromTonya I looked at your example and it's not helping me achieve what I want. What I want is to be able to manipulate the menu items. This example only allows me to manipulate the block metadata, not the actual data.

Can you give some sort of example that changes a menu item or adds one to the current homepage menu?

#23 @hellofromTonya
3 years ago

  • Milestone 6.0 deleted
  • Resolution set to reported-upstream
  • Status changed from new to closed

This feature request as been made upstream in the Gutenberg project. See Gutenberg issue https://github.com/WordPress/gutenberg/issues/37717. Closing this ticket as discussion and resolution would occur upstream and then backported to Core with an update.

This ticket was mentioned in Slack in #core by afragen. View the logs.


3 years ago

This ticket was mentioned in PR #2168 on WordPress/wordpress-develop by afragen.


3 years ago
#25

Add a filter to wp-includes/blocks/navigation.php to filter results of $inner_blocks allowing the modification of navigation block menu items.

Trac ticket: https://core.trac.wordpress.org/ticket/54684

#26 @afragen
3 years ago

  • Description modified (diff)
  • Milestone set to 6.0
  • Resolution reported-upstream deleted
  • Status changed from closed to reopened
  • Summary changed from Add ability to filter blocks in WP_Block_List to Add ability to filter navigation block items
  • Version changed from 5.5 to trunk

afragen commented on PR #2168:


3 years ago
#27

I have no idea where the proper place to make this change is.

#28 @afragen
3 years ago

  • Version changed from trunk to 5.5

I think I figured out how to properly submit a PR upstream. Please close again.

https://github.com/WordPress/gutenberg/pull/37998

#29 @afragen
3 years ago

  • Milestone changed from 6.0 to Awaiting Review

#30 @afragen
2 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

#31 @desrosj
2 years ago

  • Milestone Awaiting Review deleted

#32 @SergeyBiryukov
19 months ago

  • Resolution changed from fixed to reported-upstream
Note: See TracTickets for help on using tickets.