Cart error issue

[ludoboludo]

Why are these changes introduced?

Fixes #380

What approach did you take?

Two fixes in this PR:

• We have the cart notification on the cart page and we noticed that the form within the notification had the same ID as the form for the cart-items. Changing the one from the cart notification to make sure there wasn't a duplicate fixed a couple things: pressing Enter will submit the form and take you to checkout and when changing the quantity by highlighting the number and pressing Enter you will get an error message if you try a higher quantity than available.
• On safari specifically, when you changed the quantity in the cart, an error kept showing up (Fix add to cart error messaging #582 (comment)). It was due to how Mac handles focus management differently. It sends back focus to the body when clicking on an input that isn't of type="text". So I added a check in the if statement so it doesn't error anymore.

Other considerations

There isn't a fix for the situation where for example:

• you have 3 of the same item in your cart
• the have a stock limit of 10
• you manually input 11
• then you will notice that the quantity updates to 10 but no error is mentioned.

This to me seems like there could be some work done platform wise to handle cases like this. Otherwise we need to run lots of checks and logic.

Demo links

• Store

Checklist

• Followed theme code principles
• Linted with Theme Check
• Tested on mobile
• Tested on multiple browsers
• Tested for accessibility

[tyleralsbury]

Tested both fixes and it all looks good. This one was sneaky.

[tauthomas01]

Great stuff for finding the root cause 👍

[gregjotau]

@ludoboludo just added this to famme.no and the cart seems broken.

[ludoboludo]

@gregjotau could you provide some details on what's not working so I can see if I can replicate the issue locally ?

[gregjotau]

try adding to cart

[gregjotau]

I tried to revert, but it is still not working...

[gregjotau]

https://github.com/gregjotau/dawn-famme

The repo is here, there is not much customization @ludoboludo

[gregjotau]

product-form.js?v=3576239225032671514? [sm]:21 Uncaught TypeError: Cannot read properties of null (reading 'classList')
at HTMLElement.onSubmitHandler (product-form.js?v=3576239225032671514? [sm]:21)

[gregjotau]

We just sent out an "email blast" so a lot of lost customers now :(

[gregjotau]

@ludoboludo no product pages are working. Adding to cart simply does not work anymore.

[gregjotau]

@gregjotau could you provide some details on what's not working so I can see if I can replicate the issue locally ?

A video, this is critical as I cannot fix it even with a revert.

Screen.Recording.2021-09-17.at.23.07.10.mov

[ludoboludo]

Ah I think it's because there are some changes in other PR that you haven't added to your store. Take a look at this file and the PR that goes with it. Did you add that to your store as well ? #576 and specifically the main-product.liquid file.

@gregjotau

[gregjotau]

2 sec

[ludoboludo]

It changes how the .loading-overlay__spinner is. Now it's an element within the button rather than dealt with using a pseudo element.

[gregjotau]

yeah, have already cherry picked #576

[gregjotau]

but might not have merged it correctly

[gregjotau]

OK, spotted something in base.css

[gregjotau]

Nope, sorry, still does not work

[ludoboludo]

Yeah I see that on variant change it seem to remove the .loading-overlay__spinner element.

For now I'd suggest adding a conditional around

      this.querySelector('.loading-overlay__spinner').classList.remove('hidden');

So have it say:

if (this.querySelector('.loading-overlay__spinner')) this.querySelector('.loading-overlay__spinner').classList.remove('hidden');

[gregjotau]

2 sec

[gregjotau]

That seemed to work, but the spinner does not work for add nr 2, but OK. That should be fine for now.

[gregjotau]

OK, thanks for the quick help!

[gregjotau]

Have to sleep now. Keep up the great work 🤓

[ludoboludo]

Yeah something we will look into this first thing next week 👍 Seems like the variant change is removing the spinner. Then making the JS fail. Didn't come up from this PR but something we did recently that had this unexpected impact.

[gregjotau]

np! Have a great weekend :)