Conversation
|
Sorry for the broken tests; please hold off on reviewing until I get a chance to fix them. |
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Flaky tests detected in 0ced6a0. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/22224319197
|
|
This should be ready for a final review now. Please take a look when you have a chance, and let me know what you think! |
|
Any chance for a final review on this one? I don't think there are any pending issues. |
There was a problem hiding this comment.
Pull request overview
Aligns Gutenberg’s WP_Duotone_Gutenberg behavior with the Core lazy-initialization approach to avoid expensive duotone metadata/theme.json processing on requests that never render duotone-supporting blocks.
Changes:
- Refactors
WP_Duotone_Gutenbergto lazily initialize global styles presets and global-style block-name mappings via internal getters. - Removes eager
wp_loadedinitialization and updates therender_blockfilter registration to pass theWP_Blockinstance. - Updates PHPUnit tests to pass the
WP_Blockinstance and to handle Reflection accessibility differences across PHP versions.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
lib/class-wp-duotone-gutenberg.php |
Implements lazy-loading getters, updates selector logic to reuse WP_Block_Type, and adjusts render flow to load metadata on-demand. |
lib/block-supports/duotone.php |
Removes eager initialization hooks and changes render_block filter registration to use 3 args. |
phpunit/class-wp-duotone-test.php |
Updates tests for the new render_duotone_support() signature and adjusts Reflection usage for newer PHP. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| $global_styles_block_names = self::get_all_global_style_block_names(); | ||
|
|
||
| // The block should have a duotone attribute or have duotone defined in its theme.json to be processed. | ||
| $has_duotone_attribute = isset( $block['attrs']['style']['color']['duotone'] ); | ||
| $has_global_styles_duotone = array_key_exists( $block['blockName'], self::$global_styles_block_names ); | ||
| $has_global_styles_duotone = array_key_exists( $block['blockName'], $global_styles_block_names ); | ||
|
|
There was a problem hiding this comment.
render_duotone_support() calls get_all_global_style_block_names() (which runs WP_Theme_JSON_Resolver_Gutenberg::get_merged_data()) before checking whether the block even needs global-styles duotone. For blocks that already have a style.color.duotone attribute, this extra theme.json merge is unnecessary and undermines the lazy-load goal. Compute $has_duotone_attribute first and only load global style block names when you actually need to evaluate/apply the global-styles fallback.
There was a problem hiding this comment.
get_all_global_style_block_names() only runs WP_Theme_JSON_Resolver_Gutenberg::get_merged_data() the first time, and then keeps a cache. This is a pretty fundamental part of this PR. get_merged_data will not be called for every block.
The proposed 'fix' would only help if every single block happened to have a style.color.duotone attribute, otherwise it would have to be calculated at least once. And given that there are plenty of blocks without duotone support, this would be a useless optimisation.
| public static function render_duotone_support( $block_content, $block, $wp_block ) { | ||
| if ( ! $block['blockName'] ) { | ||
| return $block_content; | ||
| } | ||
| $duotone_selector = self::get_selector( $wp_block->block_type ); | ||
|
|
There was a problem hiding this comment.
render_duotone_support() now requires a $wp_block argument and immediately dereferences $wp_block->block_type. This is a backward-compatibility break: the deprecated gutenberg_render_duotone_support() helper (lib/block-supports/duotone.php:429+) and any external callers still passing 2 args will fatal with “Too few arguments” / null deref. Make the third parameter optional (default null) and fall back to resolving the WP_Block_Type from $block['blockName'] when $wp_block is missing/invalid; also update the deprecated wrapper to pass the instance when available (or construct one).
There was a problem hiding this comment.
This is irrelevant, as the method is only used as a filter callback, and there are thus no backwards compatibility concerns.
| private static function is_preset( $duotone_attr ) { | ||
| $slug = self::get_slug_from_attribute( $duotone_attr ); | ||
| $filter_id = self::get_filter_id( $slug ); | ||
|
|
||
| return array_key_exists( $filter_id, self::$global_styles_presets ); | ||
| return array_key_exists( $filter_id, self::get_all_global_styles_presets() ); | ||
| } |
There was a problem hiding this comment.
is_preset() always calls get_all_global_styles_presets(), which triggers gutenberg_get_global_settings() → WP_Theme_JSON_Resolver_Gutenberg::get_merged_data() even for non-preset string values like 'unset' (where get_slug_from_attribute() returns an empty string). Add an early return when the extracted slug is empty (and/or when the attribute doesn't match the preset var formats) to avoid forcing the expensive theme.json merge for simple CSS-string duotone values.
There was a problem hiding this comment.
is_preset() always calls get_all_global_styles_presets(), which triggers gutenberg_get_global_settings() → WP_Theme_JSON_Resolver_Gutenberg::get_merged_data()
This is not true. get_all_global_styles_presets() only calls gutenberg_get_global_settings() the first time, and otherwise keeps a cache. That's kind of fundamental to understanding this PR.
If the concern is that is_preset() gets called too early for the first time, and thus triggers the expensive operations too early as well, then that is also not true, because it only gets called in render_duotone_support(), which is a callback for the render_block hook.
This comment is a shallow analysis of the code in question, and unrelated to what this PR is trying to do.
There was a problem hiding this comment.
Copilot produced a nearly completely useless review, as the bot shows a fundamental lack of 'understanding' (for lack of a better word) of the code in question.
It did point out a type issue, so I guess it's not a total failure! That doesn't really amount to much, though, given that it sent me on a bunch of wild goose chases that wasted around an hour of my time.
I will be ignoring Copilot "code reviews" going forward.
| public static function render_duotone_support( $block_content, $block, $wp_block ) { | ||
| if ( ! $block['blockName'] ) { | ||
| return $block_content; | ||
| } | ||
| $duotone_selector = self::get_selector( $wp_block->block_type ); | ||
|
|
There was a problem hiding this comment.
This is irrelevant, as the method is only used as a filter callback, and there are thus no backwards compatibility concerns.
| private static function is_preset( $duotone_attr ) { | ||
| $slug = self::get_slug_from_attribute( $duotone_attr ); | ||
| $filter_id = self::get_filter_id( $slug ); | ||
|
|
||
| return array_key_exists( $filter_id, self::$global_styles_presets ); | ||
| return array_key_exists( $filter_id, self::get_all_global_styles_presets() ); | ||
| } |
There was a problem hiding this comment.
is_preset() always calls get_all_global_styles_presets(), which triggers gutenberg_get_global_settings() → WP_Theme_JSON_Resolver_Gutenberg::get_merged_data()
This is not true. get_all_global_styles_presets() only calls gutenberg_get_global_settings() the first time, and otherwise keeps a cache. That's kind of fundamental to understanding this PR.
If the concern is that is_preset() gets called too early for the first time, and thus triggers the expensive operations too early as well, then that is also not true, because it only gets called in render_duotone_support(), which is a callback for the render_block hook.
This comment is a shallow analysis of the code in question, and unrelated to what this PR is trying to do.
| $global_styles_block_names = self::get_all_global_style_block_names(); | ||
|
|
||
| // The block should have a duotone attribute or have duotone defined in its theme.json to be processed. | ||
| $has_duotone_attribute = isset( $block['attrs']['style']['color']['duotone'] ); | ||
| $has_global_styles_duotone = array_key_exists( $block['blockName'], self::$global_styles_block_names ); | ||
| $has_global_styles_duotone = array_key_exists( $block['blockName'], $global_styles_block_names ); | ||
|
|
There was a problem hiding this comment.
get_all_global_style_block_names() only runs WP_Theme_JSON_Resolver_Gutenberg::get_merged_data() the first time, and then keeps a cache. This is a pretty fundamental part of this PR. get_merged_data will not be called for every block.
The proposed 'fix' would only help if every single block happened to have a style.color.duotone attribute, otherwise it would have to be calculated at least once. And given that there are plenty of blocks without duotone support, this would be a useless optimisation.
This comment was marked as duplicate.
This comment was marked as duplicate.
FWIW, I usually find Copilot reviews to be helpful, so I want to at least see what it has to say. But I don't feel I need to action on everything, as indeed it often can be wrong. Clearly it wasn't as effective with this PR, but in my experience the majority of the review comments are helpful. Sorry to waste your time. The Duotone functionality is not familiar to me so I was trying to help get additional "eyes" on it to help unblock you. |
|
Thank you, @westonruter, I appreciate the thought, and I apologise if I let my frustration get the better of me. Perhaps I'm unlucky, but I'm yet to have a good code review experience with an LLM 😕 |
33b8c34 to
0ced6a0
Compare
Co-authored-by: Weston Ruter <weston@ruter.net>
0ced6a0 to
8abe01a
Compare
westonruter
left a comment
There was a problem hiding this comment.
LGTM, although again, I have very little experience with the Duotone functionality.
diff -u src/wp-includes/class-wp-duotone.php src/wp-content/plugins/gutenberg/lib/class-wp-duotone-gutenberg.php
--- src/wp-includes/class-wp-duotone.php 2026-02-13 10:28:25
+++ src/wp-content/plugins/gutenberg/lib/class-wp-duotone-gutenberg.php 2026-02-23 11:05:17
@@ -1,6 +1,6 @@
<?php
/**
- * WP_Duotone class
+ * WP_Duotone_Gutenberg class
*
* Parts of this source were derived and modified from colord,
* released under the MIT license.
@@ -28,16 +28,16 @@
* OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
* WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
*
- * @package WordPress
+ * @package gutenberg
* @since 6.3.0
*/
/**
* Manages duotone block supports and global styles.
*
- * @access private
+ * @access public
*/
-class WP_Duotone {
+class WP_Duotone_Gutenberg {
/**
* Block names from global, theme, and custom styles that use duotone presets and the slug of
* the preset they are using.
@@ -48,11 +48,7 @@
* …
* ]
*
- * @internal
- *
- * @since 6.3.0
- *
- * @var ?array
+ * @var ?array<string, string>
*/
private static $global_styles_block_names;
@@ -62,7 +58,7 @@
* Example:
* [
* 'wp-duotone-blue-orange' => [
- * 'slug' => 'blue-orange',
+ * 'slug' => 'blue-orange',
* 'colors' => [ '#0000ff', '#ffcc00' ],
* ],
* 'wp-duotone-red-yellow' => [
@@ -72,11 +68,7 @@
* …
* ]
*
- * @internal
- *
- * @since 6.3.0
- *
- * @var ?array
+ * @var ?array<string, array<string, string|string[]>>
*/
private static $global_styles_presets;
@@ -93,10 +85,6 @@
* …
* ]
*
- * @internal
- *
- * @since 6.3.0
- *
* @var array
*/
private static $used_global_styles_presets = array();
@@ -118,10 +106,6 @@
* …
* ]
*
- * @internal
- *
- * @since 6.3.0
- *
* @var array
*/
private static $used_svg_filter_data = array();
@@ -140,28 +124,19 @@
* …
* ]
*
- * @internal
- *
- * @since 6.3.0
- *
* @var array
*/
private static $block_css_declarations = array();
/**
- * Clamps a value between an upper and lower bound.
+ * Direct port of colord's clamp function. Using min/max instead of
+ * nested ternaries.
*
- * Direct port of colord's clamp function.
+ * @link https://github.com/omgovich/colord/blob/3f859e03b0ca622eb15480f611371a0f15c9427f/src/helpers.ts#L23
*
- * @link https://github.com/omgovich/colord/blob/3f859e03b0ca622eb15480f611371a0f15c9427f/src/helpers.ts#L23 Sourced from colord.
- *
- * @internal
- *
- * @since 6.3.0
- *
* @param float $number The number to clamp.
- * @param float $min The minimum value.
- * @param float $max The maximum value.
+ * @param float $min The minimum value.
+ * @param float $max The maximum value.
* @return float The clamped value.
*/
private static function colord_clamp( $number, $min = 0, $max = 1 ) {
@@ -169,16 +144,10 @@
}
/**
- * Processes and clamps a degree (angle) value properly.
- *
* Direct port of colord's clampHue function.
*
- * @link https://github.com/omgovich/colord/blob/3f859e03b0ca622eb15480f611371a0f15c9427f/src/helpers.ts#L32 Sourced from colord.
+ * @link https://github.com/omgovich/colord/blob/3f859e03b0ca622eb15480f611371a0f15c9427f/src/helpers.ts#L32
*
- * @internal
- *
- * @since 6.3.0
- *
* @param float $degrees The hue to clamp.
* @return float The clamped hue.
*/
@@ -188,16 +157,10 @@
}
/**
- * Converts a hue value to degrees from 0 to 360 inclusive.
- *
* Direct port of colord's parseHue function.
*
- * @link https://github.com/omgovich/colord/blob/3f859e03b0ca622eb15480f611371a0f15c9427f/src/helpers.ts#L40 Sourced from colord.
+ * @link https://github.com/omgovich/colord/blob/3f859e03b0ca622eb15480f611371a0f15c9427f/src/helpers.ts#L40
*
- * @internal
- *
- * @since 6.3.0
- *
* @param float $value The hue value to parse.
* @param string $unit The unit of the hue value.
* @return float The parsed hue value.
@@ -209,22 +172,19 @@
'rad' => 360 / ( M_PI * 2 ),
);
- $factor = $angle_units[ $unit ] ?? 1;
+ $factor = $angle_units[ $unit ];
+ if ( ! $factor ) {
+ $factor = 1;
+ }
return (float) $value * $factor;
}
/**
- * Parses any valid Hex3, Hex4, Hex6 or Hex8 string and converts it to an RGBA object.
- *
* Direct port of colord's parseHex function.
*
- * @link https://github.com/omgovich/colord/blob/3f859e03b0ca622eb15480f611371a0f15c9427f/src/colorModels/hex.ts#L8 Sourced from colord.
+ * @link https://github.com/omgovich/colord/blob/3f859e03b0ca622eb15480f611371a0f15c9427f/src/colorModels/hex.ts#L8
*
- * @internal
- *
- * @since 6.3.0
- *
* @param string $hex The hex string to parse.
* @return array|null An array of RGBA values or null if the hex string is invalid.
*/
@@ -246,7 +206,7 @@
'r' => (int) base_convert( $hex[0] . $hex[0], 16, 10 ),
'g' => (int) base_convert( $hex[1] . $hex[1], 16, 10 ),
'b' => (int) base_convert( $hex[2] . $hex[2], 16, 10 ),
- 'a' => 4 === strlen( $hex ) ? round( (int) base_convert( $hex[3] . $hex[3], 16, 10 ) / 255, 2 ) : 1,
+ 'a' => 4 === strlen( $hex ) ? round( base_convert( $hex[3] . $hex[3], 16, 10 ) / 255, 2 ) : 1,
);
}
@@ -263,16 +223,10 @@
}
/**
- * Clamps an array of RGBA values.
- *
* Direct port of colord's clampRgba function.
*
- * @link https://github.com/omgovich/colord/blob/3f859e03b0ca622eb15480f611371a0f15c9427f/src/colorModels/rgb.ts#L5 Sourced from colord.
+ * @link https://github.com/omgovich/colord/blob/3f859e03b0ca622eb15480f611371a0f15c9427f/src/colorModels/rgb.ts#L5
*
- * @internal
- *
- * @since 6.3.0
- *
* @param array $rgba The RGBA array to clamp.
* @return array The clamped RGBA array.
*/
@@ -286,16 +240,10 @@
}
/**
- * Parses a valid RGB[A] CSS color function/string.
- *
* Direct port of colord's parseRgbaString function.
*
- * @link https://github.com/omgovich/colord/blob/3f859e03b0ca622eb15480f611371a0f15c9427f/src/colorModels/rgbString.ts#L18 Sourced from colord.
+ * @link https://github.com/omgovich/colord/blob/3f859e03b0ca622eb15480f611371a0f15c9427f/src/colorModels/rgbString.ts#L18
*
- * @internal
- *
- * @since 6.3.0
- *
* @param string $input The RGBA string to parse.
* @return array|null An array of RGBA values or null if the RGB string is invalid.
*/
@@ -320,10 +268,8 @@
return null;
}
- /*
- * For some reason, preg_match doesn't include empty matches at the end
- * of the array, so we add them manually to make things easier later.
- */
+ // For some reason, preg_match doesn't include empty matches at the end
+ // of the array, so we add them manually to make things easier later.
for ( $i = 1; $i <= 8; $i++ ) {
if ( ! isset( $match[ $i ] ) ) {
$match[ $i ] = '';
@@ -345,16 +291,10 @@
}
/**
- * Clamps an array of HSLA values.
- *
* Direct port of colord's clampHsla function.
*
- * @link https://github.com/omgovich/colord/blob/3f859e03b0ca622eb15480f611371a0f15c9427f/src/colorModels/hsl.ts#L6 Sourced from colord.
+ * @link https://github.com/omgovich/colord/blob/3f859e03b0ca622eb15480f611371a0f15c9427f/src/colorModels/hsl.ts#L6
*
- * @internal
- *
- * @since 6.3.0
- *
* @param array $hsla The HSLA array to clamp.
* @return array The clamped HSLA array.
*/
@@ -368,16 +308,10 @@
}
/**
- * Converts an HSVA array to RGBA.
- *
* Direct port of colord's hsvaToRgba function.
*
- * @link https://github.com/omgovich/colord/blob/3f859e03b0ca622eb15480f611371a0f15c9427f/src/colorModels/hsv.ts#L52 Sourced from colord.
+ * @link https://github.com/omgovich/colord/blob/3f859e03b0ca622eb15480f611371a0f15c9427f/src/colorModels/hsv.ts#L52
*
- * @internal
- *
- * @since 6.3.0
- *
* @param array $hsva The HSVA array to convert.
* @return array The RGBA array.
*/
@@ -402,16 +336,10 @@
}
/**
- * Converts an HSLA array to HSVA.
- *
* Direct port of colord's hslaToHsva function.
*
- * @link https://github.com/omgovich/colord/blob/3f859e03b0ca622eb15480f611371a0f15c9427f/src/colorModels/hsl.ts#L33 Sourced from colord.
+ * @link https://github.com/omgovich/colord/blob/3f859e03b0ca622eb15480f611371a0f15c9427f/src/colorModels/hsl.ts#L33
*
- * @internal
- *
- * @since 6.3.0
- *
* @param array $hsla The HSLA array to convert.
* @return array The HSVA array.
*/
@@ -432,16 +360,10 @@
}
/**
- * Converts an HSLA array to RGBA.
- *
* Direct port of colord's hslaToRgba function.
*
- * @link https://github.com/omgovich/colord/blob/3f859e03b0ca622eb15480f611371a0f15c9427f/src/colorModels/hsl.ts#L55 Sourced from colord.
+ * @link https://github.com/omgovich/colord/blob/3f859e03b0ca622eb15480f611371a0f15c9427f/src/colorModels/hsl.ts#L55
*
- * @internal
- *
- * @since 6.3.0
- *
* @param array $hsla The HSLA array to convert.
* @return array The RGBA array.
*/
@@ -450,16 +372,10 @@
}
/**
- * Parses a valid HSL[A] CSS color function/string.
- *
* Direct port of colord's parseHslaString function.
*
- * @link https://github.com/omgovich/colord/blob/3f859e03b0ca622eb15480f611371a0f15c9427f/src/colorModels/hslString.ts#L17 Sourced from colord.
- *
- * @internal
+ * @link https://github.com/omgovich/colord/blob/3f859e03b0ca622eb15480f611371a0f15c9427f/src/colorModels/hslString.ts#L17
*
- * @since 6.3.0
- *
* @param string $input The HSLA string to parse.
* @return array|null An array of RGBA values or null if the RGB string is invalid.
*/
@@ -484,10 +400,8 @@
return null;
}
- /*
- * For some reason, preg_match doesn't include empty matches at the end
- * of the array, so we add them manually to make things easier later.
- */
+ // For some reason, preg_match doesn't include empty matches at the end
+ // of the array, so we add them manually to make things easier later.
for ( $i = 1; $i <= 6; $i++ ) {
if ( ! isset( $match[ $i ] ) ) {
$match[ $i ] = '';
@@ -507,17 +421,11 @@
}
/**
- * Tries to convert an incoming string into RGBA values.
- *
* Direct port of colord's parse function simplified for our use case. This
* version only supports string parsing and only returns RGBA values.
*
- * @link https://github.com/omgovich/colord/blob/3f859e03b0ca622eb15480f611371a0f15c9427f/src/parse.ts#L37 Sourced from colord.
+ * @link https://github.com/omgovich/colord/blob/3f859e03b0ca622eb15480f611371a0f15c9427f/src/parse.ts#L37
*
- * @internal
- *
- * @since 6.3.0
- *
* @param string $input The string to parse.
* @return array|null An array of RGBA values or null if the string is invalid.
*/
@@ -536,16 +444,10 @@
}
/**
- * Takes the inline CSS duotone variable from a block and return the slug.
- *
- * Handles styles slugs like:
+ * Take the inline CSS duotone variable from a block and return the slug. Handles styles slugs like:
* var:preset|duotone|blue-orange
* var(--wp--preset--duotone--blue-orange)
*
- * @internal
- *
- * @since 6.3.0
- *
* @param string|string[] $duotone_attr The duotone attribute from a block.
* @return string The slug of the duotone preset or an empty string if no slug is found (including when an array was passed).
*/
@@ -561,16 +463,10 @@
}
/**
- * Checks if we have a valid duotone preset.
+ * Check if we have a valid duotone preset.
*
- * Valid presets are defined in the $global_styles_presets array.
- *
- * @internal
- *
- * @since 6.3.0
- *
- * @param string $duotone_attr The duotone attribute from a block.
* @param string|string[] $duotone_attr The duotone attribute from a block.
+ * @return bool True if the duotone preset present and valid.
*/
private static function is_preset( $duotone_attr ) {
if ( ! is_string( $duotone_attr ) ) {
@@ -584,15 +480,8 @@
}
/**
- * Gets the CSS variable name for a duotone preset.
+ * Get the CSS variable name for a duotone preset.
*
- * Example output:
- * --wp--preset--duotone--blue-orange
- *
- * @internal
- *
- * @since 6.3.0
- *
* @param string $slug The slug of the duotone preset.
* @return string The CSS variable name.
*/
@@ -603,13 +492,6 @@
/**
* Get the ID of the duotone filter.
*
- * Example output:
- * wp-duotone-blue-orange
- *
- * @internal
- *
- * @since 6.3.0
- *
* @param string $slug The slug of the duotone preset.
* @return string The ID of the duotone filter.
*/
@@ -620,13 +502,6 @@
/**
* Get the CSS variable for a duotone preset.
*
- * Example output:
- * var(--wp--preset--duotone--blue-orange)
- *
- * @internal
- *
- * @since 6.3.0
- *
* @param string $slug The slug of the duotone preset.
* @return string The CSS variable.
*/
@@ -638,13 +513,6 @@
/**
* Get the URL for a duotone filter.
*
- * Example output:
- * url(#wp-duotone-blue-orange)
- *
- * @internal
- *
- * @since 6.3.0
- *
* @param string $filter_id The ID of the filter.
* @return string The URL for the duotone filter.
*/
@@ -654,13 +522,7 @@
/**
* Gets the SVG for the duotone filter definition.
- *
- * Whitespace is removed when SCRIPT_DEBUG is not enabled.
- *
- * @internal
*
- * @since 6.3.0
- *
* @param string $filter_id The ID of the filter.
* @param array $colors An array of color strings.
* @return string An SVG with a duotone filter definition.
@@ -678,11 +540,9 @@
if ( null === $color ) {
$error_message = sprintf(
- /* translators: 1: Duotone colors, 2: theme.json, 3: settings.color.duotone */
- __( '"%1$s" in %2$s %3$s is not a hex or rgb string.' ),
- $color_str,
- 'theme.json',
- 'settings.color.duotone'
+ /* translators: %s: duotone colors */
+ __( '"%s" in theme.json settings.color.duotone is not a hex or rgb string.', 'gutenberg' ),
+ $color_str
);
_doing_it_wrong( __METHOD__, $error_message, '6.3.0' );
} else {
@@ -744,58 +604,11 @@
}
/**
- * Returns the prefixed id for the duotone filter for use as a CSS id.
- *
- * Exported for the deprecated function wp_get_duotone_filter_id().
- *
- * @internal
- *
- * @since 6.3.0
- * @deprecated 6.3.0
- *
- * @param array $preset Duotone preset value as seen in theme.json.
- * @return string Duotone filter CSS id.
- */
- public static function get_filter_id_from_preset( $preset ) {
- _deprecated_function( __FUNCTION__, '6.3.0' );
-
- $filter_id = '';
- if ( isset( $preset['slug'] ) ) {
- $filter_id = self::get_filter_id( $preset['slug'] );
- }
- return $filter_id;
- }
-
- /**
- * Gets the SVG for the duotone filter definition from a preset.
- *
- * Exported for the deprecated function wp_get_duotone_filter_property().
- *
- * @internal
- *
- * @since 6.3.0
- * @deprecated 6.3.0
- *
- * @param array $preset The duotone preset.
- * @return string The SVG for the filter definition.
- */
- public static function get_filter_svg_from_preset( $preset ) {
- _deprecated_function( __FUNCTION__, '6.3.0' );
-
- $filter_id = self::get_filter_id_from_preset( $preset );
- return self::get_filter_svg( $filter_id, $preset['colors'] );
- }
-
- /**
* Get the SVGs for the duotone filters.
*
* Example output:
* <svg><defs><filter id="wp-duotone-blue-orange">…</filter></defs></svg><svg>…</svg>
*
- * @internal
- *
- * @since 6.3.0
- *
* @param array $sources The duotone presets.
* @return string The SVGs for the duotone filters.
*/
@@ -814,16 +627,11 @@
* Example output:
* body{--wp--preset--duotone--blue-orange:url('#wp-duotone-blue-orange');}
*
- * @internal
- *
- * @since 6.3.0
- * @since 6.6.0 Replaced body selector with `WP_Theme_JSON::ROOT_CSS_PROPERTIES_SELECTOR`.
- *
* @param array $sources The duotone presets.
* @return string The CSS for global styles.
*/
private static function get_global_styles_presets( $sources ) {
- $css = WP_Theme_JSON::ROOT_CSS_PROPERTIES_SELECTOR . '{';
+ $css = WP_Theme_JSON_Gutenberg::ROOT_CSS_PROPERTIES_SELECTOR . '{';
foreach ( $sources as $filter_id => $filter_data ) {
$slug = $filter_data['slug'];
$colors = $filter_data['colors'];
@@ -836,14 +644,48 @@
}
/**
- * Enqueue a block CSS declaration for the page.
+ * Get the CSS selector for a block type.
+ *
+ * @param WP_Block_Type $block_type Block type to check for support.
*
- * This does not include any SVGs.
+ * @return string|null The CSS selector. Null if there is no support or the $block_type is not a WP_Block_Type.
+ */
+ private static function get_selector( $block_type ) {
+ if ( ! ( $block_type instanceof WP_Block_Type ) ) {
+ return null;
+ }
+
+ /*
+ * Backward compatibility with `supports.color.__experimentalDuotone`
+ * is provided via the `block_type_metadata_settings` filter. If
+ * `supports.filter.duotone` has not been set and the experimental
+ * property has been, the experimental property value is copied into
+ * `supports.filter.duotone`.
+ */
+ $duotone_support = block_has_support( $block_type, array( 'filter', 'duotone' ) );
+ if ( ! $duotone_support ) {
+ return null;
+ }
+
+ /*
+ * If the experimental duotone support was set, that value is to be
+ * treated as a selector and requires scoping.
+ */
+ $experimental_duotone = $block_type->supports['color']['__experimentalDuotone'] ?? false;
+ if ( $experimental_duotone ) {
+ $root_selector = wp_get_block_css_selector( $block_type );
+ return is_string( $experimental_duotone )
+ ? WP_Theme_JSON::scope_selector( $root_selector, $experimental_duotone )
+ : $root_selector;
+ }
+
+ // Regular filter.duotone support uses filter.duotone selectors with fallbacks.
+ return wp_get_block_css_selector( $block_type, array( 'filter', 'duotone' ), true );
+ }
+
+ /**
+ * Enqueue a block CSS declaration for the page.
*
- * @internal
- *
- * @since 6.3.0
- *
* @param string $filter_id The filter ID. e.g. 'wp-duotone-000000-ffffff-2'.
* @param string $duotone_selector The block's duotone selector. e.g. '.wp-block-image img'.
* @param string $filter_value The filter CSS value. e.g. 'url(#wp-duotone-000000-ffffff-2)' or 'unset'.
@@ -854,11 +696,9 @@
$selectors_scoped = array();
foreach ( $selectors as $selector_part ) {
- /*
- * Assuming the selector part is a subclass selector (not a tag name)
- * so we can prepend the filter id class. If we want to support elements
- * such as `img` or namespaces, we'll need to add a case for that here.
- */
+ // Assuming the selector part is a subclass selector (not a tag name)
+ // so we can prepend the filter id class. If we want to support elements
+ // such as `img` or namespaces, we'll need to add a case for that here.
$selectors_scoped[] = '.' . $filter_id . trim( $selector_part );
}
@@ -873,14 +713,8 @@
}
/**
- * Enqueue custom filter assets for the page.
+ * Enqueue custom filter assets for the page. Includes an SVG filter and block CSS declaration.
*
- * Includes an SVG filter and block CSS declaration.
- *
- * @internal
- *
- * @since 6.3.0
- *
* @param string $filter_id The filter ID. e.g. 'wp-duotone-000000-ffffff-2'.
* @param string $duotone_selector The block's duotone selector. e.g. '.wp-block-image img'.
* @param string $filter_value The filter CSS value. e.g. 'url(#wp-duotone-000000-ffffff-2)' or 'unset'.
@@ -892,14 +726,8 @@
}
/**
- * Enqueue preset assets for the page.
+ * Enqueue preset assets for the page. Includes a CSS custom property, SVG filter, and block CSS declaration.
*
- * Includes a CSS custom property, SVG filter, and block CSS declaration.
- *
- * @internal
- *
- * @since 6.3.0
- *
* @param string $filter_id The filter ID. e.g. 'wp-duotone-blue-orange'.
* @param string $duotone_selector The block's duotone selector. e.g. '.wp-block-image img'.
* @param string $filter_value The filter CSS value. e.g. 'url(#wp-duotone-blue-orange)' or 'unset'.
@@ -908,10 +736,9 @@
$global_styles_presets = self::get_all_global_styles_presets();
if ( ! array_key_exists( $filter_id, $global_styles_presets ) ) {
$error_message = sprintf(
- /* translators: 1: Duotone filter ID, 2: theme.json */
- __( 'The duotone id "%1$s" is not registered in %2$s settings' ),
- $filter_id,
- 'theme.json'
+ /* translators: %s: duotone filter ID */
+ __( 'The duotone id "%s" is not registered in theme.json settings', 'gutenberg' ),
+ $filter_id
);
_doing_it_wrong( __METHOD__, $error_message, '6.3.0' );
return;
@@ -923,17 +750,13 @@
/**
* Registers the style and colors block attributes for block types that support it.
*
- * Block support is added with `supports.filter.duotone` in block.json.
- *
* @since 6.3.0
*
* @param WP_Block_Type $block_type Block Type.
*/
public static function register_duotone_support( $block_type ) {
- /*
- * Previous `color.__experimentalDuotone` support flag is migrated
- * to `filter.duotone` via `block_type_metadata_settings` filter.
- */
+ // Previous `color.__experimentalDuotone` support flag is migrated
+ // to `filter.duotone` via `block_type_metadata_settings` filter.
if ( block_has_support( $block_type, array( 'filter', 'duotone' ), null ) ) {
if ( ! $block_type->attributes ) {
$block_type->attributes = array();
@@ -948,67 +771,20 @@
}
/**
- * Get the CSS selector for a block type.
+ * Get all possible duotone presets from global and theme styles and store as slug => [ colors array ]
+ * We only want to process this one time. On block render we'll access and output only the needed presets for that page.
*
- * This handles selectors defined in `color.__experimentalDuotone` support
- * if `filter.duotone` support is not defined.
- *
- * @internal
* @since 6.3.0
*
- * @param WP_Block_Type $block_type Block type to check for support.
- * @return string|null The CSS selector or null if there is no support.
+ * @return array<string, array<string, string|string[]>> An array of global styles presets, keyed on the filter ID.
*/
- private static function get_selector( $block_type ) {
- if ( ! ( $block_type instanceof WP_Block_Type ) ) {
- return null;
- }
-
- /*
- * Backward compatibility with `supports.color.__experimentalDuotone`
- * is provided via the `block_type_metadata_settings` filter. If
- * `supports.filter.duotone` has not been set and the experimental
- * property has been, the experimental property value is copied into
- * `supports.filter.duotone`.
- */
- $duotone_support = block_has_support( $block_type, array( 'filter', 'duotone' ) );
- if ( ! $duotone_support ) {
- return null;
- }
-
- /*
- * If the experimental duotone support was set, that value is to be
- * treated as a selector and requires scoping.
- */
- $experimental_duotone = $block_type->supports['color']['__experimentalDuotone'] ?? false;
- if ( $experimental_duotone ) {
- $root_selector = wp_get_block_css_selector( $block_type );
- return is_string( $experimental_duotone )
- ? WP_Theme_JSON::scope_selector( $root_selector, $experimental_duotone )
- : $root_selector;
- }
-
- // Regular filter.duotone support uses filter.duotone selectors with fallbacks.
- return wp_get_block_css_selector( $block_type, array( 'filter', 'duotone' ), true );
- }
-
- /**
- * Scrape all possible duotone presets from global and theme styles and
- * store them in self::$global_styles_presets.
- *
- * Used in conjunction with self::render_duotone_support for blocks that
- * use duotone preset filters.
- *
- * @since 6.3.0
- *
- * @return array An array of global styles presets, keyed on the filter ID.
- */
private static function get_all_global_styles_presets() {
if ( isset( self::$global_styles_presets ) ) {
return self::$global_styles_presets;
}
+
// Get the per block settings from the theme.json.
- $tree = wp_get_global_settings();
+ $tree = gutenberg_get_global_settings();
$presets_by_origin = $tree['color']['duotone'] ?? array();
self::$global_styles_presets = array();
@@ -1024,21 +800,32 @@
}
/**
- * Scrape all block names from global styles and store in self::$global_styles_block_names.
+ * Ensure all possible duotone presets are ready.
+ * This function is deprecated, as it's no longer necessary to handle this manually.
+ * It is kept to ensure external users of this method don't break.
*
- * Used in conjunction with self::render_duotone_support to output the
- * duotone filters defined in the theme.json global styles.
+ * @since 6.3.0
+ * @deprecated
+ */
+ public static function set_global_styles_presets() {
+ _deprecated_function( __METHOD__, 'Gutenberg 22.5.0' );
+ self::get_all_global_styles_presets();
+ }
+
+ /**
+ * Scrape all block names from global styles and store in self::$global_styles_block_names
*
* @since 6.3.0
*
- * @return string[] An array of global style block slugs, keyed on the block name.
+ * @return null|array<string, string> An array of global style block slugs, keyed on the block name.
*/
private static function get_all_global_style_block_names() {
if ( isset( self::$global_styles_block_names ) ) {
return self::$global_styles_block_names;
}
+
// Get the per block settings from the theme.json.
- $tree = WP_Theme_JSON_Resolver::get_merged_data();
+ $tree = WP_Theme_JSON_Resolver_Gutenberg::get_merged_data();
$block_nodes = $tree->get_styles_block_nodes();
$theme_json = $tree->get_raw_data();
@@ -1069,15 +856,26 @@
self::$global_styles_block_names[ $block_node['name'] ] = $slug;
}
}
+
return self::$global_styles_block_names;
}
/**
+ * Ensure all possible global style block names are ready.
+ * This function is deprecated, as it's no longer necessary to handle this manually.
+ * It is kept to ensure external users of this method don't break.
+ *
+ * @since 6.3.0
+ * @deprecated
+ */
+ public static function set_global_style_block_names() {
+ _deprecated_function( __METHOD__, 'Gutenberg 22.5.0' );
+ self::get_all_global_style_block_names();
+ }
+
+ /**
* Render out the duotone CSS styles and SVG.
*
- * The hooks self::set_global_style_block_names and self::set_global_styles_presets
- * must be called before this function.
- *
* @since 6.3.0
*
* @param string $block_content Rendered block content.
@@ -1108,12 +906,10 @@
// Generate the pieces needed for rendering a duotone to the page.
if ( $has_duotone_attribute ) {
- /*
- * Possible values for duotone attribute:
- * 1. Array of colors - e.g. array('#000000', '#ffffff').
- * 2. Variable for an existing Duotone preset - e.g. 'var:preset|duotone|blue-orange' or 'var(--wp--preset--duotone--blue-orange)''
- * 3. A CSS string - e.g. 'unset' to remove globally applied duotone.
- */
+ // Possible values for duotone attribute:
+ // 1. Array of colors - e.g. array('#000000', '#ffffff').
+ // 2. Variable for an existing Duotone preset - e.g. 'var:preset|duotone|blue-orange' or 'var(--wp--preset--duotone--blue-orange)''
+ // 3. A CSS string - e.g. 'unset' to remove globally applied duotone.
$duotone_attr = $block['attrs']['style']['color']['duotone'];
$is_preset = is_string( $duotone_attr ) && self::is_preset( $duotone_attr );
@@ -1121,7 +917,6 @@
$is_custom = is_array( $duotone_attr );
if ( $is_preset ) {
-
$slug = self::get_slug_from_attribute( $duotone_attr ); // e.g. 'blue-orange'.
$filter_id = self::get_filter_id( $slug ); // e.g. 'wp-duotone-filter-blue-orange'.
$filter_value = self::get_css_var( $slug ); // e.g. 'var(--wp--preset--duotone--blue-orange)'.
@@ -1159,9 +954,10 @@
// Like the layout hook, this assumes the hook only applies to blocks with a single wrapper.
$tags = new WP_HTML_Tag_Processor( $block_content );
- if ( $tags->next_tag() ) {
+ if ( $tags->next_tag() && isset( $filter_id ) ) {
$tags->add_class( $filter_id );
}
+
return $tags->get_updated_html();
}
@@ -1169,10 +965,10 @@
* Fixes the issue with our generated class name not being added to the block's outer container
* in classic themes due to gutenberg_restore_image_outer_container from layout block supports.
*
- * @since 6.6.0
+ * @since 6.5.0
*
- * @param string $block_content Rendered block content.
- * @return string Filtered block content.
+ * @param string $block_content Rendered block content.
+ * @return string Filtered block content.
*/
public static function restore_image_outer_container( $block_content ) {
if ( wp_theme_has_theme_json() ) {
@@ -1207,13 +1003,11 @@
/**
* Appends the used block duotone filter declarations to the inline block supports CSS.
*
- * Uses the declarations saved in earlier calls to self::enqueue_block_css.
- *
* @since 6.3.0
*/
public static function output_block_styles() {
if ( ! empty( self::$block_css_declarations ) ) {
- wp_style_engine_get_stylesheet_from_css_rules(
+ gutenberg_style_engine_get_stylesheet_from_css_rules(
self::$block_css_declarations,
array(
'context' => 'block-supports',
@@ -1225,8 +1019,6 @@
/**
* Appends the used global style duotone filter presets (CSS custom
* properties) to the inline global styles CSS.
- *
- * Uses the declarations saved in earlier calls to self::enqueue_global_styles_preset.
*
* @since 6.3.0
*/
@@ -1239,9 +1031,6 @@
/**
* Outputs all necessary SVG for duotone filters, CSS for classic themes.
*
- * Uses the declarations saved in earlier calls to self::enqueue_global_styles_preset
- * and self::enqueue_custom_filter.
- *
* @since 6.3.0
*/
public static function output_footer_assets() {
@@ -1257,18 +1046,17 @@
wp_add_inline_style( $style_tag_id, self::get_global_styles_presets( self::$used_global_styles_presets ) );
}
if ( ! empty( self::$block_css_declarations ) ) {
- wp_add_inline_style( $style_tag_id, wp_style_engine_get_stylesheet_from_css_rules( self::$block_css_declarations ) );
+ wp_add_inline_style( $style_tag_id, gutenberg_style_engine_get_stylesheet_from_css_rules( self::$block_css_declarations ) );
}
wp_enqueue_style( $style_tag_id );
}
}
/**
- * Adds the duotone SVGs and CSS custom properties to the editor settings.
+ * Adds the duotone SVGs and CSS custom properties to the editor settings so
+ * they can be pulled in by the EditorStyles component in JS and rendered in
+ * the post editor.
*
- * This allows the properties to be pulled in by the EditorStyles component
- * in JS and rendered in the post editor.
- *
* @since 6.3.0
*
* @param array $settings The block editor settings from the `block_editor_settings_all` filter.
@@ -1286,7 +1074,7 @@
'assets' => self::get_svg_definitions( $global_styles_presets ),
// The 'svgs' type is new in 6.3 and requires the corresponding JS changes in the EditorStyles component to work.
'__unstableType' => 'svgs',
- // These styles not generated by global styles, so this must be false or they will be stripped out in wp_get_block_editor_settings.
+ // These styles not generated by global styles, so this must be false or they will be stripped out in gutenberg_get_block_editor_settings.
'isGlobalStyles' => false,
);
@@ -1295,7 +1083,7 @@
'css' => self::get_global_styles_presets( $global_styles_presets ),
// This must be set and must be something other than 'theme' or they will be stripped out in the post editor <Editor> component.
'__unstableType' => 'presets',
- // These styles are no longer generated by global styles, so this must be false or they will be stripped out in wp_get_block_editor_settings.
+ // These styles are no longer generated by global styles, so this must be false or they will be stripped out in gutenberg_get_block_editor_settings.
'isGlobalStyles' => false,
);
}
@@ -1304,14 +1092,14 @@
}
/**
- * Migrates the experimental duotone support flag to the stabilized location.
+ * Migrate the old experimental duotone support flag to its stabilized location
+ * under `supports.filter.duotone` and sets.
*
- * This moves `supports.color.__experimentalDuotone` to `supports.filter.duotone`.
- *
* @since 6.3.0
*
* @param array $settings Current block type settings.
* @param array $metadata Block metadata as read in via block.json.
+ *
* @return array Filtered block type settings.
*/
public static function migrate_experimental_duotone_support_flag( $settings, $metadata ) {
@@ -1325,16 +1113,53 @@
}
/**
- * Gets the CSS filter property value from a preset.
+ * Returns the prefixed id for the duotone filter for use as a CSS id.
*
- * Exported for the deprecated function wp_get_duotone_filter_id().
+ * Exported for the deprecated function gutenberg_get_duotone_filter_id().
*
- * @internal
+ * @since 6.3.0
+ * @deprecated 6.3.0
*
+ * @param array $preset Duotone preset value as seen in theme.json.
+ * @return string Duotone filter CSS id.
+ */
+ public static function get_filter_id_from_preset( $preset ) {
+ _deprecated_function( __FUNCTION__, '6.3.0' );
+
+ $filter_id = '';
+ if ( isset( $preset['slug'] ) ) {
+ $filter_id = self::get_filter_id( $preset['slug'] );
+ }
+ return $filter_id;
+ }
+
+ /**
+ * Gets the SVG for the duotone filter definition from a preset.
+ *
+ * Exported for the deprecated function gutenberg_get_duotone_filter_property().
+ *
* @since 6.3.0
* @deprecated 6.3.0
*
* @param array $preset The duotone preset.
+ * @return string The SVG for the filter definition.
+ */
+ public static function get_filter_svg_from_preset( $preset ) {
+ _deprecated_function( __FUNCTION__, '6.3.0' );
+
+ $filter_id = self::get_filter_id_from_preset( $preset );
+ return self::get_filter_svg( $filter_id, $preset['colors'] );
+ }
+
+ /**
+ * Gets the CSS filter property value from a preset.
+ *
+ * Exported for the deprecated function gutenberg_get_duotone_filter_id().
+ *
+ * @since 6.3.0
+ * @deprecated 6.3.0
+ *
+ * @param array $preset The duotone preset.
* @return string The CSS filter property value.
*/
public static function get_filter_css_property_value_from_preset( $preset ) {
@@ -1345,7 +1170,6 @@
}
$filter_id = self::get_filter_id_from_preset( $preset );
-
return 'url(#' . $filter_id . ')';
}
}Gemini analysis of WP_Duotone versus WP_Duotone_Gutenberg
Prompt: Please compare src/wp-includes/class-wp-duotone.php with src/wp-content/plugins/gutenbeg/lib/class-wp-duotone-gutenberg.php where the latter is being updated to match the former.
The comparison between wp-includes/class-wp-duotone.php (Core) and wp-content/plugins/gutenberg/lib/class-wp-duotone-gutenberg.php (Gutenberg) reveals several differences, primarily related to class naming, dependencies, and additional compatibility methods in Gutenberg.
1. Structural and Naming Differences
- Class Name: Core uses
WP_Duotone, while Gutenberg usesWP_Duotone_Gutenberg. - Package: Core is tagged
@package WordPress, Gutenberg is@package gutenberg. - Access Level: Core's class docblock specifies
@access private, whereas Gutenberg's specifies@access public. - Internal Tagging: Core uses the
@internaltag on nearly all private and supporting methods; Gutenberg does not.
2. Dependency Differences
Gutenberg uses "Gutenberg-prefixed" versions of Core classes and functions to maintain compatibility or provide experimental enhancements:
WP_Theme_JSONvs.WP_Theme_JSON_GutenbergWP_Theme_JSON_Resolvervs.WP_Theme_JSON_Resolver_Gutenbergwp_get_global_settings()vs.gutenberg_get_global_settings()wp_style_engine_get_stylesheet_from_css_rules()vs.gutenberg_style_engine_get_stylesheet_from_css_rules()
3. Functional Differences
- Extra Deprecated Methods: Gutenberg contains
set_global_styles_presets()andset_global_style_block_names(). These are missing in Core and marked as deprecated in Gutenberg (since "Gutenberg 22.5.0"). - Logic in
render_duotone_support: Gutenberg includes an additionalisset( $filter_id )check:- Core:
if ( $tags->next_tag() ) { $tags->add_class( $filter_id ); } - Gutenberg:
if ( $tags->next_tag() && isset( $filter_id ) ) { $tags->add_class( $filter_id ); }
- Core:
@sinceTags:restore_image_outer_containeris tagged@since 6.6.0in Core but@since 6.5.0in Gutenberg.
4. Documentation and Types
- PHPDoc Types: Gutenberg uses more modern, specific PHPDoc types (e.g.,
?array<string, string>vs. Core's?array). - Translators Comments: Core uses more detailed, numbered translator comments in
get_filter_svg, whereas Gutenberg uses a simpler placeholder format and thegutenbergtext domain. - Hue Parsing: In
colord_parse_hue, Core uses the null coalescing operator ($factor = $angle_units[ $unit ] ?? 1;), whereas Gutenberg uses an explicitif ( ! $factor )check.
5. Method Order
- Core places
get_filter_id_from_presetandget_filter_svg_from_presetin the middle of the class. - Gutenberg groups these deprecated methods at the very end of the file.
Prompt: Does anything look amiss? Anything not updated in the Gutenberg implementation to functionally match what is in the core implementation?
Yes, there are several subtle but noteworthy differences where the Gutenberg implementation (WP_Duotone_Gutenberg) is either behind the Core implementation (WP_Duotone) or inconsistent with it.
1. Robustness Issue: Hue Parsing (Potential Notice)
In the colord_parse_hue method, Core uses the null coalescing operator to handle units gracefully. Gutenberg uses an older pattern that will trigger an Undefined index notice if an unsupported unit (or the default deg which is not in the array) is passed:
- Core (Safe):
$factor = $angle_units[ $unit ] ?? 1;
- Gutenberg (Amiss):
$factor = $angle_units[ $unit ]; // Notice: Undefined index if $unit is not grad/turn/rad if ( ! $factor ) { $factor = 1; }
2. Inconsistent Type Casting: Hex Parsing
In colord_parse_hex, Core explicitly casts base_convert results to (int) for the alpha channel in both Hex4 and Hex8 scenarios. Gutenberg does it for Hex8 but omits it for Hex4, relying on PHP's loose typing for the subsequent division:
- Core (Consistent): Uses
(int)in both cases. - Gutenberg (Inconsistent):
- Hex4:
'a' => 4 === strlen( $hex ) ? round( base_convert(...) / 255, 2 ) : 1, - Hex8:
'a' => 8 === strlen( $hex ) ? round( (int) base_convert(...) / 255, 2 ) : 1,
- Hex4:
3. Localization & Flexibility
Core's translation strings are more robust. They use positional arguments (%1$s) and pass "theme.json" and the setting path as arguments, which makes the strings more reusable and easier to translate without hardcoding file names:
- Core:
__( '"%1$s" in %2$s %3$s is not a hex or rgb string.' ) - Gutenberg:
__( '"%s" in theme.json settings.color.duotone is not a hex or rgb string.', 'gutenberg' )
4. Versioning & Documentation
@sinceMismatch: Therestore_image_outer_containermethod is marked@since 6.6.0in Core but@since 6.5.0in Gutenberg.- Missing Tags: Core uses the
@internaltag on nearly all supporting methods to indicate they aren't part of the public API. Gutenberg omits these tags. - Comments: Core has more detailed block comments explaining logic (e.g., in
get_global_styles_presetsregarding the selector change in 6.6.0), which are missing in Gutenberg.
5. Redundant Logic in render_duotone_support
Gutenberg includes an isset( $filter_id ) check in the final conditional of render_duotone_support. In Core, this check is omitted because the variable is guaranteed to be set if the function hasn't already returned. While not a bug, it's a structural divergence.
6. Method Order
The method order differs significantly. Core keeps the deprecated methods like get_filter_id_from_preset interleaved with the logic they relate to, while Gutenberg moves them to the very end of the class.
Summary
The hue parsing notice is the most significant functional issue. The rest are primarily related to documentation quality, localization best practices, and code consistency.
Prompt: ok, but these may be existing issues. Take a look at #74748 and verify that the changes there are unrelated to the issues you identified
The changes in PR #74748 are indeed unrelated to the functional and stylistic issues I identified. The PR is strictly focused on a performance optimization (lazy loading) and a structural sync of the rendering pipeline, rather than a line-by-line reconciliation of the internal helper methods.
Here is the verification based on the PR's scope versus the issues found:
1. PR Focus: Architectural Lazy Loading
The PR's primary change is moving from eager initialization (on wp_loaded) to lazy loading via getters (get_all_global_styles_presets and get_all_global_style_block_names).
- Verification: The issues I found in
colord_parse_hue(the potential notice) andcolord_parse_hex(the missing cast) are in static helper methods that perform pure math/string parsing. These methods were not the target of the lazy-loading refactor.
2. PR Focus: Rendering Signature Sync
The PR updates the render_duotone_support signature to match Core by accepting the WP_Block_Type object as a third parameter.
- Verification: While this syncs the "entry point" of the feature, it does not address the redundant
isset( $filter_id )check at the end of that same function in Gutenberg, which remains as a minor divergence from the Core logic.
3. PR Focus: Documentation & Types
The PR claims to improve documentation and type checking.
- Verification: The Gutenberg implementation actually has superior PHPDoc types (e.g.,
array<string, string>) compared to Core's genericarray. This confirms the PR's intent was to improve the codebase's modern PHP standards within Gutenberg, but it didn't involve syncing the@internaltags or the@sinceversion numbers (which still lag at 6.5.0 vs 6.6.0).
4. PR Focus: Localization
The PR does not appear to touch the _doing_it_wrong or __ calls.
- Verification: The hardcoded "theme.json" strings I noted in Gutenberg are localized to the
gutenbergdomain. Changing these to match Core's positional placeholder approach (%2$s) would involve changing the translation strings themselves, which is outside the "Performance/Lazy-loading" scope of PR #74748.
Conclusion
The PR is a structural and performance sync, while the issues I identified are implementation details and low-level utility bugs. They exist in parallel to the PR's changes and would require a separate "Clean-up/Utility Sync" to resolve.
|
Sorry for taking so long to reply, @westonruter, I was stuck handling a bunch of unrelated things. Thank you for the review! 🙇 Do you think this is ready to go as-is, or would you like me to spend some time bringing that diff down further? |
|
@sgomes I think it's good to go. |
What?
Currently, the Duotone metadata initialization runs on
wp_loaded. This PR changes it to instead run the first time the metadata is needed, by implementing the approach in the Core changeset https://core.trac.wordpress.org/changeset/56226.This PR replaces #74591.
Why?
Per the Core change, regarding the Core
WP_Duotone:As
WP_Duotone_Gutenbergwas implemented following the same approach, but was never updated, it kept the same performance issues. This PR updates its approach to match the one in Core.How?
As in the Core change, this PR refactors the
WP_Duotone_Gutenbergclass to lazily load the global styles andtheme.jsondata, only when a block that supports duotone is encountered.render_duotone_support()was changed to take a third parameter to reuse the existingWP_Block_Typeobject passed to the filter, to save it being looked up again.The code has also got improved type checking, via the use of the util function
block_has_support, and improved type documentation beyond the Core change.Unlike in the Core change, the
set_global_styles_presets()andset_global_style_block_names()functions are preserved, as they are public and may be in use by external code (albeit unlikely). They're now marked as deprecated to avoid new usage. Please let me know if you'd rather see them removed altogether, and I'll be happy to make the change.Testing Instructions
Smoke-test the Duotone feature and ensure that it continues to work correctly, by adding a Duotone filter to an image block and ensuring the page loads correctly, with the filter correctly applied.
Also ensure that the PHP unit tests work correctly, as they needed some small updates to match the changed function signature for
render_duotone_support().