Skip to content
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

Navigation block: Use apply_block_hooks_to_content() #65703

Merged

Conversation

ockham
Copy link
Contributor

@ockham ockham commented Sep 27, 2024

What?

In the Navigation block, use apply_block_hooks_to_content (introduced in WP 6.6), if available.

Why?

This is part of a centralization effort, i.e. to make apply_block_hooks_to_content the canonical entrypoint for applying the Block Hooks logic to a given piece of block markup. The same change has already been applied to the relevant call sites of Block Hooks logic in Core, see WordPress/wordpress-develop#7220.

This is particularly relevant for WordPress/wordpress-develop#7443, which will ensure that apply_block_hooks_to_content respects a hooked block's "multiple": false setting. In other words, the present PR is required to make sure that a hooked block isn't inserted more than once into a Navigation menu if it has "multiple": false set.

How?

By checking if apply_block_hooks_to_content exists, and if it does, by invoking it instead of the more indirect sequence of make_before_block_visitor/make_after_block_visitor, followed by traverse_and_serialize_block.

Testing Instructions

Verify that hooked blocks insertion into the Navigation menu works as before. For more detailed testing instructions, refer to #57754.

@ockham ockham added [Type] Enhancement A suggestion for improvement. [Block] Navigation Affects the Navigation Block Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta labels Sep 27, 2024
@ockham ockham self-assigned this Sep 27, 2024
@ockham ockham requested a review from ajitbohra as a code owner September 27, 2024 10:53
Copy link

github-actions bot commented Sep 27, 2024

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 props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: ockham <bernhard-reiter@git.wordpress.org>
Co-authored-by: ramonjd <ramonopoly@git.wordpress.org>
Co-authored-by: aaronrobertshaw <aaronrobertshaw@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Copy link
Member

@ramonjd ramonjd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went through the test instructions in #57754

✅ The hooked_block_types PHP filter works as expected, including switching positions
blockHooks in block.json also works as expected, including switching positions

The injected blocks only appear once

Copy link
Contributor

@aaronrobertshaw aaronrobertshaw left a 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 👍

This tests as advertised for me:

✅ Login/Logout appears as inner block of core navigation block
✅ Block is only added once
✅ Block appears when editing header template part
✅ Custom positioning of block works and is persisted
✅ Removal of block works
✅ When using blockHooks in block.json, the same behaviour occurs

Screenshot 2024-09-30 at 11 22 11 am Screenshot 2024-09-30 at 11 21 22 am

LGTM 🚢

@noisysocks noisysocks merged commit 56f6deb into trunk Sep 30, 2024
64 checks passed
@noisysocks noisysocks deleted the update/use-apply-block-hooks-to-content-in-navigation-block branch September 30, 2024 01:39
@github-actions github-actions bot added this to the Gutenberg 19.4 milestone Sep 30, 2024
@github-actions github-actions bot removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Sep 30, 2024
gutenbergplugin pushed a commit that referenced this pull request Sep 30, 2024
* Navigation block: Use apply-block-hooks-to-content

* WPCS

Co-authored-by: ockham <bernhard-reiter@git.wordpress.org>
Co-authored-by: ramonjd <ramonopoly@git.wordpress.org>
Co-authored-by: aaronrobertshaw <aaronrobertshaw@git.wordpress.org>
@github-actions github-actions bot added the Backported to WP Core Pull request that has been successfully merged into WP Core label Sep 30, 2024
Copy link

I just cherry-picked this PR to the wp/6.7 branch to get it included in the next release: e2f2dce

@ockham
Copy link
Contributor Author

ockham commented Sep 30, 2024

Thank you very much, @ramonjd and @aaronrobertshaw!

@ockham
Copy link
Contributor Author

ockham commented Sep 30, 2024

and @noisysocks 🙂

huubl pushed a commit to huubl/gutenberg that referenced this pull request Oct 2, 2024
* Navigation block: Use apply-block-hooks-to-content

* WPCS

Co-authored-by: ockham <bernhard-reiter@git.wordpress.org>
Co-authored-by: ramonjd <ramonopoly@git.wordpress.org>
Co-authored-by: aaronrobertshaw <aaronrobertshaw@git.wordpress.org>
huubl added a commit to huubl/gutenberg that referenced this pull request Oct 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backported to WP Core Pull request that has been successfully merged into WP Core [Block] Navigation Affects the Navigation Block [Type] Enhancement A suggestion for improvement.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants