Add events to notify of drawer opening / closing#481
Conversation
|
I agree this is a great approach. Are you able to update the docs to include these as well? |
|
Yes, would be happy to do it. |
|
I added more triggers and fixed the namespace. I've implemented this in a few projects and it's been working great. Let me know what we can do to push this forward, I'm happy to help with anything needed. Have a beautiful weekend. Peace ;) |
|
Awesome. This is exactly what I was talking about a little while back. Thanks heaps @humancopy 😃 |
assets/ajax-cart.js.liquid
Outdated
| ==============================================================================*/ | ||
| ShopifyAPI.addItemFromForm = function(form, callback, errorCallback) { | ||
| var params = { | ||
| var $body = $('body'), |
There was a problem hiding this comment.
A bit of a nitpick, but can you change this to $(document.body) - it's slightly faster (source).
There was a problem hiding this comment.
Done, changed it in a couple of other places also ;)
| cartCallback = function(cart) { | ||
| $body.removeClass('drawer--is-loading'); | ||
| $body.trigger('ajaxCart.afterCartLoad', cart); | ||
| $body.trigger('afterCartLoad.ajaxCart', cart); |
There was a problem hiding this comment.
Can you revert this name. It's the only breaking change I can see in the PR
There was a problem hiding this comment.
Scratch that. I'll update the version number differently instead. All good
assets/ajax-cart.js.liquid
Outdated
| else { | ||
| ShopifyAPI.onCartUpdate(cart); | ||
| } | ||
| $(document.body).trigger('afterGotCart.ajaxCart', cart); |
There was a problem hiding this comment.
Last change, switch this to afterGetCart. Realizing the naming issues as I'm writing up some new docs
There was a problem hiding this comment.
No worries, done... Let me know if need any help with docs. :)
assets/ajax-cart.js.liquid
Outdated
|
|
||
| // If it has a data-line, update the cart. | ||
| // Otherwise, just update the input's number | ||
| $body.trigger('adjustCart.ajaxCart', [this, line, qty]); |
There was a problem hiding this comment.
Can remove this. updateQuantity (the function right after this) calls ShopifyAPI.changeItem, which already has triggers fired (before/after/error/complete ChangeItem)
There was a problem hiding this comment.
updateQuantity is not always getting triggered which is why I put this one here
|
Alright, we're almost there. Let's remove:
|
|
Removed redundant events. Docs look really good, well done! :) |
|
Thanks for the updates. Did a bunch of testing and it looks in order. The only duplicate events triggered are Will give the docs a quick review the merge both. Cheers to putting this together. |
|
Last ask, could you squash the commits down to one. Here are the steps. |
|
Unfortunately I can't squash because I merged stuff from the Shopify/Timber as I needed to keep it up to date cause I was using it for some projects, sorry. If you have an idea how to resolve this I'm happy to follow... ;) |
|
No worries, more of a nice to have than anything. |
3 participants
Hey :)
I think it's a good idea to start using events in Timber instead of callbacks to make it easy to tap into interface changes. I also want to add events to the ajax_cart.js file, what you think?