-
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 Hooks API: Move ignoredHookedBlocks metadata injection logic #6604
Block Hooks API: Move ignoredHookedBlocks metadata injection logic #6604
Conversation
cc @ockham |
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. |
This is shaping up nicely! 😄 |
We have some test coverage in https://github.com/WordPress/gutenberg/blob/6108134aae75d1bd4826256490c609cb29044cd8/phpunit/blocks/block-navigation-block-hooks-test.php that we can probably copy over (with some small tweaks). |
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. |
tests/phpunit/tests/blocks/updateIgnoredHookedBlocksPostMeta.php
Outdated
Show resolved
Hide resolved
tests/phpunit/tests/blocks/updateIgnoredHookedBlocksPostMeta.php
Outdated
Show resolved
Hide resolved
tests/phpunit/tests/blocks/updateIgnoredHookedBlocksPostMeta.php
Outdated
Show resolved
Hide resolved
tests/phpunit/tests/blocks/updateIgnoredHookedBlocksPostMeta.php
Outdated
Show resolved
Hide resolved
tests/phpunit/tests/blocks/updateIgnoredHookedBlocksPostMeta.php
Outdated
Show resolved
Hide resolved
tests/phpunit/tests/blocks/updateIgnoredHookedBlocksPostMeta.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.
Thank you very much, this is looking great! 🚀 Especially appreciate the test coverage, good thinking to add test cases for missing post ID and type 👍
Apologies for the number of notes I left, most of it is minor and/or coding standards compliance stuff 😬
Awesome, thank you very much! We'll need to land the GB counterpart PR first; I've approved it, you wanna do the honors? After that, we'll still need to wait for the code to be sync'd over to Core, so that the built Navigation block code in Core is updated and the "old" filter isn't still being added there if the new one is present. As always, the package sync will happen before Feature Freeze (i.e. next Tue, June 4), but probably only after the GB RC is released (this Friday). This means we'll likely land this PR on Monday 🙂 |
I remembered you won't be able to do so today, so I want ahead and merged it 😬 Hope that's okay! |
c5d5f7c
to
3364ae2
Compare
Committed to Core in https://core.trac.wordpress.org/changeset/58291. |
Trac ticket: https://core.trac.wordpress.org/ticket/60759
Testing instructions:
wp_postmeta
table to see ifcore/loginout
has been added to a row with themeta_key
=>_wp_ignored_hooked_blocks
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.