-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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 Hooks API: Harmonize ignoredHookedBlocks metadata injection logic #6550
Block Hooks API: Harmonize ignoredHookedBlocks metadata injection logic #6550
Conversation
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
…ase_object to _build_block_template_object_from_database_object
@ockham interested to know your thoughts on this approach. I can see an argument against the new function ( I would prefer to write more unit tests but will wait on your feedback on the general approach before doing so. |
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 Core Committers: Use this line as a base for the props when committing in SVN:
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
// Note: When can we remove this conditional? | ||
if ( ! has_filter( 'rest_pre_insert_wp_navigation', 'block_core_navigation_update_ignore_hooked_blocks_meta' ) && ! has_filter( 'rest_pre_insert_wp_navigation', 'gutenberg_block_core_navigation_update_ignore_hooked_blocks_meta' ) ) { |
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.
We can remove it when the block_core_navigation_update_ignore_hooked_blocks_meta
filter is no longer attached to the rest_pre_insert_wp_navigation
hook. Since this is done by GB code, we'll have to wait until it's removed there, and blocks code is sync'd to Core.
OTOH, we'll have to retain the code in GB for at least two WP releases, as it's required to support the current WordPress version and the one before. So if this PR makes it into WP 6.6, GB still needs to support 6.5, so we'll have to retain the code there.
I'd suggest doing it the other way round: Add the filter unconditionally here, and instead add (yet another 😬) has_filter
check in GB. (We will need to wait for GB to release a version that includes that check before we can land the Core PR in order to prevent Core trunk
+ GB stable from breaking.)
Hmm, looks like I'm starting to think that we should after all start with a distinct filter function for |
We could probably extract a helper function for the applying the Block Hooks algorithm itself (i.e. running |
I think it makes sense to keep the two concerns (injecting the data into template markup vs updating it in I've experimented with this approach here #6604 Keen to know how you feel it compares to this one based on your comments above. |
Loving it! That one reads a lot more naturally IMO 😄 |
Should we close this one in favor of #6604? 😊 |
Closing 😄 |
This PR aims to haromize the injection of the
ignoredHookedBlocks
data. We currently have two points of processing for this with the first being forwp_template
&wp_template_part
and the second being forwp_navigation
which is stored separately in thewp_postmeta
table. For us to combine these into a single approach this PR has a conditional statement to handlewp_navigation
items separately.I've also created (and tested) a new function to build template objects (
WP_Template
) from database objects (stdClass
) which is where we intercept this data when the controller prepares it for the database.Trac ticket: https://core.trac.wordpress.org/ticket/60759
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.