1

Background

I'm not the hottest jQuery guy out there by a very long shot and I'm trying to strip out the repeated work in the below code. Whilst the performance overhead is probably minimal and negligible - this is more a case of not wanting to write crappy code that does the same thing several times.

Basically I have a simple invoice form, that a user can add multiple items too.

The form initially has 4 inputs: Item Name, Item Price, Item Quantity and Total

  • The total is calculated whenever the Price or Quantity field fires a change event

Problem - Partially resolved (See Update)

The user can add an additional row of inputs for a second (or third, fourth, fifth, etc...) item

  • The existing javascript which attaches the event handler to the price and quantity fields has already run so will not attach listeners to the newly added row of inputs.

Solution ?

Currently I've hashed out something horrible whereby after adding the new row of inputs I re-attach an event listener to all input fields.

That's cool I guess, if you take no pride in the quality of your work, but if the invoice is 20 items do I really need to on adding the 20th item row loop over the 19 rows that already have listeners attached, attach them again and then attach listeners to the new row. I would hope not.

I've already managed to target the newly added row of inputs to wipe the values from the cloned inputs - so I'm thinking just target the new input fields and attach listeners - but I'm getting in a right two and eight because ideally I'd like to do this like so

  1. Clone the input row
  2. Clear the values
  3. Attach listeners
  4. Add to the DOM

What I'm currently doing which feels grotesque is

  1. Clone the row
  2. Add the row to the DOM
  3. Select the newly added row and wipe the values
  4. Select the newly added Quantity field and attach a listener
  5. Select the newly added Price field and attach a listener
  6. Select the newly added Total field and attach a listener (to update the invoice total)

Code below, for you to laugh at and then hopefully take pity on me and provide a more succinct solution or at least a suggestion as to how to go about writing my own better version.

/** Add additional item lines */
$('#add-item').click(function(e){
    e.preventDefault();
    /** clone first line and insert it */
    $('.input-row:first').clone().insertAfter('.input-row:last');
    /** clear the newly inserted inputs of values */
    $(':input', '.input-row:last').val("");
    /** ensure all item price and qty inputs have events attached to their change value */
    $('input[name="item_qty[]"],input[name="item_price[]"]').on("change",function () {
        var $container = $(this).closest('.form-group');
        qty = Number($('input[name="item_qty[]"]',$container).val())||0,
        price = Number($('input[name="item_price[]"]',$container).val())||0;
        $('input[name="item_total[]"]',$container).val(qty * price);
        $('input[name="item_total[]"]',$container).change();
    });
    /** Sum inputs for invoice total */
    $('input[name="item_total[]"').change(function() {
        var total = 0;
        $.each($("[name='item_total[]']"), function(index, value) {
            total += parseFloat($(this).val());
        });
        $("#total").val(total);
    });
});

Update

So by utilising event delegation, events propagate (or bubble) up the dom - thanks guys! I've got the invoice total being recalculated any time one of the inputs within the new parent div change

<div id="invoice-items">
    <input name /> <input quantity /> <input price /> <input total />
    <input name /> <input quantity /> <input price /> <input total />
    <input name /> <input quantity /> <input price /> <input total />
    ...
</div>

/** if any input rows change update the invoice total */
$('#invoice-items').on('change', 'input', function(event){
    var total = 0;
    $.each($("[name='item_total[]']"), function(index, value) {
        total += parseFloat($(this).val());
    });
    $("#total").val(total);
});

Problem I'm left with...

I'm still stuck on how I go about updating <input total /> to reflect the changes to that particular line. I'm guessing somewhere in my new jQuery snippet I need to determine which field changed and update the total on the same row ?

This is how I'm currently attaching the change listeners to the first / existing row of input to populate the line total

/** calculate item total */
  $('input[name="item_qty[]"],input[name="item_price[]"]').on("change", function () {
    var $container = $(this).closest('.form-group');
    qty = Number($('input[name="item_qty[]"]',$container).val())||0,
    price = Number($('input[name="item_price[]"]',$container).val())||0;

    $('input[name="item_total[]"]',$container).val(qty * price);
    $('input[name="item_total[]"]',$container).change();
  });

I guess what I still need is some means to run this code after a line has been added, or following the cleaner event delegation route - some way to target just the item_total[] for the row in which the change event happens ? Maybe I can capture the specific index of the element on which the change event is fired - and update only the item_total[] at that index ?

Just thinking out loud here, I guess if I capture the event and loop through all of the inputs til I find that element which matches the element the event was fired from I could then grab the next form input with the name invoice_total[] and update it ? - let's go check.

Update

So I can capture the event - happy days :)

event.currentTarget.attributes.name.nodeValue == 'item_qty[]'

So I still don't know which of the item_qty[] elements I've updated and therefore I don't know which item_total[] element to update.

Any suggestions guys ?!?

Community
  • 1
  • 1
Luke
  • 3,481
  • 6
  • 39
  • 63
  • 1
    you want to attach event handlers to cloned rows. tahts the point? – A.B Oct 26 '14 at 21:19
  • 1
    You're right to be looking for a better way, and it comes in the form of **event delegation**. [Read up](http://stackoverflow.com/questions/8110934/direct-vs-delegated-jquery-on) on it – it lets you attach the event handler to just one parent element, which will handle `change` events for e.g. every child `input` element (whether they were always there or are newly added). – Shai Oct 26 '14 at 21:25
  • I've just commented on the `event delegation` answer - if I can attach a listener to all inputs in the div - great but how do i then target specific inputs to populate them with the value. This way will work great for my invoice total - cause if anything changes that needs to update but what about for the individual line items ? do I wrap each row in a div and target that div for changes ? – Luke Oct 26 '14 at 21:29
  • OK so each row of inputs is now contained in a single parent div - which I've attached a listener to. Sweet! – Luke Oct 26 '14 at 21:36

1 Answers1

3

You want to take the wrapping element

<div>
    <input />
    <input />
</div>

$('div').on('change', 'input', function(){
    // your magic here
});

This will work on the two who are there now, and new elements as well. Why? Simply put:
You bind the events to elements that exists. You make a new one, change it, but never bound the event to the new elements. The event bubbles up the tree, nothing to catch it.

My code doest bind to the elements itself, your telling it to listen to changes in it on input. New elements come in, you change them, nothing happends, so it bubbles up the tree.
And this is the big difference: this time we told the existing element to do something now.

Martijn
  • 15,791
  • 4
  • 36
  • 68
  • I'm going to try this out first just to be sure - but I suspect I'm going to have issues - e.g. `when qty[0] changes how do i make total[0] == qty[0] * price[0]` – Luke Oct 26 '14 at 21:27
  • Ok so it works great to watch all inputs and update the invoice total when any of them change - I've updated my question in the hopes someone can suggest a means to update each row when that particular row changes. – Luke Oct 26 '14 at 21:45