#46187 closed enhancement (fixed)
Add a dynamic hook to filter the content of a single block.
Reported by: | manzoorwani.jk | Owned by: | SergeyBiryukov |
---|---|---|---|
Milestone: | 5.7 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Editor | Keywords: | has-patch has-unit-tests commit has-dev-note |
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 (4)
Change History (19)
#2
follow-up:
↓ 3
@
6 years 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
@
6 years ago
Replying to manzoorwani.jk:
lol, thanks for the link 🙂
#4
@
4 years 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
@
4 years 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
@
4 years 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
@
4 years 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}
.
#9
@
4 years 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
@
4 years ago
- Keywords needs-refresh added
Hey @manzoorwanijk, could you please refresh the patch? It no longer applies cleanly.
#11
@
4 years ago
@noisysocks Since that filter has been moved to wp-includes/class-wp-block.php
, I have refreshed the patch. Thanks
#12
@
4 years ago
- Keywords has-unit-tests commit added; needs-refresh removed
Thanks @manzoorwanijk! 46187-2.diff looks good to me. I tested it by writing a unit test which is in 46187.2.diff.
#13
@
4 years ago
- Owner set to SergeyBiryukov
- Resolution set to fixed
- Status changed from new to closed
In 50123:
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 :
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.