-
Notifications
You must be signed in to change notification settings - Fork 219
Make Filter Products by Stock block compatible with PHP rendered Classic Template block #6261
Conversation
…products-block into update/filter-product-by-attribute-php-template
…products-block into update/filter-product-by-attribute-php-template
…hub.com:woocommerce/woocommerce-gutenberg-products-block into add/6137-filter-product-by-stock
…products-block into add/6137-filter-product-by-stock
3b10e75
to
92c8b5d
Compare
Script Dependencies ReportThe
This comment was automatically generated by the |
1 similar comment
Script Dependencies ReportThe
This comment was automatically generated by the |
Size Change: +1.17 kB (0%) Total Size: 864 kB
ℹ️ View Unchanged
|
const url = currentQueryArgKeys.reduce( | ||
( currentUrl, queryArg ) => | ||
removeQueryArgs( currentUrl, queryArg ), |
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.
I updated this logic because with the previous implementation, ALL query parameters were removed, instead only the query parameters related to the attribute filter have to be removed.
Script Dependencies ReportThe
This comment was automatically generated by the |
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.
@gigitux The filter isn't working for me. I tested on the shop page, and the filter worked with the Active Filter block but the product grid didn't take the stock status into account. The Stock filter is still working with the All Products block as expected.
@dinhtungdu I have just fixed that in 38e3642 so should be working as expected. Please can you re-test this? |
Script Dependencies ReportThe
This comment was automatically generated by the |
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.
@tjcafferkey thanks for the update, this is working for me, the selected stock filter also appears in the Active filters block.
But when I enabled the Filter button, I got some issues:
- The page reloads immediately after I selected a filter. It should wait until I click the filter button.
- The selected filter doesn't appear in the Active Filters block anymore.
Screen.Recording.2022-04-20.at.10.02.39.mov
Good catch @dinhtungdu apologies that this was overlooked. I've addressed your feedback in this commit 03c607f and its now ready for another round of testing. |
Script Dependencies ReportThe
This comment was automatically generated by the |
Script Dependencies ReportThe
This comment was automatically generated by the |
Script Dependencies ReportThe
This comment was automatically generated by the |
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.
I left a minor comment but this is LGTM so pre-approving it.
Script Dependencies ReportThe
This comment was automatically generated by the |
This PR updates the Filter Products By Stock block to work with the Classic Template block which is PHP rendered, but also still maintains compatibility with the All Products block.
Filter Products by Stock block will only work with the PHP rendered Classic Template block if the filters are applied to the URL and the page is re-rendered. This behavior happens with the Classic Themes such as Storefront.
Fixes #6137
Technical details
I believe there is a lot of space to improve the code regarding all the blocks that filter the products.
Now, there are a lot of side effects, hard to debug, and these blocks are not performant (there are a lot of rerender :'( )
Also, As said by @dinhtungdu this approach is not really extendible when we will blockify this template.
My suggestion is to dedicate some time (in cooldown or in a dedicated project) to improve the quality of the codebase regarding the block that handles filters cc @Aljullu
Testing
Automated Tests
Manual Testing
How to test the changes in this Pull Request:
All Products
block, and theFilter Product by Stock
block and ensure no regressions have been introduced here.User Facing Testing
These are steps for user testing (where "user" is someone interacting with this change that is not editing any code).
Changelog