update cart example #57
Conversation
| <!-- .cart-tab start --> | ||
| <button class="btn btn--cart-tab"> | ||
| <div class="btn__counter"></div> | ||
| <svg xmlns="http://www.w3.org/2000/svg" class="icon-cart icon-cart--side" viewBox="0 0 25 25" enable-background="new 0 0 25 25"><g fill="#fff"><path d="M24.6 3.6c-.3-.4-.8-.6-1.3-.6h-18.4l-.1-.5c-.3-1.5-1.7-1.5-2.5-1.5h-1.3c-.6 0-1 .4-1 1s.4 1 1 1h1.8l3 13.6c.2 1.2 1.3 2.4 2.5 2.4h12.7c.6 0 1-.4 1-1s-.4-1-1-1h-12.7c-.2 0-.5-.4-.6-.8l-.2-1.2h12.6c1.3 0 2.3-1.4 2.5-2.4l2.4-7.4v-.2c.1-.5-.1-1-.4-1.4zm-4 8.5v.2c-.1.3-.4.8-.5.8h-13l-1.8-8.1h17.6l-2.3 7.1z"></path><circle cx="9" cy="22" r="2"></circle><circle cx="19" cy="22" r="2"></circle></g></svg> |
There was a problem hiding this comment.
Should this live in its own file maybe? Keep large blobs out of the index?
I don't feel strongly either way.
There was a problem hiding this comment.
It's such a small example that I think we should keep it all in the same place. Keeps the example as straight forward as possible
| if (cart.lineItems.length > 0) { | ||
| var totalItems = cart.lineItems.reduce(function(total, item) { | ||
| return total + item.quantity; | ||
| }, 0); |
There was a problem hiding this comment.
Made a note to move this work to the cart: #51
|
Couple small issues, then set sail! ⛵ |
|
Let's expand these out a bit now that the context is all available from being open source. @Shopify/channels-fed can one of you take a look at this? |
|
IE9 CORS currently predicated by: https://github.com/Shopify/shopify/pull/68257 |
| @@ -61,6 +61,14 @@ <h2 class="cart-title">Your cart</h2> | |||
| <!-- .cart end --> | |||
|
|
|||
|
|
|||
| <!-- .cart-tab start --> | |||
| <button class="btn btn--cart-tab"> | |||
| <div class="btn__counter"></div> | |||
There was a problem hiding this comment.
Technically only phrasing content can live inside a button, which means this would need to be a span instead of a div. I’ve never seen it cause an actual problem, but if switching to span with display: block doesn’t cause issues it may be safer.
|
looks good to me, though this crazy js-buy stuff is all new to me. |
harisaurus
force-pushed
the
cart-example-with-variants
branch
3 times, most recently
from
76df237
to
33f282b
Compare
|
@minasmart ready to go. @nwtn thanks for the review, homie |
| cart = newCart; | ||
| }); | ||
| var cartLineItemCount; | ||
| if(localStorage.lastCartId) { |
There was a problem hiding this comment.
always use getItem and setItem
|
👍 just that one thing about using |
harisaurus
force-pushed
the
cart-example-with-variants
branch
from
33f282b
to
6bcb2c9
Compare
|
Updated all |
| var $cartLineItems = cart.lineItems.map(function (lineItem, index) { | ||
| var $lineItemTemplate = $(lineItemEmptyTemplate); | ||
| var itemImage = lineItem.image.src; | ||
| $lineItemTemplate.find('.cart-item__img').css('background-image', 'url(' + itemImage + ')'); |
There was a problem hiding this comment.
are you avoiding using a templating system here for any reason?
There was a problem hiding this comment.
Just trying to keep the example as simple and small as possible. The focus of the example is meant to be the interactions with the SDK, so I didn't want to add in anything fancy with templating. I'm open to hearing other options though - whatever makes the example as simple and digestible as possible.
@richgilbank thoughts?
There was a problem hiding this comment.
just chatted with tyler in person - verdict was that it makes sense to not add an additional dependency for an example.
Updates cart example to add the following and to match SDK documentation:
please review: @minasmart