bornfight-studio / goCart.js

Complete Shopify Ajax cart solution written in vanilla JS
MIT License
162 stars 56 forks source link

Not working with product variants #8

Open bigskillet opened 5 years ago

bigskillet commented 5 years ago

The script is working on products with one variant, but products with multiple variants like sizes, are not added to the drawer. A few things I've noticed:

  1. The <form> tag provided in the documentation works for single variant products, but also kills my variant options. Switching the id back to product_form_{{ product.id }} fixes the issue with the options. Regardless of the

    tag I use, it yields the same results: it will add single variant products to the drawer/cart, but not multi-variant.

  2. When I remove the js-go-cart-add-to-cart class from the submit button, then add the variant product to the cart normally, the product will show up in the drawer when I click the cart icon. The drawer will show all products in the cart, but add-to-cart will only add single variant products. It looks like something might be going wrong with the add-to-cart script when triggered?

I have this implemented on a client site and would love to get this resolved. For now I'm using a condition to add/remove the add-to-cart class depending on if there are multiple variants or not. {% if product.has_only_default_variant %}js-go-cart-add-to-cart{% endif %}

Regardless of the issues, I'm loving the script so far! Thanks!

marioloncarek commented 5 years ago

thank you for your input, we will look at this issue asap

marioloncarek commented 5 years ago

@bigskillet could you confirm that you are not using Slate as a core for a theme? I just checked on our demo store and everything is working on Slate

bigskillet commented 5 years ago

@marioloncarek I am using Slate. Let me test with a fresh Slate theme and get back to you. I may have jumped the gun on this issue.

marioloncarek commented 5 years ago

@bigskillet no problem, here is a product with two variants on our demo shop with package installed with npm and with fresh Slate: https://ajax-cart-plugin-npm-test.myshopify.com/products/classic-varsity-top

Let me know if you need assistance with installation so we can improve the docs

bigskillet commented 5 years ago

@marioloncarek dude, i created a new slate theme, then followed your steps exactly, but it quit working the minute i added a size variant to the product. what am i doing wrong? https://bigskillet-slate.myshopify.com/products/sample-product

marioloncarek commented 5 years ago

@bigskillet Hello friend and sorry for a late response. Please check your console (always). This has nothing to do with GoCart.js . Slate is causing the problems for you. I now installed the latest slate and have the same problem. I will let you know how to fix slate

marioloncarek commented 5 years ago

@bigskillet Hello again. So about the slate, there is a bug in the starter-theme when products have no variants, you can read about it here: https://github.com/Shopify/starter-theme/issues/128

That being said i agree that goCart is not working in this example. Let me explain why.

GoCart relays on on changing the hidden variant selectbox inside the product form. When working with options and adding to cart, form is serialised to get unique ID of product with selected variants. But visible option elements are not the one that are being serialised, but the hidden select box containing all the variant combinations with unique IDs. So what happens in the background when you choose the variant is that actually Javascript is listening to change event for the option form elements and then finds the combination inside the hidden input, which is then serialised. But the newest starter-theme is not listening to change event, it does this when you press add to cart button right before sending the form. We are preventing that in goCart and then you actually send the first combination from hidden select because it never changed on option change.

Thank you very much for pointing this out, im very sorry that this happened, i tested goCart on older starter-theme. Please give us some time to fix this, i will contact you when its done.

bigskillet commented 5 years ago

Ah yes, that reminds me that I forgot to add the single variant fix:

{% else %}
      {% for variant in product.variants %}
        <input type="hidden" name="id" value="{{ variant.id }}">
      {% endfor %}
{% endunless %}

But yeah, that doesn't seem to fix the issue. I really hope Shopify starts working on a production version of Slate again soon, because there has been bugs galore lately. Most recently the cookies issue.

Thank you Mario! I look forward to seeing your fix.

jmd-wilsons commented 4 years ago

Hi there,

I'm just wondering if there was ever a fix for this?

Many thanks,

marioloncarek commented 4 years ago

I will try to find some time next week to address this. Thanks for patience

Jupitercouple29 commented 4 years ago

Hi, this looks very awesome. Did you have a chance to fix this?

Thanks in advance.

cfxd commented 3 years ago

This is not an issue with goCart.js. If you're using Slate v1 with https://github.com/Shopify/starter-theme then you need to update your theme a bit. I have a PR with all the changes you'll need to make right here. I was able to reproduce the error and my fix solves it. Give it a go and lmk if you get stuck!

@bigskillet @Jupitercouple29 @marioloncarek

marioloncarek commented 3 years ago

@cfxd really thanks for for work you are doing. Im actually thinking about rewriting the goCart.js from the beginning and move away from theme being dependency for variants to work. That would mean that goCart would provide the whole product form and had custom events for handing variations and updating hidden select (like slate v1). If you have any idea or feature you think would be great i would be more than happy to share some emails with you.

cfxd commented 3 years ago

@marioloncarek Sure, I'd love to help any way that I can and be involved!

cfxd commented 3 years ago

Hi @chris-charle , are you able to deploy your Slate theme and post a link (and password if it's protected)?

cfxd commented 3 years ago

Can you please also post some steps to reproduce? I'm not seeing the issue.

Also, I can't see your commit history so I'm not clear what you mean when you refer to the commit history.

One issue that I do see is multiple form elements with name="id" and that's not ideal.

cfxd commented 3 years ago

Steps to reproduce the issues, please.

ghost commented 3 years ago

@cfxd ok looks like with some digging I have sorted it... but one issue is if the button lives inside a div (in this case prd-Product_Buy) then it won't work can this be fixed? See below:

Errors doesnt-work

Works does-work

fowlercraig commented 3 years ago

@cfxd running into the same problem on my end -- when the submit button is wrapped in a div, it errors out.

ghost commented 3 years ago

@fowlercraig this is really frustrating have to position the button outside clean html and css

marioloncarek commented 3 years ago

this is the part of the code which causes button could not be wrapped: https://github.com/bornfight/goCart.js/blob/master/src/lib/GoCart.js#L95 problem is in the way this event works, but on the other hand it fixes having dynamically created add to cart button (like with ajax) this is the PR which i merged so you can check reasoning behind: https://github.com/bornfight/goCart.js/pull/15

Since i really dont have time for this right now, as you can all see a lot of comments and issues are not being addressed, if anyone has the fix for this problem and can create a PR, i will merge it in the same day, thats a promise.

marioloncarek commented 3 years ago

@chris-charle , @fowlercraig , @cfxd

cfxd commented 3 years ago

Can anyone please share a live link to a shop with this issue (and passwd) and steps to reproduce? @fowlercraig @chris-charle @widding

I do recall having this problem but I don't remember how I solved it, seeing it live again may remind me.

cfxd commented 3 years ago

...I may have done something like this:

let form = event.target.closest('form');

To replace lines 98-102 @ https://github.com/bornfight/goCart.js/blob/master/src/lib/GoCart.js#L98

dillonlara115 commented 2 years ago

has anyone gotten a workaround figured out for this yet? We're using the new dawn theme and everything is working except for this.

cfxd commented 2 years ago

@dillonlara115 See #1407

dillonlara115 commented 2 years ago

@dillonlara115 See #1407

So its just an issue with Dawn and I just need to update or can you elaborate by chance?

cfxd commented 2 years ago

@dillonlara115 Technically it's a "feature" and not an issue, but it's being worked on in that PR. If you view the Files tab you can see what changes and where. You could pull down that PR and use it, or make the changes to your theme yourself, but it's not officially part of Dawn just yet. It will hopefully be part of Dawn soon, although it's not a perfect solution and it still doesn't cover every use case.