This repository has been archived by the owner on Feb 23, 2024. It is now read-only.
Implement the Block Hooks API to automatically inject the Mini-Cart block #11745
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nerrad
added
type: enhancement
|
The release ZIP for this PR is accessible via: Script Dependencies ReportThere is no changed script dependency between this branch and trunk. This comment was automatically generated by the TypeScript Errors Report
🎉 🎉 This PR does not introduce new TS errors. |
|
Size Change: +3 B (0%) Total Size: 1.53 MB
ℹ️ View Unchanged
|
|
Looks like a test needs updated because the default changed. Noting I've seen this and will address. |
13 tasks
|
This PR has been marked as If deemed still relevant, the pr can be kept active by ensuring it's up to date with the main branch and removing the stale label. |
github-actions
bot
added
the
status: stale
github-actions
bot
removed
the
status: stale
nerrad
force-pushed
the
add/block-hooks-mini-cart
branch
from
0a19b79 to
1ac77ef
Compare
nefeline
approved these changes
Nov 29, 2023
There was a problem hiding this comment.
Great work! Confirmed the Mini-Cart block is being correctly injected in all expected scenarios.
While for the most part the API works as advertised. However, there are a couple caveats that need to be addressed before we can roll this out more widely (and remove our included patterns/template parts for the header)
👍
Test Results
The mini-cart block should only appear after the navigation block in header patterns or template parts.
✅ Added the navigation block across different areas of the site: header, footer, posts, and pages, and confirmed that the mini-cart is only displayed in the header.
It should not appear in custom patterns or template parts.
✅ Confirmed that when the template is edited, and the auto-injected mini-cart block is removed, it is not displayed again: the user decision is respected. When clicking on "Clear Customizations" for the header template part, confirmed the mini-cart block is auto-inserted again.
It should only be auto-injected when the Twenty Twenty-Four theme is active.
✅ Confirmed that the mini-cart is injected exclusively on TT4: while having other themes such as TT3 or TT2, the block is not injected.
You should never see two mini-cart blocks in a header pattern or template part (for example WooCommerce header patterns should be untouched).
✅ Inserted all WooCommerce header patterns and confirmed the mini-cart is not injected and not displayed twice for them.
If the mini-cart block is moved or removed and the template part/pattern/template is saved, that should persist and not change on reload.
✅ Confirmed that the mini-cart block remains excluded if the user deliberately removes it. The change is persistent and does not change on page reload.
The auto-injection should be reflected on the frontend.
✅ Confirmed the auto-injected mini-cart block is visible on the front end.
Verify that:
woocommerce_blocks_mini_cart_auto_insert_pattern_exclude_listfilter works as expected for excluding auto-injection on a specific pattern slug (it'd have to be a pattern that would normally match the conditions).
✅ Confirmed that auto-injection doesn't happen for patterns marked for exclusion while using this filter.
woocommerce_blocks_mini_cart_auto_insert_theme_include_listfilter works as expected for specific themes that are active as named in the array.
✅ Tested with 'Twenty Twenty-Three' and confirmed that whenever used, the filter enables auto-insertion for the selected theme(s)
The Block Hooks API currently doesn’t allow for setting the default state of the block injected into content so this ensures the mini-cart block has a better default state for injection. The current default (displaying total value in cart) takes up more width increasing the risk of poor layout.
On thinking about this, I don’t think these need to be experimental. They are intentionally provided as escape hatches for hosts/themes that want to opt-in/out so we’ll have to support them when this is shipped (at least until its no longer needed!)
nerrad
force-pushed
the
add/block-hooks-mini-cart
branch
from
1ac77ef to
7f64bae
Compare
Defaults for the block affect this expectation.
|
I'm merging this PR! |
Aljullu
pushed a commit
that referenced
this pull request
Dec 4, 2023
…lock (#11745) * Change the default for Mini Cart block The Block Hooks API currently doesn’t allow for setting the default state of the block injected into content so this ensures the mini-cart block has a better default state for injection. The current default (displaying total value in cart) takes up more width increasing the risk of poor layout. * Utilize Block Hooks to automatically inject mini-cart block. * include experimental prefix on filters * Fix filter name. * remove experimental prefix. On thinking about this, I don’t think these need to be experimental. They are intentionally provided as escape hatches for hosts/themes that want to opt-in/out so we’ll have to support them when this is shipped (at least until its no longer needed!) * fix variable name! * fix unit tests because of new default * remove another incorrect text expectation Defaults for the block affect this expectation. * fix E2E tests * Mini Cart Block: improve E2E test * fix: improve check for the Product Collection block --------- Co-authored-by: Luigi Teschio <gigitux@gmail.com>
Aljullu
added a commit
that referenced
this pull request
Dec 4, 2023
Aljullu
added a commit
that referenced
this pull request
Dec 5, 2023
This was referenced Dec 13, 2023
This was referenced Jan 4, 2024
tjcafferkey
reviewed
Jan 4, 2024
| @@ -72,6 +72,7 @@ public function __construct( AssetApi $asset_api, AssetDataRegistry $asset_data_ | |||
| protected function initialize() { | |||
| parent::initialize(); | |||
| add_action( 'wp_loaded', array( $this, 'register_empty_cart_message_block_pattern' ) ); | |||
| add_action( 'hooked_block_types', array( $this, 'register_auto_insert' ), 10, 4 ); | |||
There was a problem hiding this comment.
I don't know how/why this worked but the hook is a filter according to the docs and I've only just realised this was added as an action.
There was a problem hiding this comment.
It's a little known fact that add_action is actually a wrapper around add_filter. To make the intent clear, it would be better to use the appropriate function though.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
WordPress 6.4 included a new Block Hooks API that makes it possible for us to automatically inject Woo Blocks into content. This is very useful for blocks like the Mini-Cart block where we want it to be automatically available in header template parts and patterns when WooCommerce is activated.
While for the most part the API works as advertised. However, there are a couple caveats that need to be addressed before we can roll this out more widely (and remove our included patterns/template parts for the header):
WP_Block_Templatecontext limitationsWhen an instance of
WP_Block_Templateis provided (for template part and template context), the$contentproperty isnull. Without the serialized template content, doing finer-grained restrictions based on that content is not possible.An example of where this is problematic is when an active block theme includes custom template parts or templates with the mini-cart block already present, there is no way to prevent auto-injection and the content would end up with two instances of the block.
An example where this surfaces:
I've submitted a fix for this here.
As a workaround until the above fix is in place, I've implemented opt-in handling for themes via a filter and the only theme this functionality will support out of the box is
Twenty Twenty-Fourwhich appears to work okay with auto-injection.Lack of control over layouts where the block is injected.
In some cases, a pattern or a template part might not have the anchor block (in this case
core/navigation) wrapped in a container which is responsive. This means that when the mini-cart block is injected, it's default behaviour is not great. A couple examples (from patterns included with a block theme):As a workaround, this is being restricted via the filter added for opt-in theme application as well as another filter for excluding patterns by slug. Block themes (or hosts) that want to work great with Woo can use these two filters to fine-tune the auto-injection, at least until improvements are made to the Block Hooks API that allows for more programmatic fine-tuning in Woo.
Using a better default for Mini-Cart block
This PR changes the default of the Mini-Cart block to be just the icon instead of the icon plus the total value of items in the cart. The reason for this is two-fold:
Auto-injection does not work for customized template parts, templates or custom patterns.
This is by design with the initial iteration of the Block Hooks API. However, this could change in the future so we'll have to watch for this and adjust.
Testing Instructions
Currently, the application of this auto-injection only applies when Twenty Twenty-Four is active. In future iterations, we can expand the list of themes this supports as we test this out and/or eventually remove the restrictions once the Block Hooks API has more support.
General expectations for testing:
With TT4 active
This is how things look before this PR's changes:
CleanShot.2023-11-10.at.16.00.38.mp4
And how things look after:
CleanShot.2023-11-10.at.16.03.19.mp4
Developer Testing
Verify that:
woocommerce_blocks_mini_cart_auto_insert_pattern_exclude_listfilter works as expected for excluding auto-injection on a specific pattern slug (it'd have to be a pattern that would normally match the conditions).woocommerce_blocks_mini_cart_auto_insert_theme_include_listfilter works as expected for specific themes that are active as named in the array.WooCommerce Visibility
Required:
Checklist
Required:
[type]label or a[skip-changelog]label.Conditional:
[skip-changelog]label is not present).Next Steps
Some potential next steps after this PR is merged:
AbstractBlocklevel?) to make it easier to roll out auto-injection for various Woo Blocks.Changelog