-
Notifications
You must be signed in to change notification settings - Fork 219
Filter by Attribute
block design updates
#6920
Conversation
The release ZIP for this PR is accessible via:
|
Size Change: +1.14 kB (0%) Total Size: 870 kB
ℹ️ View Unchanged
|
f286363
to
6732bcf
Compare
14af209
to
310caf7
Compare
Hey @albarin just a few things Ive noticed:
|
Hey Alba! This is looking really great 🤩 I have made a few notes below: For both, can we disable the Dropdown: I noticed the dropdown arrow is missing - are we able to add that in? After hitting Are we able to match the chip styles in List: I will do another review of these tomorrow, but that's what stuck out to me so far! |
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.
@albarin This is looking great! The block tested well on my end. However, I found a scenario where the behavior of the Reset button can be improved, please follow these steps to reproduce:
- Add the attribute filter block to the shop page.
- Enable the filter button.
- On the front end, select an attribute.
- Click on the Reset button, and see the page reloads.
The page doesn't reload when I do the same for Stock and Price filter blocks. So I'm wondering if we can do the same for the attribute filter block. I think we shouldn't reload the page when clicking the Reset button if the filter isn't applied.
I took another look on my test site and I have a couple of other thoughts:
|
Good catch! I just pushed a fix :) |
Thanks guys! I just had a review and it's looking great.
Hmm maybe. Do we have an ETA for this issue? How difficult of a change is it?
👍
Yes that is why I originally placed the button and links in the overlay. Keep me posted if you are able to find a solution for this. |
What about something like this @vivialice 👇 |
@tjcafferkey or @dinhtungdu do you have any news regarding the issue to remove the headings #6845 ? 🤔 |
It's a good attempt but it's too inconsistent from the other blocks, so I don't think we should do this. |
We're currently exploring a few approaches here. Im not sure on the ETA but it feels we may be close to an approach we can start to implement with a HOC. It would also be good if @vivialice can take a look at the video in the PR too and post thoughts on it there. |
@tjcafferkey @dinhtungdu @vivialice I think the only thing missing here is to decide what to do with the Reset/Apply buttons when the |
I think we can tackle this separately and it shouldn't block this PR, but happy to go with the majority vote on this. |
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.
@albarin I was playing around with this PR and I noticed a z-index
issue when two dropdowns overlap, can you reproduce as well?
And one question about the design: when only a single item can be selected, could we remove the pill border around? It feels a bit strange.
That pill shouldn't be full width, what is happening here? |
Totally missed testing the behavior for single item! It's fixed now :)
Can someone give me a hand with this z-index? I've been trying but haven't managed to fix it so far 😞 Thanks for the review and the patience @Aljullu @vivialice, could you give another try to the single selection issue? |
on it 😄 |
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.
It looks like the remaining issues raised by @vivialice and @Aljullu have been fixed. The only outstanding issue is the z-index
one spotted by Albert, however we've decided to merge this PR and tackle this final issue independently. Good job on this PR @albarin !
* Add and style Reset and Apply buttons * Style the dropdown option * Fix class and label * Fix checkbox margin in editor * Fix alignment * Fix the reset button * Disable Apply button when no changes have been made to the filter to improve UX * Reduce vertical spacing between list items * Style chips * Don't reload on reset if filters have not been applied * Fix heading styles on the front end * Add chevron down to filter by attribute dropdown ui component * Force FormTokenField to remount on reset * Fix dropdown z-index * Reduce apply button margin * Fix styles for single token selection Co-authored-by: tjcafferkey <[email protected]>
Update the
Filter by Attribute
block UI to match the new designs.Fixes #6843
Screenshots
Testing
Automated Tests
User Facing Testing
Filter by Attribute
block to a page.Reset
button appears when a checkbox is checked.WooCommerce Visibility
Changelog