-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Theme JSON: Check for null values to cater for blockGap #59258
Conversation
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. |
This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress. If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged. If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack. Thank you! ❤️ View changed files❔ lib/class-wp-theme-json-gutenberg.php ❔ phpunit/class-wp-theme-json-test.php |
* WP_Theme_JSON_Gutenberg::resolve_variables may be called with merged data from WP_Theme_JSON_Resolver_Gutenberg::get_merged_data() | ||
* WP_Theme_JSON_Resolver_Gutenberg::get_block_data() sets blockGap for supported blocks to `null` if the value is not defined. | ||
*/ | ||
$this->assertNull( $styles['blocks']['core/post-template']['spacing']['blockGap'], 'core/post-template block: blockGap' ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i added this test to check the happy path, e.g., resolution of spacing presets when those presets are applied to blockGap values.
Flaky tests detected in ece1a4d. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7999294851
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
98c4d02
to
ece1a4d
Compare
@ramonjd Do you want to backport this to WP 6.5? If so please can you add the requisite labels? Many thanks. |
This will require a PHP back port. I was going to prep one today. I'm not sure it'll make 6.5, but I'll label the trac ticket to try 😄 |
I noticed the backport isn't committed. I'm going to try and wrangle this in time for Beta 3 today. Perhaps @swissspidy or @youknowriad can commit as I don't have permissions. |
What?
I noticed this PHP error while trying to extract block styles, whose values have a CSS var, using wp|gutenberg_get_global_styles().
It's complaining about passing a null value to
strpos()
in WP_Theme_JSON_Gutenberg::convert_variables_to_valueSo I added a check to make sure the value is not empty before we run it through
strpos()
.Why
When using
wp|gutenberg_get_global_styles()
with theresolve-variables
flag like this —`gutenberg_get_global_styles( array( 'typography', 'fontSize' ), array( 'block_name' => 'core/search', 'transforms' => array( 'resolve-variables' ) ) );`
— it will grab merged Theme JSON data from
WP_Theme_JSON_Resolver_Gutenberg::get_merged_data()
and run it throughWP_Theme_JSON_Gutenberg::resolve_variables
, which callsself::convert_variables_to_value
.The snag is that:
WP_Theme_JSON_Resolver_Gutenberg::get_merged_data()
callsstatic::get_block_data()
static::get_block_data()
sets blockGap for supported blocks tonull
if the value is not defined.Because
WP_Theme_JSON_Gutenberg::convert_variables_to_value
is expecting strings as values, it will trigger the error.Testing Instructions
Run the tests:
npm run test:unit:php:base -- --filter WP_Theme_JSON_Gutenberg_Test
If you like, try to call the function from inside a block render/block supports function, or anywhere you're sure that block styles have already been registered
You should see
NULL
and no errors.