-
Notifications
You must be signed in to change notification settings - Fork 3.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update UX for quick order list mobile view #3733
Conversation
…e remove all if there are no variants in the cart
@@ -233,7 +233,7 @@ | |||
<div class="variant-remove-total"> | |||
{%- render 'loading-spinner' -%} | |||
<quick-order-list-remove-all-button | |||
class="no-js-hidden" |
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.
no-js-hidden
is a legacy class that used to hide this element if the user had js disabled. We don't support no-JS anymore since checkout requires JS so the class doesn't actually do anything.
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.
Works as expected, TY!
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.
Changes LGTM.
I did however, notice a different bug that I assume is unrelated. It looks like if I paginate to a page other than the first page, add quantities, and then "Remove all", the quantity switcher for that item still shows the stale quantity. Works correctly on the first page, though.
remove-all-bug.mp4
Yeah good catch, I dug into that some yesterday and found it's an API issue. Once the API is fixed we should double check that the theme code is still okay but that should fix it. |
…e remove all if there are no variants in the cart (Shopify#3733)
PR Summary:
Updates quick order list mobile view to hide the
remove all
button if the product has no variants in the cart, and display the loading spinner for pending requests.Why are these changes introduced?
Found this behavior while testing V15.3.0 release and it seemed unexpected, confirmed desired behavior with @Oliviammarcello from UX. From the existing code it looks like this was the original intent and probably just got missed at some point.
Note there is a layout shift when the button is shown/hidden. Olivia and I paired and determined the layout shift is preferable to maintaining the spacing, since (1) that makes it seem like the component padding is incorrect and (2) we already have a layout shift when going through the remove all flow already.
Before
https://screenshot.click/26-59-n5ou5-3gs0f.mp4
After
https://screenshot.click/26-57-n82jf-ggk7a.mp4
Demo links
Checklist