-
Notifications
You must be signed in to change notification settings - Fork 219
Enable Compatibility Layer for Product Collection #10332
Enable Compatibility Layer for Product Collection #10332
Conversation
The release ZIP for this PR is accessible via:
Script Dependencies ReportThe
This comment was automatically generated by the TypeScript Errors Report
🎉 🎉 This PR does not introduce new TS errors. |
Size Change: 0 B Total Size: 1.34 MB ℹ️ View Unchanged
|
Perhaps better testing steps would be to trigger hooks programmatically rather than by using extensions 🤔 What do you think @gigitux? |
I suggest following the Tung approach #8172. Furthermore, it would be great to add some E2E tests. In the next few days, in the trunk it will be available an easy way to test hook with playwright: #10206 |
Thanks for this suggestion @gigitux! It was really helpful as I actually found two issues:
I'll raise an issue for 2., but 1. has to be fixed within this PR, so I'm switching this PR to draft until I fix it. |
…h is based on the block name
@gigitux I fixed both issues:
Commit: here
Commit: here
Regarding tests I'm planning to add them next week (Cooldown), but this documentation looks promising! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your great work! 🚀
I added two comments. One should be addressed. For the second one, I want to get your feedback about it.
Also, I noticed that with your changes, the woocommerce_before_shop_loop
and woocommerce_after_shop_loop
hooks are injected into the page even if any product hasn't been found. For the Products block, I fixed this in a dedicated PR: #9464. I think that the approach should be the same.
@gigitux, I didn't change that logic, though, this check is still in here. I double-checked in the
As you can see, the same hooked content appears on the page in each case. If you're saying that |
I created an issue for the problem described in this comment: #10452 EDIT: and the fix: #10453 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! LGTM! I just added a new comment about updating the comments on the AbstractTemplateCompatibility class (https://github.com/woocommerce/woocommerce-blocks/blob/6339f97e70b207a677f2f568c9b749c790eccc4c/src/Templates/AbstractTemplateCompatibility.php/#L99-L121).
BTW, I will pre-approve this PR! 🚀
Product Collection has to be compatible with the extensions and cover the same extensibility points as Products (Beta) block does.
In this PR Compatibility Layer is enabled for Product Collection block.
Fixes #10194 and #10378
Testing
Automated Tests
User Facing Testing
Core tests - include in the testing steps (fix for #10378)
Expected: The content is rendered above and below each product
Experimental tests - do not include in testing steps (fix for #10194)
Repeat for other hooks (script covering all of those actions at the bottom of PR for better readibility):
woocommerce_before_main_content
woocommerce_after_main_content
woocommerce_before_shop_loop_item_title
woocommerce_shop_loop_item_title
woocommerce_after_shop_loop_item_title
woocommerce_before_shop_loop_item
woocommerce_after_shop_loop_item
woocommerce_before_shop_loop
woocommerce_after_shop_loop
woocommerce_no_products_found
- to check this scenario, you can use Product Search and type some phrase that won't return any results (e.g. "asdfasdf").Double check these still work for Products (Beta) block by removing Product Collection and adding Products (Beta) block back.
WooCommerce Visibility
Appendix - script for testing steps
Changelog