Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Navigation Link block: register variations on post type / taxonomy registration #54801

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 57 additions & 0 deletions packages/block-library/src/navigation-link/index.php
Original file line number Diff line number Diff line change
Expand Up @@ -322,13 +322,35 @@ function build_variation_for_navigation_link( $entity, $kind ) {
return $variation;
}

/**
* Register a variation for a post type / taxonomy for the navigation link block
*
* @param array $variation Variation array from build_variation_for_navigation_link.
* @return void
*/
function register_block_core_navigation_link_variation( $variation ) {
getdave marked this conversation as resolved.
Show resolved Hide resolved
// Directly set the variations on the registered block type
// because there's no server side registration for variations (see #47170).
$navigation_block_type = WP_Block_Type_Registry::get_instance()->get_registered( 'core/navigation-link' );
// If the block is not registered yet, bail early.
// Variation will be registered in register_block_core_navigation_link then.
if ( ! $navigation_block_type ) {
return;
}

$navigation_block_type->variations[] = $variation;
getdave marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* Register the navigation link block.
*
* @uses render_block_core_navigation()
* @throws WP_Error An WP_Error exception parsing the block definition.
*/
function register_block_core_navigation_link() {
// This will only handle post types and taxonomies registered until this point (init on priority 9).
// See action hooks below for other post types and taxonomies.
// See https://github.com/WordPress/gutenberg/issues/53826 for details.
$post_types = get_post_types( array( 'show_in_nav_menus' => true ), 'objects' );
$taxonomies = get_taxonomies( array( 'show_in_nav_menus' => true ), 'objects' );

Expand Down Expand Up @@ -369,3 +391,38 @@ function register_block_core_navigation_link() {
);
}
add_action( 'init', 'register_block_core_navigation_link' );
// Register actions for all post types and taxonomies, to add variations when they are registered.
// All post types/taxonomies registered before register_block_core_navigation_link, will be handled by that function.
add_action( 'registered_post_type', 'register_block_core_navigation_link_post_type_variation', 10, 2 );
add_action( 'registered_taxonomy', 'register_block_core_navigation_link_taxonomy_variation', 10, 3 );
Comment on lines +396 to +397
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these two lines maybe causing the breaking PHPUNit tests for the PHP package sync process for WordPress 6.5

WordPress/wordpress-develop#5922

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe after #56952 was merged, we can just use the variations_callback function and don't need to hook into the registered_post_type and registered_taxonomy actions anymore. I can test locally.

But do you have any idea as to why those hooks may cause phpunit timeouts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@getdave I've drafted up a PR which uses the now in core available get_block_type_variations filter instead of the hooks mentioned above. This should give us performance benefits, but also hopefully remove the problems with the unit tests you mentioned.

#58389

The PR is just a draft/exploration, because it will only work when that filter is available (which is core 6.5+). So it definitely has some BC pitfalls. Happy for any feedback or if you think, that this PR should just be reverted so gutenberg can successfully be merged into core.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should give us performance benefits, but also hopefully remove the problems with the unit tests you mentioned.

Lets bring that in. I'll try it manually here.

why those hooks may cause phpunit timeouts?

PHPUnit probably registers/unregisters post types hundreds of times. That could be it? Still only a guess.

The PR is just a draft/exploration, because it will only work when that filter is available (which is core 6.5+). So it definitely has some BC pitfalls.

These Gutenberg changes will be for 6.5 so it should be ok. The only issue is that the Plugin should run with the last x2 versions of WP (I think) so we'd need to gate it based on the availability of the new variations_callback API in Core. Can we do that?


/**
* Register custom post type variations for navigation link on post type registration
* Handles all post types registered after the block is registered in register_navigation_link_post_type_variations
*
* @param string $post_type The post type name passed from registered_post_type filter.
* @param WP_Post_Type $post_type_object The post type object passed from registered_post_type.
* @return void
*/
function register_block_core_navigation_link_post_type_variation( $post_type, $post_type_object ) {
getdave marked this conversation as resolved.
Show resolved Hide resolved
if ( $post_type_object->show_in_nav_menus ) {
$variation = build_variation_for_navigation_link( $post_type_object, 'post-type' );
register_block_core_navigation_link_variation( $variation );
}
}

/**
* Register a custom taxonomy variation for navigation link on taxonomy registration
* Handles all taxonomies registered after the block is registered in register_navigation_link_post_type_variations
*
* @param string $taxonomy Taxonomy slug.
* @param array|string $object_type Object type or array of object types.
* @param array $args Array of taxonomy registration arguments.
* @return void
*/
function register_block_core_navigation_link_taxonomy_variation( $taxonomy, $object_type, $args ) {
getdave marked this conversation as resolved.
Show resolved Hide resolved
if ( isset( $args['show_in_nav_menus'] ) && $args['show_in_nav_menus'] ) {
$variation = build_variation_for_navigation_link( (object) $args, 'post-type' );
register_block_core_navigation_link_variation( $variation );
}
}
89 changes: 89 additions & 0 deletions phpunit/blocks/block-navigation-link-variations-test.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
<?php
/**
* Navigation block rendering tests.
*
* @package WordPress
* @subpackage Blocks
*/

/**
* Tests for the Navigation block variations for post types.
*
* @group blocks
*/
class Block_Navigation_Link_Variations_Test extends WP_UnitTestCase {
public function set_up() {
parent::set_up();
register_post_type(
'custom_book',
array(
'labels' => array(
'item_link' => 'Custom Book',
),
'public' => true,
'show_in_rest' => true,
'show_in_nav_menus' => true,
)
);
register_taxonomy(
'book_type',
'custom_book',
array(
'labels' => array(
'item_link' => 'Book Type',
),
'show_in_nav_menus' => true,
)
);
}

public function tear_down() {
unregister_post_type( 'custom_book' );
unregister_taxonomy( 'book_type' );
parent::tear_down();
}

/**
* @covers ::register_block_core_navigation_link_post_type_variation
*/
gaambo marked this conversation as resolved.
Show resolved Hide resolved
public function test_navigation_link_variations_custom_post_type() {
$registry = WP_Block_Type_Registry::get_instance();
$nav_link_block = $registry->get_registered( 'core/navigation-link' );
$this->assertNotEmpty( $nav_link_block->variations, 'Block has no variations' );
$variation = $this->get_variation_by_name( 'custom_book', $nav_link_block->variations );
$this->assertIsArray( $variation, 'Block variation is not an array' );
$this->assertArrayHasKey( 'title', $variation, 'Block variation has no title' );
$this->assertEquals( 'Custom Book', $variation['title'], 'Variation title is different than the post type label' );
}

/**
* @covers ::register_block_core_navigation_link_taxonomy_variation
*/
public function test_navigation_link_variations_custom_taxonomy() {
$registry = WP_Block_Type_Registry::get_instance();
$nav_link_block = $registry->get_registered( 'core/navigation-link' );
$this->assertNotEmpty( $nav_link_block->variations, 'Block has no variations' );
$variation = $this->get_variation_by_name( 'book_type', $nav_link_block->variations );
$this->assertIsArray( $variation, 'Block variation is not an array' );
$this->assertArrayHasKey( 'title', $variation, 'Block variation has no title' );
$this->assertEquals( 'Book Type', $variation['title'], 'Variation title is different than the post type label' );
}

/**
* Get a variation by its name from an array of variations.
*
* @param string $variation_name The name (= slug) of the variation.
* @param array $variations An array of variations.
* @return array|null The found variation or null.
*/
private function get_variation_by_name( $variation_name, $variations ) {
$found_variation = null;
foreach ( $variations as $variation ) {
if ( $variation['name'] === $variation_name ) {
$found_variation = $variation;
}
}

return $found_variation;
}
}
Loading