-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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: Insert metadata at the same time as hooked blocks #6358
Block Hooks API: Insert metadata at the same time as hooked blocks #6358
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. |
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. |
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.
Looking good overall, thank you! Just a few small thing we should tweak a bit 😄
tests/phpunit/tests/blocks/insertHookedBlocksAndSetIgnoredHookedBlocksMetadata.php
Outdated
Show resolved
Hide resolved
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.
Works like a charm! Thank you very much for taking care of this 🚀
Committed to Core in https://core.trac.wordpress.org/changeset/58186. |
Trac ticket: https://core.trac.wordpress.org/ticket/59574
The block hooks UI toggle relies on the
ignoredHookedBlocks
when determining whether to show the toggle in the Site Editor UI. The current problem is that for uncustomized templates we don't add this meta data. Currently the visitor functions have a default callback ofinsert_hooked_blocks
and we only add the meta data when we're writing to the database via an available filter in the template controller.This PR creates a new default callback which both inserts the hooked blocks and adds the
ignoredHookedBlocks
meta data to the anchor block, and makes this the new default callback for the visitor functions. This means we will continue to addignoredHookedBlocks
meta when writing to the database meaning this operation happens twice. Although not ideal this is necessary to cover the following scenarios;ignoredHookedBlocks
meta to it to prevent hooked blocks hooking on to it unexpectedly on the frontend, this means we keep parity between the frontend and editor.It is worth noting that with this change it means the first hooked block to insert relative to its anchor block will be accepted. Any additional blocks of the same type (ie a second
core/loginout
block) trying to hook onto the same anchor block will be ignored, irrespective of the position.Manual testing instructions:
1 . Clear template customizations
2. Add the first code block to your themes functions.php
3. Go to the Single Posts template and check the loginout block is inserted after the post content.
4. Click the post content to check the toggle exists to show/hide the hooked block.
5. Save the template
6. Add the second code block and reload the Single Posts template. Follow steps 1-4 again and check them against the Page List block.
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.