From 6a109e6d1bc072469beb01e2992a2e9f3e20b7bf Mon Sep 17 00:00:00 2001 From: scruffian Date: Wed, 31 Jan 2024 16:56:07 +0000 Subject: [PATCH 01/12] Navigation: Fix performance regression --- .../block-library/src/navigation/index.php | 58 +++++++++++++------ 1 file changed, 39 insertions(+), 19 deletions(-) diff --git a/packages/block-library/src/navigation/index.php b/packages/block-library/src/navigation/index.php index bd8cfebb5bf70f..67b890f67f3b22 100644 --- a/packages/block-library/src/navigation/index.php +++ b/packages/block-library/src/navigation/index.php @@ -9,6 +9,12 @@ * Helper functions used to render the navigation block. */ class WP_Navigation_Block_Renderer { + + /** + * Used to determine whether or not a navigation has submenus. + */ + private static $has_submenus = false; + /** * Used to determine which blocks are wrapped in an
  • . * @@ -57,23 +63,25 @@ private static function is_responsive( $attributes ) { /** * Returns whether or not a navigation has a submenu. * - * @param WP_Block_List $inner_blocks The list of inner blocks. + * //@param WP_Block_List $inner_blocks The list of inner blocks. * @return bool Returns whether or not a navigation has a submenu. */ - private static function has_submenus( $inner_blocks ) { - foreach ( $inner_blocks as $inner_block ) { - $inner_block_content = $inner_block->render(); - $p = new WP_HTML_Tag_Processor( $inner_block_content ); - if ( $p->next_tag( - array( - 'name' => 'LI', - 'class_name' => 'has-child', - ) - ) ) { - return true; - } + private static function has_submenus( $inner_blocks_content_rendered ) { + if ( true === static::$has_submenus ) { + return static::$has_submenus; + } + + $p = new WP_HTML_Tag_Processor( $inner_blocks_content_rendered ); + if ( $p->next_tag( + array( + 'name' => 'LI', + 'class_name' => 'has-child', + ) + ) ) { + static::$has_submenus = true; } - return false; + + return static::$has_submenus; } /** @@ -84,7 +92,7 @@ private static function has_submenus( $inner_blocks ) { * @return bool Returns whether or not to load the view script. */ private static function is_interactive( $attributes, $inner_blocks ) { - $has_submenus = static::has_submenus( $inner_blocks ); + $has_submenus = static::$has_submenus; $is_responsive_menu = static::is_responsive( $attributes ); return ( $has_submenus && ( $attributes['openSubmenusOnClick'] || $attributes['showSubmenuIcon'] ) ) || $is_responsive_menu; } @@ -107,6 +115,12 @@ private static function does_block_need_a_list_item_wrapper( $block ) { */ private static function get_markup_for_inner_block( $inner_block ) { $inner_block_content = $inner_block->render(); + + // This sets the `has_submenus` member variable + // which is used by other functions in the class + // to determine whether or not a navigation has submenus. + static::has_submenus( $inner_block_content ); + if ( ! empty( $inner_block_content ) ) { if ( static::does_block_need_a_list_item_wrapper( $inner_block ) ) { return '
  • ' . $inner_block_content . '
    • '; @@ -124,9 +138,6 @@ private static function get_markup_for_inner_block( $inner_block ) { * @return string Returns the html for the inner blocks of the navigation block. */ private static function get_inner_blocks_html( $attributes, $inner_blocks ) { - $has_submenus = static::has_submenus( $inner_blocks ); - $is_interactive = static::is_interactive( $attributes, $inner_blocks ); - $style = static::get_styles( $attributes ); $class = static::get_classes( $attributes ); $container_attributes = get_block_wrapper_attributes( @@ -163,6 +174,8 @@ private static function get_inner_blocks_html( $attributes, $inner_blocks ) { } // Add directives to the submenu if needed. + $has_submenus = static::$has_submenus; + $is_interactive = static::is_interactive( $attributes, $inner_blocks ); if ( $has_submenus && $is_interactive ) { $tags = new WP_HTML_Tag_Processor( $inner_blocks_html ); $inner_blocks_html = block_core_navigation_add_directives_to_submenu( $tags, $attributes ); @@ -632,6 +645,13 @@ public static function render( $attributes, $content, $block ) { unset( $attributes['rgbTextColor'], $attributes['rgbBackgroundColor'] ); $inner_blocks = static::get_inner_blocks( $attributes, $block ); + + // This function calls `get_markup_for_inner_block` which also + // sets the has_submenus member variable. + // This has to be called before any other code that relies + // on the has_submenus member variable. + $wrapper_markup = static::get_wrapper_markup( $attributes, $inner_blocks ); + // Prevent navigation blocks referencing themselves from rendering. if ( block_core_navigation_block_contains_core_navigation( $inner_blocks ) ) { return ''; @@ -642,7 +662,7 @@ public static function render( $attributes, $content, $block ) { return sprintf( '', static::get_nav_wrapper_attributes( $attributes, $inner_blocks ), - static::get_wrapper_markup( $attributes, $inner_blocks ) + $wrapper_markup ); } } From 84640230668bff0a27dd6e6d8498a1e2aa9a9ecb Mon Sep 17 00:00:00 2001 From: scruffian Date: Wed, 31 Jan 2024 17:07:30 +0000 Subject: [PATCH 02/12] update docblock --- packages/block-library/src/navigation/index.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-library/src/navigation/index.php b/packages/block-library/src/navigation/index.php index 67b890f67f3b22..e5cd9b6cc54867 100644 --- a/packages/block-library/src/navigation/index.php +++ b/packages/block-library/src/navigation/index.php @@ -63,7 +63,7 @@ private static function is_responsive( $attributes ) { /** * Returns whether or not a navigation has a submenu. * - * //@param WP_Block_List $inner_blocks The list of inner blocks. + * @param string $inner_blocks_content_rendered A rendered inner block. * @return bool Returns whether or not a navigation has a submenu. */ private static function has_submenus( $inner_blocks_content_rendered ) { From 35b8916cbef32c9f29ec7601b3c70a50a25b9213 Mon Sep 17 00:00:00 2001 From: scruffian Date: Thu, 1 Feb 2024 11:07:59 +0000 Subject: [PATCH 03/12] use a different method for determining if we have submenus --- .../block-library/src/navigation/index.php | 33 ++++++++----------- 1 file changed, 13 insertions(+), 20 deletions(-) diff --git a/packages/block-library/src/navigation/index.php b/packages/block-library/src/navigation/index.php index e5cd9b6cc54867..c14163a8e0a281 100644 --- a/packages/block-library/src/navigation/index.php +++ b/packages/block-library/src/navigation/index.php @@ -63,22 +63,19 @@ private static function is_responsive( $attributes ) { /** * Returns whether or not a navigation has a submenu. * - * @param string $inner_blocks_content_rendered A rendered inner block. - * @return bool Returns whether or not a navigation has a submenu. + * @param array $inner_blocks An array of inner blocks. + * @return bool Returns whether or not a navigation has a submenu and also sets the member variable. */ - private static function has_submenus( $inner_blocks_content_rendered ) { + private static function has_submenus( $inner_blocks ) { if ( true === static::$has_submenus ) { return static::$has_submenus; } - $p = new WP_HTML_Tag_Processor( $inner_blocks_content_rendered ); - if ( $p->next_tag( - array( - 'name' => 'LI', - 'class_name' => 'has-child', - ) - ) ) { - static::$has_submenus = true; + foreach( $inner_blocks as $inner_block ) { + if ( "core/navigation-submenu" === $inner_block->name ) { + static::$has_submenus = true; + return static::$has_submenus; + } } return static::$has_submenus; @@ -116,11 +113,6 @@ private static function does_block_need_a_list_item_wrapper( $block ) { private static function get_markup_for_inner_block( $inner_block ) { $inner_block_content = $inner_block->render(); - // This sets the `has_submenus` member variable - // which is used by other functions in the class - // to determine whether or not a navigation has submenus. - static::has_submenus( $inner_block_content ); - if ( ! empty( $inner_block_content ) ) { if ( static::does_block_need_a_list_item_wrapper( $inner_block ) ) { return '
  • ' . $inner_block_content . '
    • '; @@ -646,11 +638,12 @@ public static function render( $attributes, $content, $block ) { $inner_blocks = static::get_inner_blocks( $attributes, $block ); - // This function calls `get_markup_for_inner_block` which also - // sets the has_submenus member variable. + // This sets the `has_submenus` member variable + // which is used by other functions in the class + // to determine whether or not a navigation has submenus. // This has to be called before any other code that relies // on the has_submenus member variable. - $wrapper_markup = static::get_wrapper_markup( $attributes, $inner_blocks ); + static::has_submenus( $inner_blocks ); // Prevent navigation blocks referencing themselves from rendering. if ( block_core_navigation_block_contains_core_navigation( $inner_blocks ) ) { @@ -662,7 +655,7 @@ public static function render( $attributes, $content, $block ) { return sprintf( '', static::get_nav_wrapper_attributes( $attributes, $inner_blocks ), - $wrapper_markup + static::get_wrapper_markup( $attributes, $inner_blocks ), ); } } From 4c300b43934ca75fd3188c8d9e1d6d274ed55ecd Mon Sep 17 00:00:00 2001 From: scruffian Date: Thu, 1 Feb 2024 11:09:41 +0000 Subject: [PATCH 04/12] linting --- packages/block-library/src/navigation/index.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/block-library/src/navigation/index.php b/packages/block-library/src/navigation/index.php index c14163a8e0a281..0d72014db92af8 100644 --- a/packages/block-library/src/navigation/index.php +++ b/packages/block-library/src/navigation/index.php @@ -71,8 +71,8 @@ private static function has_submenus( $inner_blocks ) { return static::$has_submenus; } - foreach( $inner_blocks as $inner_block ) { - if ( "core/navigation-submenu" === $inner_block->name ) { + foreach ( $inner_blocks as $inner_block ) { + if ( 'core/navigation-submenu' === $inner_block->name ) { static::$has_submenus = true; return static::$has_submenus; } From c6f2788483c13ebe10c0ca948ec4cd5dfba7d313 Mon Sep 17 00:00:00 2001 From: scruffian Date: Thu, 1 Feb 2024 11:13:18 +0000 Subject: [PATCH 05/12] remove unused param --- packages/block-library/src/navigation/index.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/block-library/src/navigation/index.php b/packages/block-library/src/navigation/index.php index 0d72014db92af8..3c58307ce18409 100644 --- a/packages/block-library/src/navigation/index.php +++ b/packages/block-library/src/navigation/index.php @@ -85,10 +85,9 @@ private static function has_submenus( $inner_blocks ) { * Determine whether the navigation blocks is interactive. * * @param array $attributes The block attributes. - * @param WP_Block_List $inner_blocks The list of inner blocks. * @return bool Returns whether or not to load the view script. */ - private static function is_interactive( $attributes, $inner_blocks ) { + private static function is_interactive( $attributes ) { $has_submenus = static::$has_submenus; $is_responsive_menu = static::is_responsive( $attributes ); return ( $has_submenus && ( $attributes['openSubmenusOnClick'] || $attributes['showSubmenuIcon'] ) ) || $is_responsive_menu; From d8ffd194c369379d3ddffddd1eebddc0ca1dff00 Mon Sep 17 00:00:00 2001 From: scruffian Date: Thu, 1 Feb 2024 11:18:01 +0000 Subject: [PATCH 06/12] remove the extra call to has_submenus --- packages/block-library/src/navigation/index.php | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/packages/block-library/src/navigation/index.php b/packages/block-library/src/navigation/index.php index 3c58307ce18409..b974f762a73789 100644 --- a/packages/block-library/src/navigation/index.php +++ b/packages/block-library/src/navigation/index.php @@ -85,10 +85,11 @@ private static function has_submenus( $inner_blocks ) { * Determine whether the navigation blocks is interactive. * * @param array $attributes The block attributes. + * @param WP_Block_List $inner_blocks The list of inner blocks. * @return bool Returns whether or not to load the view script. */ - private static function is_interactive( $attributes ) { - $has_submenus = static::$has_submenus; + private static function is_interactive( $attributes, $inner_blocks ) { + $has_submenus = static::has_submenus( $inner_blocks); $is_responsive_menu = static::is_responsive( $attributes ); return ( $has_submenus && ( $attributes['openSubmenusOnClick'] || $attributes['showSubmenuIcon'] ) ) || $is_responsive_menu; } @@ -165,7 +166,7 @@ private static function get_inner_blocks_html( $attributes, $inner_blocks ) { } // Add directives to the submenu if needed. - $has_submenus = static::$has_submenus; + $has_submenus = static::has_submenus( $inner_blocks ); $is_interactive = static::is_interactive( $attributes, $inner_blocks ); if ( $has_submenus && $is_interactive ) { $tags = new WP_HTML_Tag_Processor( $inner_blocks_html ); @@ -637,13 +638,6 @@ public static function render( $attributes, $content, $block ) { $inner_blocks = static::get_inner_blocks( $attributes, $block ); - // This sets the `has_submenus` member variable - // which is used by other functions in the class - // to determine whether or not a navigation has submenus. - // This has to be called before any other code that relies - // on the has_submenus member variable. - static::has_submenus( $inner_blocks ); - // Prevent navigation blocks referencing themselves from rendering. if ( block_core_navigation_block_contains_core_navigation( $inner_blocks ) ) { return ''; From fcc383caef7a28f9d54ccf00e4e5d9c3011fd0ed Mon Sep 17 00:00:00 2001 From: scruffian Date: Thu, 1 Feb 2024 11:19:47 +0000 Subject: [PATCH 07/12] update docblock --- packages/block-library/src/navigation/index.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-library/src/navigation/index.php b/packages/block-library/src/navigation/index.php index b974f762a73789..d4822f1394d8d4 100644 --- a/packages/block-library/src/navigation/index.php +++ b/packages/block-library/src/navigation/index.php @@ -63,7 +63,7 @@ private static function is_responsive( $attributes ) { /** * Returns whether or not a navigation has a submenu. * - * @param array $inner_blocks An array of inner blocks. + * @param WP_Block_List $inner_blocks The list of inner blocks. * @return bool Returns whether or not a navigation has a submenu and also sets the member variable. */ private static function has_submenus( $inner_blocks ) { From afbe2e2cfbf1182a1c92a3f57012ffdfe826f664 Mon Sep 17 00:00:00 2001 From: Ben Dwyer Date: Thu, 1 Feb 2024 11:20:53 +0000 Subject: [PATCH 08/12] Update packages/block-library/src/navigation/index.php Co-authored-by: Andrei Draganescu --- packages/block-library/src/navigation/index.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-library/src/navigation/index.php b/packages/block-library/src/navigation/index.php index d4822f1394d8d4..641f9e59e49692 100644 --- a/packages/block-library/src/navigation/index.php +++ b/packages/block-library/src/navigation/index.php @@ -89,7 +89,7 @@ private static function has_submenus( $inner_blocks ) { * @return bool Returns whether or not to load the view script. */ private static function is_interactive( $attributes, $inner_blocks ) { - $has_submenus = static::has_submenus( $inner_blocks); + $has_submenus = static::has_submenus( $inner_blocks ); $is_responsive_menu = static::is_responsive( $attributes ); return ( $has_submenus && ( $attributes['openSubmenusOnClick'] || $attributes['showSubmenuIcon'] ) ) || $is_responsive_menu; } From 248242402e2b20472adf3ece15b2af6412e016ad Mon Sep 17 00:00:00 2001 From: scruffian Date: Thu, 1 Feb 2024 11:22:18 +0000 Subject: [PATCH 09/12] remove unnecessary changes --- packages/block-library/src/navigation/index.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/block-library/src/navigation/index.php b/packages/block-library/src/navigation/index.php index d4822f1394d8d4..134b062bc4d912 100644 --- a/packages/block-library/src/navigation/index.php +++ b/packages/block-library/src/navigation/index.php @@ -130,6 +130,9 @@ private static function get_markup_for_inner_block( $inner_block ) { * @return string Returns the html for the inner blocks of the navigation block. */ private static function get_inner_blocks_html( $attributes, $inner_blocks ) { + $has_submenus = static::has_submenus( $inner_blocks ); + $is_interactive = static::is_interactive( $attributes, $inner_blocks ); + $style = static::get_styles( $attributes ); $class = static::get_classes( $attributes ); $container_attributes = get_block_wrapper_attributes( @@ -166,8 +169,6 @@ private static function get_inner_blocks_html( $attributes, $inner_blocks ) { } // Add directives to the submenu if needed. - $has_submenus = static::has_submenus( $inner_blocks ); - $is_interactive = static::is_interactive( $attributes, $inner_blocks ); if ( $has_submenus && $is_interactive ) { $tags = new WP_HTML_Tag_Processor( $inner_blocks_html ); $inner_blocks_html = block_core_navigation_add_directives_to_submenu( $tags, $attributes ); From dd00ad19f885165674563b86057c1bff3363c5a6 Mon Sep 17 00:00:00 2001 From: scruffian Date: Thu, 1 Feb 2024 11:36:53 +0000 Subject: [PATCH 10/12] remove early return --- packages/block-library/src/navigation/index.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-library/src/navigation/index.php b/packages/block-library/src/navigation/index.php index c8382f72f7d4d1..333bf78c905192 100644 --- a/packages/block-library/src/navigation/index.php +++ b/packages/block-library/src/navigation/index.php @@ -74,7 +74,7 @@ private static function has_submenus( $inner_blocks ) { foreach ( $inner_blocks as $inner_block ) { if ( 'core/navigation-submenu' === $inner_block->name ) { static::$has_submenus = true; - return static::$has_submenus; + break; } } From 1fe86a448990f3e08c72a9bbf776d07a3c62624f Mon Sep 17 00:00:00 2001 From: scruffian Date: Thu, 1 Feb 2024 11:37:58 +0000 Subject: [PATCH 11/12] remove unnecessary changes --- packages/block-library/src/navigation/index.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/block-library/src/navigation/index.php b/packages/block-library/src/navigation/index.php index 333bf78c905192..aca0ca0a18df44 100644 --- a/packages/block-library/src/navigation/index.php +++ b/packages/block-library/src/navigation/index.php @@ -112,7 +112,6 @@ private static function does_block_need_a_list_item_wrapper( $block ) { */ private static function get_markup_for_inner_block( $inner_block ) { $inner_block_content = $inner_block->render(); - if ( ! empty( $inner_block_content ) ) { if ( static::does_block_need_a_list_item_wrapper( $inner_block ) ) { return '
  • ' . $inner_block_content . '
    • '; @@ -638,7 +637,6 @@ public static function render( $attributes, $content, $block ) { unset( $attributes['rgbTextColor'], $attributes['rgbBackgroundColor'] ); $inner_blocks = static::get_inner_blocks( $attributes, $block ); - // Prevent navigation blocks referencing themselves from rendering. if ( block_core_navigation_block_contains_core_navigation( $inner_blocks ) ) { return ''; @@ -649,7 +647,7 @@ public static function render( $attributes, $content, $block ) { return sprintf( '', static::get_nav_wrapper_attributes( $attributes, $inner_blocks ), - static::get_wrapper_markup( $attributes, $inner_blocks ), + static::get_wrapper_markup( $attributes, $inner_blocks ) ); } } From 044cf4acb8a88a54a53893c9a91cdaed53fb4943 Mon Sep 17 00:00:00 2001 From: scruffian Date: Fri, 2 Feb 2024 12:46:35 +0000 Subject: [PATCH 12/12] also set has_submenus true when we have a page list with sub pages --- packages/block-library/src/navigation/index.php | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/packages/block-library/src/navigation/index.php b/packages/block-library/src/navigation/index.php index aca0ca0a18df44..0a0404e7a02dbb 100644 --- a/packages/block-library/src/navigation/index.php +++ b/packages/block-library/src/navigation/index.php @@ -72,6 +72,22 @@ private static function has_submenus( $inner_blocks ) { } foreach ( $inner_blocks as $inner_block ) { + // If this is a page list then work out if any of the pages have children. + if ( 'core/page-list' === $inner_block->name ) { + $all_pages = get_pages( + array( + 'sort_column' => 'menu_order,post_title', + 'order' => 'asc', + ) + ); + foreach ( (array) $all_pages as $page ) { + if ( $page->post_parent ) { + static::$has_submenus = true; + break; + } + } + } + // If this is a navigation submenu then we know we have submenus. if ( 'core/navigation-submenu' === $inner_block->name ) { static::$has_submenus = true; break;