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

Use lower priority for block context filters #4780

Closed
wants to merge 1 commit into from

Conversation

dlh01
Copy link

@dlh01 dlh01 commented Jul 3, 2023

@ockham
Copy link
Contributor

ockham commented Jul 6, 2023

Good catch; this admittedly didn't occur to me when working on WordPress/gutenberg#50313.

I think your solution makes sense; I was briefly debating whether we should use a slightly higher value (e.g. 5) for priority in order to allow other filters to kick in earlier, but I think it's true that from a backwards-compat point-of-view, using the lowest possible value is the closest we can get to retain the previous behavior.

However, this change will need to go "through" Gutenberg: As the e2e tests are flagging, blocks' dynamic PHP code is synced from the @wordpress/block-library npm package at build time in Core -- any changes made directly to that Code in Core will be flagged as a collision.

I'll go ahead and will file a GB PR 😊

*/
add_filter( 'render_block_context', $filter_block_context );
add_filter( 'render_block_context', $filter_block_context, 0 );
Copy link
Contributor

Choose a reason for hiding this comment

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

One minor note: The docs say that

priorities are positive integers, typically between 1 and 20

Suggested change
add_filter( 'render_block_context', $filter_block_context, 0 );
add_filter( 'render_block_context', $filter_block_context, 1 );

@ockham
Copy link
Contributor

ockham commented Jul 6, 2023

I'll go ahead and will file a GB PR 😊

WordPress/gutenberg#52364

@ockham
Copy link
Contributor

ockham commented Jul 6, 2023

Per #4780 (comment), I'll go ahead and close this PR.

The suggested fix is over at WordPress/gutenberg#52364 😊

@ockham ockham closed this Jul 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants