Skip to content
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

Merged
merged 1 commit into from
Feb 26, 2025

Conversation

lhoffbeck
Copy link
Contributor

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

…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"
Copy link
Contributor Author

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.

@LA1CH3 LA1CH3 self-requested a review February 26, 2025 15:04
Copy link
Contributor

@Oliviammarcello Oliviammarcello left a 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!

Copy link

@LA1CH3 LA1CH3 left a 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

@lhoffbeck
Copy link
Contributor Author

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.

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.

@lhoffbeck lhoffbeck merged commit 4225095 into main Feb 26, 2025
8 checks passed
@lhoffbeck lhoffbeck deleted the lh-fix-quick-order-list-mobile-ux branch February 26, 2025 15:36
NathanScheele pushed a commit to Misha-Puff/dawn that referenced this pull request Mar 3, 2025
…e remove all if there are no variants in the cart (Shopify#3733)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants