WordPress.org

Make WordPress Core

Opened 22 months ago

Last modified 4 days ago

#46187 new enhancement

Add a dynamic hook to filter the content of a single block.

Reported by: manzoorwani.jk Owned by:
Milestone: 5.7 Priority: normal
Severity: normal Version:
Component: Editor Keywords: has-patch needs-refresh
Focuses: Cc:

Description

render_block filter in render_block() is very helpful. It would be good to have a dynamic hook (render_block_{$block_name}) to filter only the content of a specific block, to avoid the need of using the conditionals inside the filter callback.

register_block_type() does provide an option to pass render_callback but still that's not useful when you are not the creator of the block.

Attachments (3)

46187.diff (868 bytes) - added by manzoorwani.jk 22 months ago.
46187-1.diff (1.5 KB) - added by manzoorwani.jk 22 months ago.
46187-2.diff (1.0 KB) - added by manzoorwani.jk 4 days ago.

Download all attachments as: .zip

Change History (14)

#1 @birgire
22 months ago

  • Keywords has-patch added

Thanks for the ticket @manzoorwanijk

At a first glance this sounds useful.

Using a dynamic filter, we should make sure that the block names can't contain characters that are not supported as filter names.

Few questions come to mind: What limitations are on the block names and filter names? For example what is the maximum length of filter names and block names? Is it the same?

Regarding the if-condition in 46187.diff, should e.g. "0" be allowed, that is if "0" is a possible block name?

If this goes through, then it would need a @since x.y.z for the the filter documentation.

There seems to be some code standard adjustments needed in 46187.diff :

  • I think the variable assigning within the if-condition is not recommended.
  • Missing end of line periods in inline documentation.
  • Fix indentation.
  • First word in inline documentation not capitalized.

Should the specific dynamic filter come before or after the existing and more general render_block filter? I didn't find quickly some good existing core examples for the case of filters, but we have e.g. the dynamic action "save_post_{$post->post_type}" running before "save_post". In that case the more specific comes before the general one. This is the same order as in 46187.diff.

#2 follow-up: @manzoorwani.jk
22 months ago

Thank you for the valuable inputs @birgire

I have made the changes pointed out by you and also moved the dynamic filter after the general filter which seems to be a better place for a dynamic filter unlike hooks, couldn't find an example from the core.

Also, about the number of characters in a filter tag, I found your own answer 🙂

As far as the allowed characters in a filter are concerned, WordPress is stricter when it comes to a block name, so it should be pretty fine to use a block name in a dynamic filter, provided / between namespace and block is not a problem.

#3 in reply to: ↑ 2 @birgire
22 months ago

Replying to manzoorwani.jk:

lol, thanks for the link 🙂

#4 @noisysocks
10 days ago

  • Keywords close added

Hey, thanks for the patch. What's the benefit of a dynamic filter name as opposed to using render_block and checking $block['blockName']?

In my opinion it is not worth doing this given that there is a sensible alternative and given that it is confusing that the dynamic part of the filter name would nearly never match the block's name, e.g. core/image would have to become core_image.

#5 @manzoorwani.jk
10 days ago

@noisysocks benefit of that dynamic filter is written in the description - i.e. to avoid unnecessary if conditions inside the filter callback and also to avoid calling the callback on every single block.

Also, why core/image cannot work in a filter name? Doesn't it simply become an array index?
FYI, / is pretty fine for array indices.

#6 @noisysocks
10 days ago

Yes, / is a valid character in an array index but it doesn't fit with the WordPress naming conventions for filters.

https://make.wordpress.org/core/handbook/best-practices/coding-standards/php/#naming-conventions

Another problem if we do this is that it will not be intuitive which filter (render_block vs render_block_$block) is called first. It will also not be possible for developers to use the $priority param in add_filter to e.g. make their render_block_$block filter fire before or after another plugin's render_block filter.

I don't personally think that the convenience is worth the added ambiguity.

#7 @manzoorwani.jk
10 days ago

Well, none of those arguments is valid.

You need to follow the coding standards in the code you write and you are not responsible for the dynamic part of the filter. An example of that is save_post_{$post->post_type}, where post_type can have some characters like - which apparently are not as per the coding standards for filter names, although there is nothing mentioned about filter names in WPCS. Another example is admin_post_{$action}, where I can write anything as $action, including /.

Regarding the order of dynamic and normal filter, aren't there tens of such dynamic filters in WordPress?? Is there any standard for order of such filters? Example is again save_post vs save_post_{$post_type}.

Last edited 10 days ago by manzoorwani.jk (previous) (diff)

#8 @noisysocks
10 days ago

  • Keywords 2nd-opinion added

#9 @johnbillion
9 days ago

  • Keywords close 2nd-opinion removed
  • Milestone changed from Awaiting Review to 5.7

This is a fairly common pattern in core, see for example: gettext/gettext_{$domain}, plugin_action_links/plugin_action_links_{$plugin_file}, manage_posts_columns/manage_{$post_type}_posts_columns, etc. The dynamically named filter should come after the generic one.

#10 @noisysocks
8 days ago

  • Keywords needs-refresh added

Hey @manzoorwanijk, could you please refresh the patch? It no longer applies cleanly.

#11 @manzoorwani.jk
4 days ago

@noisysocks Since that filter has been moved to wp-includes/class-wp-block.php, I have refreshed the patch. Thanks

Note: See TracTickets for help on using tickets.