-
Notifications
You must be signed in to change notification settings - Fork 219
Update the title and description for the Filter by Rating Block. #7905
Conversation
The release ZIP for this PR is accessible via:
|
Script Dependencies ReportThe
This comment was automatically generated by the |
TypeScript Errors ReportFiles with errors: 429 🎉 🎉 This PR does not introduce new TS errors. |
Size Change: -5 B (0%) Total Size: 975 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.
Thanks for creating this PR, @nefeline! I left a question regarding the block name change, which I think we should reconsider.
@@ -1,8 +1,8 @@ | |||
{ | |||
"name": "woocommerce/rating-filter", | |||
"version": "1.0.0", | |||
"title": "Filter by Rating Controls", |
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 use the Controls
suffix to differentiate the block with the controls from the wrapper block. Otherwise the structure is a bit confusing:
(the last one is the one that had the Controls
suffix)
@vivialice are we sure we want to go ahead with this change? Other filter blocks also have the Controls
suffix in the inner block to differentiate it from the wrapper:
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. |
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. |
Thanks for raising this point @Aljullu! I agree we should differentiate the naming for a couple of these. However, the term A couple questions:
Considering the List structure, this could be:
Thoughts? |
I don't think we have any other block, besides filter blocks, which has
We get that from Gutenberg: heading blocks are named as the text contained in them.
I think the last decision was not to go ahead this change yet. The reason is that the heading visibility depends on the filter status: if there are no available options, the filter (including the header) is not displayed. However, I acknowledge that's something that was discussed, so this might change in the future.
Sounds good to me. Considering we can't change the name the heading block has in the list, I understand the only change is that |
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. |
Since it looks like this is pending design input before we can determine if we plan to move forward, I'm going to move this back to the |
I'm going ahead and closing this PR since it has been inactive for a while: we can always circle back later if we decide to implement those changes cc: @jarekmorawski |
This PR updates the title and description for the Filter by Rating block as recommended by @vivialice on #7763 (comment)
Screenshots
Testing
User Facing Testing
WooCommerce Visibility
Changelog