Add 'Go' button to Attribute Filter #1332
Conversation
Aljullu
added
status: needs review
type: enhancement
mikejolley
left a comment
mikejolley
left a comment
There was a problem hiding this comment.
Looks good 👍 Tested well too.
Added some small items of feedback but pre-approving.
| if ( ! blockAttributes.showFilterButton ) { | ||
| onSubmit(); | ||
| } | ||
| }, [ checked, onSubmit ] ); |
There was a problem hiding this comment.
Does blockAttributes.showFilterButton need to be a dependency here?
There was a problem hiding this comment.
Good point! I think it's not needed because blockAttributes.showFilterButton can only change in the editor and onSubmit should only be triggered in the frontend.
But that makes me thing that when eluding dependencies we must always explain it in a comment so it doesn't look like a bug. Added in 909a4f9.
| help={ () => { | ||
| if ( showFilterButton ) { | ||
| return __( | ||
| 'Results will only update when the button is pressed.', |
There was a problem hiding this comment.
Maybe "products" would be clearer than "results" here.
| } | ||
| return displayStyle === 'list' | ||
| ? __( | ||
| 'Results will update when the options are checked.', |
There was a problem hiding this comment.
We could probably use the same string here for both, maybe "Products will update as options are selected".
Fixes #1255 (comment). I also refactored
/attribute-filter/block.jsa bit because after the changes it was a bit too long so I tried to move some parts to other files.Screenshots
How to test the changes in this Pull Request:
Changelog