Theme JSON: migrate get_style_nodes to 6.1 #41262
Conversation
ramonjd
added
[Type] Plugin Interoperability
ramonjd
force-pushed
the
update/refactor-get-style-nodes
branch
from
fe58633
to
b31726e
Compare
|
Seems to work as advertised to me ... but while testing noted that some styles from theme.json block settings are not being applied in the frontend. Doesn't seem to be this PR as also on trunk, but ok on 6.0, so just tracking down when it was introduced in case related to #41160 changes |
There was a problem hiding this comment.
Thanks for exploring this @ramonjd!
Unfortunately, I think the approach might wind up being in conflict with the effort to update the minimum WP version to 5.9 in #41306
In that PR, the 6.0 class inherits from the core WP_Theme_JSON class which will still have the two params for get_style_nodes.
So, with the current approach of class inheritance we might wind up being stuck with the extra params, unless we can think of another way to handle it?
Good point, thanks. I think then I'll rather wait for #41306 to be merged, then see if we can resolve the conflict. If not, no biggie, #41217 took care of the original PHP warnings regardless. Thanks for testing @andrewserong and @glendaviesnz I'll place this PR on the back burner. |
ramonjd
added
the
[Status] Blocked
…uired. This commit migrates methods that call get_style_nodes() from WP_Theme_JSON_5_9 to WP_Theme_JSON_6_1, and removes the argument.
ramonjd
force-pushed
the
update/refactor-get-style-nodes
branch
from
b31726e
to
da15554
Compare
ramonjd
removed
the
[Status] Blocked
|
#41306 has been merged thereby removing the 5.9 compat file completely. I've rebased this PR. |
|
Actually, I think I might have to Warning: Declaration of WP_Theme_JSON_6_1::get_style_nodes($theme_json) should be compatible with WP_Theme_JSON::get_style_nodes($theme_json, $selectors = Array) in /var/www/html/wp-content/plugins/gutenberg/lib/compat/wordpress-6.1/class-wp-theme-json-6-1.phpSo We'll need a core patch, so possibly it's best we do it all at once during 6.1. |
ME: "Whatever, dude."
| * | ||
| * @return array The block nodes in theme.json. | ||
| */ | ||
| private static function get_block_nodes( $theme_json, $selectors = array() ) { | ||
| $selectors = empty( $selectors ) ? static::get_blocks_metadata() : $selectors; | ||
| private static function get_block_nodes( $theme_json ) { |
There was a problem hiding this comment.
Removing params like this could be considered a breaking change.
There was a problem hiding this comment.
Thanks for chiming in. This PR is pretty stale and I was going to abandon it.
After the min version of WordPress Gutenberg requires is moved to 6.1, maybe we could revisit it?
There's a PR to bump it to 6.0 at the moment.
| * @return array | ||
| */ | ||
| protected static function get_style_nodes( $theme_json, $selectors = array() ) { | ||
| protected static function get_style_nodes( $theme_json ) { |
There was a problem hiding this comment.
What?
Migrates WP_Theme_JSON methods that call
get_style_nodes()from WP_Theme_JSON_5_9 to WP_Theme_JSON_6_1.Why?
After #41160, the
$selectorsargumentget_style_nodes()is longer required. See @oandregal's comments in #41217 (comment)How?
This commit reverts #41217 and migrates WP_Theme_JSON methods that call
get_style_nodes()from WP_Theme_JSON_5_9 to WP_Theme_JSON_6_1.Testing Instructions
Run this branch, ensure the site and post editors work as well as the frontend.
npm run test-unit-php