-
Notifications
You must be signed in to change notification settings - Fork 219
Add a Products by Rating filter block #7048
Conversation
…o try/add-rating-component
…o try/add-rating-component
…o try/add-rating-component
The release ZIP for this PR is accessible via:
|
Script Dependencies ReportThe
This comment was automatically generated by the |
Size Change: +12.9 kB (+1%) Total Size: 916 kB
ℹ️ View Unchanged
|
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.
This is working great, congrats for working on this big project, @danieldudzic! I did an initial review and left some minor issues below.
I also noticed the stars are not visible in the Site Editor:
I'm reporting it here but IMO this can be fixed in a follow-up PR. (Sorry if you had already identified it, maybe that's what you were referring to with Fix the editor preview display)
For this PR to be ready to merge, I think we need to:
- Add an experimental build gate (similar to this one and this one).
- I think it would be good to fix all the inline comments I left in this PR since they are minor things and affect translations.
- I also think it would be great to work on this item: Migrate the block from directly using the Rating component to utilizing the CheckboxList component. If I'm understanding this properly, this will simplify the code significantly.
IMO, all other To-do items you identified can then be worked on in follow-up issues/PRs. How does it sound?
"name": "woocommerce/rating-filter", | ||
"version": "1.0.0", | ||
"title": "Filter Products by Rating", | ||
"description": "Allow customers to filter the grid by products rating. Works in combination with the All Products block.", |
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.
We are removing the Works in combination...
part of the filter blocks description (see #7045).
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.
Sorry if you had already identified it, maybe that's what you were referring to with Fix the editor preview display
That's exactly it :)
Add an experimental build gate
I'll add it today
Migrate the block from directly using the Rating component to utilizing the CheckboxList component. If I'm understanding this properly, this will simplify the code significantly.
Yes, I think it would make a lot of sense. I'll work on that and see if this is something I can get done by the end of the week.
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 - otherwise it will automatically be closed after 10 days. |
@Aljullu Re:
I have just noticed the same issue occurs for the All Products block. Would you mind having a look? |
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 applying the changes and updating this PR, @danieldudzic!
I have just noticed the same issue occurs for the All Products block. Would you mind having a look?
You are right! Could you fill a separate issue for that? I don't think we should block this PR if this is affecting other blocks.
I left some inline comments below. 🙂
Issue created: #7167 |
…commerce-blocks into try/add-rating-component
Remove this logic when WordPress 6.1 is released.Remove this logic when WordPress 6.1 is released.
woocommerce-blocks/tests/e2e/config/setup.js Lines 33 to 44 in 986cf18
🚀 This comment was generated by the automations bot based on a
|
Remove runOnlyWhenGutenbergIsDisabled function and relati...Remove runOnlyWhenGutenbergIsDisabled function and relative workarounds when WordPress 6.1 is released.
woocommerce-blocks/tests/e2e/specs/backend/site-editing-templates.test.js Lines 132 to 143 in 986cf18
🚀 This comment was generated by the automations bot based on a
|
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 continuing improving this PR, @danieldudzic! I left a comment about the TS definition of displayedOptions
.
Besides that, I noticed that the placeholder title is strangely big compared to the other filter blocks:
Can you reproduce as well? I don't think this is blocking this PR to be merged, so IMO you can fix it on a follow-up PR so we don't block this one from getting into trunk
.
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'm able to reproduce the big placeholder, but it makes sense to create a follow-up issue 👍
I think that before merging this PR, it is necessary fixing the TS type error. (I suggested to you how we could fix the issue, let me know what you think!)
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.
LGTM!
Thanks for having applied the fix!
* Add interactivity to the Product by Rating filter block * Fix block with the latest repo changes * Product by Rating: Code tidying up * Add an experimental build gate and update block title and description * Remove redundant title and description * Add support for the CheckboxList component in the Products by Rating block * Products by Rating: Minor code clean-up * Active Filters: Fix the Clear All button for Ratings. Closes #woocommerce#7172 * Products by Rating: Add misc TS fixes
This is an attempt at adding a Products by Rating filter block.
For now, I have focused on the core functionality, and there are several pending items on the to-do list:
To-do
Rating
component to utilizing theCheckboxList
component (this will also make the UX match the preferred option ( pdnLyh-1sR-p2#comment-1044 ))Clear All
button doesn't work correctly (doesn't clear Rating filters on a Block Shop page)Rating
component to support other contexts (for example the All Products block)Screenshots
Testing
All Products
blockFilter Products by Rating
block andActive Filters
block on both pagesFilter Products by Rating
block is correctly:Active Filters
remove buttonUser Facing Testing
Same as the above steps
WooCommerce Visibility
Changelog