Navigation: Remove duplicate css color building functions #48700
Conversation
scruffian
added
[Block] Navigation
|
Flaky tests detected in 983d9d0. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4316597094
|
| * @param array $attributes Block attributes. | ||
| * @return array Colors CSS classes and inline styles. | ||
| */ | ||
| function block_core_navigation_link_build_css_colors( $context, $attributes ) { |
There was a problem hiding this comment.
Since all functions in PHP are public, should we probably add _deprecated_function( __FUNCTION__, '6.2' ); to this one and just call the other function from it, right?
| * @param array $attributes Block attributes. | ||
| * @return array Colors CSS classes and inline styles. | ||
| */ | ||
| function block_core_navigation_submenu_build_css_colors( $context, $attributes ) { |
There was a problem hiding this comment.
Should we change the name to not reference submenu here?
There was a problem hiding this comment.
| function block_core_navigation_submenu_build_css_colors( $context, $attributes ) { | |
| function block_core_navigation_build_css_colors( $context, $attributes ) { |
| @@ -145,7 +74,7 @@ function render_block_core_navigation_submenu( $attributes, $content, $block ) { | |||
| return ''; | |||
| } | |||
|
|
|||
| $colors = block_core_navigation_submenu_build_css_colors( $block->context, $attributes ); | |||
| $colors = function_exists( 'block_core_navigation_submenu_build_css_colors' ) ? block_core_navigation_submenu_build_css_colors( $block->context, $attributes ) : array(); | |||
There was a problem hiding this comment.
| $colors = function_exists( 'block_core_navigation_submenu_build_css_colors' ) ? block_core_navigation_submenu_build_css_colors( $block->context, $attributes ) : array(); | |
| $colors = function_exists( 'block_core_navigation_submenu_build_css_colors' ) ? block_core_navigation_build_css_colors( $block->context, $attributes ) : array(); |
| @@ -174,7 +103,7 @@ function render_block_core_navigation_link( $attributes, $content, $block ) { | |||
| return ''; | |||
| } | |||
|
|
|||
| $colors = block_core_navigation_link_build_css_colors( $block->context, $attributes ); | |||
| $colors = function_exists( 'block_core_navigation_submenu_build_css_colors' ) ? block_core_navigation_submenu_build_css_colors( $block->context, $attributes ) : array(); | |||
There was a problem hiding this comment.
| $colors = function_exists( 'block_core_navigation_submenu_build_css_colors' ) ? block_core_navigation_submenu_build_css_colors( $block->context, $attributes ) : array(); | |
| $colors = function_exists( 'block_core_navigation_build_css_colors' ) ? block_core_navigation_build_css_colors( $block->context, $attributes ) : array(); |
draganescu
changed the title
|
I would not land this PR :D - I think the duplication comes from the fact that the blocks should not have to do this work, so the "central" place is not another block but it should be something else like the style engine or some style thing. This PR is good, but it also hides the problem under the rug. |
What?
These two functions are the same - let's move the code to a shared location.
Why?
If we're not careful we'll make changes to one and not the other and they will get out of sync.
How?
I have moved the functions to the navigation block PHP code, since these blocks require that block to work. I have also put a check that the function exists before we call it.
Testing Instructions