-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Block Themes: Ensure that _build_block_template_result_from_file()
injects theme
attribute
#5186
Conversation
…jects theme attribute
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.
Thanks for the PR @ockham! I've left some feedback for consistency with test standards and conventions. 🙂
References:
Co-authored-by: Mukesh Panchal <[email protected]>
As for the functionality, these new tests cover the part of the functionality that is going to be refactored as outlined in #5158 to bring support for the new Block Hooks feature. I can confirm that the tests look as intended. |
@costdev Thank you for your feedback! I believe I've addressed it all 😊 Would you mind giving it another look when you have a moment? I'd love to land this soon as it's only the first step for Block Hooks to land in 6.4 😊 |
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.
Let's get this in, as all the feedback seems to be addressed. Let's ensure to follow up with the code coverage part seperately.
Thanks all! Committed to Core in https://core.trac.wordpress.org/changeset/56562/. |
While we have coverage for
_inject_theme_attribute_in_block_template_content
, those tests only verify that that function does what is supposed to do; there's however no guarantee that_build_block_template_result_from_file
uses that function (or whatever other technique) to actually inject the theme attribute (see).Testing Instructions
trunk
, and run unit tests (npm run test:php -- --group=block-templates
). Verify that they pass (although they shouldn't) ✅ 👎Trac ticket: https://core.trac.wordpress.org/ticket/59325
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.