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.

Sign up for GitHub

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

Jump to bottom

Block Hooks: Run block hooks on all block themes #45581

Merged
merged 6 commits into from Mar 21, 2024
Merged

Block Hooks: Run block hooks on all block themes #45581

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e75dc2d
Remove the condition to only allow Woo block hooks to run on approved…
tjcafferkey Mar 14, 2024
95dedc1
Add changefile(s) from automation for the following project(s): wooco…
invalid-email-address Mar 14, 2024
d6e001e
Merge branch 'trunk' into remove-block-hooks-theme-include-list-guard
tjcafferkey Mar 19, 2024
0127448
Add since version number to BlockHooksTrait
tjcafferkey Mar 20, 2024
5b68f23
Update filter since version
tjcafferkey Mar 20, 2024
03d087c
Merge branch 'trunk' into remove-block-hooks-theme-include-list-guard
tjcafferkey Mar 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions plugins/woocommerce/changelog/45581-remove-block-hooks-theme-include-list-guard
View file
Open in desktop
@@ -0,0 +1,4 @@
Significance: minor
Type: enhancement

Block Hooks: Run block hooks on all block themes instead of approved themes only.
13 changes: 2 additions & 11 deletions plugins/woocommerce/src/Blocks/Utils/BlockHooksTrait.php
View file
Open in desktop
Expand Up @@ -38,18 +38,9 @@ public function register_hooked_block( $hooked_blocks, $position, $anchor_block,
*
* @since $VID:$
*/
$pattern_exclude_list = apply_filters( 'woocommerce_hooked_blocks_pattern_exclude_list', array( 'twentytwentytwo/header-centered-logo', 'twentytwentytwo/header-stacked' ) );
$pattern_exclude_list = apply_filters( 'woocommerce_hooked_blocks_pattern_exclude_list', array( 'twentytwentytwo/header-centered-logo', 'twentytwentytwo/header-stacked', 'blockbase/header-centered' ) );
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I am wondering if we should directly contribute to blockbase theme instead of excluding it here as Automattic owns it. Shouldn't themes leverage the woocommerce_hooked_blocks_pattern_exclude_list hook to exclude patterns instead of incorporating these exclusions within the WooCommerce core? 🧐

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we should do both. Exclude it for now > Contribute > Include it back in. I'm leaving it excluded for now because if we want to fix this issue it'll fall quite low on the priority list for now and I'm unsure on timescales.

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense to me 🤝


/**
* A list of theme slugs to execute this with. This is a temporary
* measure until improvements to the Block Hooks API allow for exposing
* to all block themes.
*
* @since $VID:$
*/
$theme_include_list = apply_filters( 'woocommerce_hooked_blocks_theme_include_list', array( 'Twenty Twenty-Four', 'Twenty Twenty-Three', 'Twenty Twenty-Two', 'Tsubaki', 'Zaino', 'Thriving Artist', 'Amulet', 'Tazza' ) );

if ( $context && in_array( $active_theme_name, $theme_include_list, true ) ) {
if ( $context ) {
foreach ( $this->hooked_block_placements as $placement ) {
if (
$placement['position'] === $position &&
Expand Down