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

Webfonts: increase priority of init hook to account for block reregistration #41569

Merged

Conversation

ramonjd
Copy link
Member

@ramonjd ramonjd commented Jun 7, 2022

Fixes #41296 (I think)

What?

Webfonts are registered via the init hook at the highest priority.

Gutenberg server-side blocks are registered via the init hook with a priority value of 20. E.g., add_action( 'init', 'register_block_core_image', 20 );

This priority value is added dynamically during the build. See: tools/webpack/blocks.js.

gutenberg_register_webfonts_from_theme_json() calls WP_Theme_JSON_Resolver_Gutenberg::get_merged_data(),
which instantiates WP_Theme_JSON_Gutenberg();

Unfortunately this happens before Gutenberg server-side blocks are re-registered, therefore any plugin-only updates to block.json for example do not appear in the theme.json merged data.

Why?

We want to make sure Gutenberg blocks are re-registered before any Theme_JSON operations take place so that we have access to updated merged data.

How?

Bumping priority from 20 to 21: add_action( 'init', 'gutenberg_register_webfonts_from_theme_json', 21 );

Testing Instructions

Add an __experimentalSelector property to a block in its block.json file:

	"supports": {
		...
		"__experimentalSelector": ".wp-block-image img"
	},

Check that the selector is registered in get_blocks_metadata

And that blocks load as per normal 😄

Go through the test steps on the original webfonts PR:

…Resolver_Gutenberg::get_merged_data()`, which instantiates `WP_Theme_JSON_Gutenberg()`;

Gutenberg server-side blocks are registered via the init hook with a priority value of `20`. E.g., `add_action( 'init', 'register_block_core_image', 20 )`;
This priority value is added dynamically during the build. See: tools/webpack/blocks.js.
We want to make sure Gutenberg blocks are re-registered before any Theme_JSON operations take place so that we have access to updated merged data.
@ramonjd ramonjd added [Type] Bug An existing feature does not function as intended [Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi [Feature] Webfonts labels Jun 7, 2022
@ramonjd ramonjd requested a review from spacedmonkey as a code owner June 7, 2022 02:34
@ramonjd ramonjd self-assigned this Jun 7, 2022
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 fix @ramonjd 🎉

✅ Tested re-registration of blocks by updating the image block to have an __experimentalSelector and opting into expanded border support. Works well.

✅ Tested successfully following the instructions on the original Webfonts PR.

LGTM! 🚢

@ramonjd ramonjd merged commit 877668d into trunk Jun 7, 2022
@ramonjd ramonjd deleted the call/get-merged-data-after-gutenberg-reregister-blocks branch June 7, 2022 22:32
@github-actions github-actions bot added this to the Gutenberg 13.5 milestone Jun 7, 2022
@mburridge mburridge added the Needs Dev Note Requires a developer note for a major WordPress release cycle label Sep 13, 2022
@mburridge
Copy link
Contributor

Added the Needs Dev Note label in case this needs a dev note (either individual or as part of a "misc" dev note) for WP 6.1 release.

@ramonjd ramonjd removed the Needs Dev Note Requires a developer note for a major WordPress release cycle label Jun 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Theme.json: __experimentalSelector not working for blocks newly adopting it
3 participants