2

I'm trying to do something I thought would be simple (!), but having trouble. Every time a user updates the form below, I want to update the cart total by multiplying the quantity (input values) by the data attribute unit-price.

I thought I would just need to loop through each input and grab the value and unit-price so I can multiply them together to create a total, but I'm the subtotal is returning '0'... any ideas of what I'm doing wrong?

JS FIDDLE

http://jsfiddle.net/wy5hy42x/

HTML

<form action="" id="form">
    <div class="cart-row">
        <p>Product name</p>
        <label for="updates">Quantity:</label>
        <input type="number" name="updates[]" value="2" min="0" data-unit-price="18.00" class="cart-variant--quantity_input"><br />
        &pound;18 each
    </div>

    <div class="cart-row">
        <p>Product name</p>
        <label for="updates">Quantity:</label>
        <input type="number" name="updates[]" value="4" min="0" data-unit-price="31.00" class="cart-variant--quantity_input"><br />
        &pound;31 each
    </div>

    <div class="cart-row">
        <p>Product name</p>
        <label for="updates">Quantity:</label>
        <input type="number" name="updates[]" value="4" min="0" data-unit-price="12.00" class="cart-variant--quantity_input"><br />
        &pound;12 each
    </div>
    <input type="submit" value="submit" />
</form>

JQUERY

// Update final price on quantity change
$('.cart-variant--quantity_input').on("change", function() {
  var st            = 0;
  $('.cart-row').each(function() {
    var i           = $('.cart-variant--quantity_input');
    var up          = $(i).data('unit-price');
    var q           = $(i).val();
    var st          = st + (up * q);
  });
  // Subtotal price
  alert('Cart updated. Subtotal : ' + st + 'GBP');
});
Osu
  • 1,105
  • 2
  • 17
  • 32
  • I'd highly recommend you place your `data-unit-price` attribute on the `.cart-row`. The unit price is irrellevant to the quantity, so placing it on the quanity input does not make sense. – Erik Philips Apr 07 '15 at 17:26
  • 1
    I strongly recommend to replace your inputs with a drop down list or perform a validation on what user enters. I can easily change my total balance to 0 by manually entering a negative number. – Makan Apr 07 '15 at 17:43
  • Thanks for the tips, both good ideas! – Osu Apr 07 '15 at 18:59

3 Answers3

3

You just need to remove var keyword from this line:

var st = st + (up * q);
// ^-- remove var in fron of st

otherwise you create a local variable inside a $.each callback and never update outer st value.

UPD. (credits to Dustin Hoffner for this catch) The second problem is that you need to find .cart-variant--quantity_input elements within current .cart-row container. For example, by providing a context for selector:

var i = $('.cart-variant--quantity_input', this);

All together it will become

$('.cart-variant--quantity_input').on("change", function () {
    var st = 0;
    $('.cart-row').each(function () {
        var i = $('.cart-variant--quantity_input', this);
        var up = $(i).data('unit-price');
        var q = $(i).val();
        st = st + (up * q);
    });
    // Subtotal price
    alert('Cart updated. Subtotal : ' + st + 'GBP');
});

Demo: http://jsfiddle.net/wy5hy42x/9/

dfsq
  • 182,609
  • 24
  • 222
  • 242
  • That's great, thanks for this and also to Dustin and user3906922 for their contributions, felt I learned something today! – Osu Apr 07 '15 at 19:00
2

First answer is not complete:

$('.cart-row').each(function(k) {
        var i           = $('.cart-variant--quantity_input')[k];
        var up          = parseFloat($(i).data('unit-price'));
        var q           = parseInt($(i).val());
        st          += (up * q);
      });

Edit: Changed parseInt to parseFloat (thanks to War10ck) (see his comment for the reason)

I removed the 'var' because you would overwrite in each loop.

Furthermore you will not calculate correctly:

var i           = $('.cart-variant--quantity_input');

This will always give you the first element. But you have more than one, so you need to get only the actual one from the returned array.

$('.cart-row').each(function(k) {
        var i           = $('.cart-variant--quantity_input')[k];

You can do this by taking the counter 'k' from your loop and get the right element with '[k]'.

Dustin Hoffner
  • 1,545
  • 1
  • 12
  • 15
  • 1
    I'd recommend that you change `parseInt()` to [`parseFloat()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/parseFloat). This would account for any prices that have cents. It would be highly unlikely that all prices would actually turn out to be whole numbers... – War10ck Apr 07 '15 at 18:18
  • Actually in this case `parseInt` is not needed, as multiplication operation concerts operands to Number type anyway. – dfsq Apr 07 '15 at 19:40
0

In your each loop you need to take the current input, instead of the first one:

$('.cart-row').each(function(e) {
    var i           = $('.cart-variant--quantity_input').eq(e); // get item in the e position
    var up          = $(i).data('unit-price');
    var q           = $(i).val();
    st          = st + (up * q);
});

Also you need to remove the var keyword before st, as its creating a new st variable instead of updating the outer one.

See this fiddle

eladcon
  • 5,685
  • 1
  • 11
  • 17