Jquery, .length + 1 seems to be adding multiples of 7

Question

I have a jquery script to add some input fields at the click of a button, I need these to be numbering up, so there are 4 fields called first:

JSFIDDLE LINK http://jsfiddle.net/1orkxk2y/

ptno1
desc1
qty1
amnt1

when add field is clicked I expected this to be but I ended up with the numbers in brackets:

ptno2(7)
desc2(7)
qty2(7)
amnt2(7)

Code below:

$(document).ready(function() {
    $("#add").click(function() {
        var intId = $("#test div").length + 1;
        var fieldWrapper = $("<div id=\"test\" class=\"row\"/>");
        var fptno = $("<div class=\"col-md-2\"><input type=\"text\" name=\"ptno" + intId + "\" class=\"form-control\" /></>");
        var fdesc = $("<div class=\"col-md-2\"><input type=\"text\" name=\"desc" + intId + "\" class=\"form-control\" /></>");
        var fqty = $("<div class=\"col-md-2\"><input type=\"text\" name=\"qty" + intId + "\" class=\"form-control\" /></>");
        var famnt = $("<div class=\"col-md-2\"><input type=\"text\" name=\"amnt" + intId + "\" class=\"form-control\" /></>");
        var removeButton = $("<div class=\"col-md-2\"><input type=\"button\" class=\"remove\" value=\"-\" /></>");
        removeButton.click(function() {
            $(this).parent().remove();
        });
        fieldWrapper.append(fptno);
        fieldWrapper.append(fdesc);
        fieldWrapper.append(fqty);
        fieldWrapper.append(famnt);
        fieldWrapper.append(removeButton);
        $("#buildyourform").append(fieldWrapper);
    });
});

<fieldset id="buildyourform">
</fieldset>
<input type="button" value="Add a charge" class="add" id="add" />

`.length` returns the number of `
` elements belonging to `#test`, + 1. Is this what you're intending? — Tim Lewis, Jul 25 '16 at 18:30
No I would prefer it to be the number of rows, so first set should have 1 and the second set 2 and so on — Lewis Smith, Jul 25 '16 at 18:32
@TimLewis are you sure about this ? ;) I think $('#test').length returns number of elements with that id — Marko Mackic, Jul 25 '16 at 18:32
@MarkoMackic Yeah, `$('#test').length` would, but would `$("#test div").length`? It's a different selector. Also, id's should be unique, so `$("#anything").length` should only **ever** return 1. — Tim Lewis, Jul 25 '16 at 18:34

Simon · Accepted Answer · 2017-10-11T18:09:26.440

3

The expression $("#test div") will get every div inside every #test. Since you add 5 div inside #test each time, the length attribute will increment by 5. Perhaps you could simply use a global variable instead of reading the DOM each time?

var intId = 1;

$(document).ready(function() {
    $("#add").click(function() {
        var fieldWrapper = $("<div id=\"test\" class=\"row\"/>");
        var fptno = $("<div class=\"col-md-2\"><input type=\"text\" name=\"ptno" + intId + "\" class=\"form-control\" /></>");
        ...
        intId++;
    });
});

Edit: Thanks Tim Lewis for pointing out that multiple id attributes produces invalid HTML.

edited Oct 11 '17 at 18:09

answered Jul 25 '16 at 18:33

Simon

774
4
21

3

And I'll say it again, make sure not to repeat `id` attributes, they need to be unique, so having multiple `
` elements is invalid.
– Tim Lewis Jul 25 '16 at 18:39
Just posted an answer that doesn't use duplicate ids, for reference – Jonathan Michalik Jul 25 '16 at 18:44

score 0 · Answer 2 · edited May 23 '17 at 10:28

First of, when you're appending to something make sure that you close the element tag, and secondly you can't query $('#test div') because it will query the divs inside the id named test, no matter the level. So your code should be :

$(document).ready(function() {
    function reIndex(){
      $('.test').each(function(index,element){    
         $(element).children().each(function(){    
           var inp_elem = $(this).children("input");
           if(inp_elem.attr("name").split("_").length == 2)
             {
              inp_elem.attr("name",inp_elem.attr("name").split("_")[0] + "_" + (index+1));
             }
      });
    });
    }
    $("#add").click(function() {
        var intId = $(".test").length + 1;
        var fieldWrapper = $("<div class=\"test row\"/></div>");
        var fptno = $("<div class=\"col-md-2\"><input type=\"text\" name=\"ptno_" + intId + "\" class=\"form-control\" /></div>");
        var fdesc = $("<div class=\"col-md-2\"><input type=\"text\" name=\"desc_" + intId + "\" class=\"form-control\" /></div>");
        var fqty = $("<div class=\"col-md-2\"><input type=\"text\" name=\"qty_" + intId + "\" class=\"form-control\" /></div>");
        var famnt = $("<div class=\"col-md-2\"><input type=\"text\" name=\"amnt_" + intId + "\" class=\"form-control\" /></div>");
        var removeButton = $("<div class=\"col-md-2\"><input type=\"button\" class=\"remove\" value=\"-\" /></div>");
        removeButton.click(function() {
            $(this).parent().remove();
            reIndex();
        });
        fieldWrapper.append(fptno);
        fieldWrapper.append(fdesc);
        fieldWrapper.append(fqty);
        fieldWrapper.append(famnt);
        fieldWrapper.append(removeButton);
        $("#buildyourform").append(fieldWrapper);
    });
});

I missed a thing Does ID have to be unique in the whole page? ID of element has to be unique because it gives an element a name, so you could use it with a class test. EDIT: I added reindexing when element is removed, so the indexing would be consistent and no overlaps will occur :) and here is a new fiddle http://jsfiddle.net/1orkxk2y/3/

That looks better; Only issue I can see is if you have 3 elements `
` and you remove number 2, then create another one, you all of a sudden have multiple `` elements :P — Tim Lewis, Jul 25 '16 at 18:51
Well yeah :P correct answer doesn't address this issue too but I could do reindexing :P — Marko Mackic, Jul 25 '16 at 18:52
Actually Simon's does, since it uses an internal counter, so you'll never end up with the same `intID` twice. Just needs a bit more refactoring lol There's other solutions, like assigning the input names/index after appending/removing, but that's the fun of JS; so many solutions to the same problem — Tim Lewis, Jul 25 '16 at 18:53
yeah, I thought you were talking about consistence of numbering — Marko Mackic, Jul 25 '16 at 18:56

score 0 · Answer 3 · answered Jul 25 '16 at 18:42

$("#test div").length is returning the number of div elements that have a parent with an id of test. Currently, you are generating a new container for these inputs, all with an id of test, so they all meet the criteria for the selector.

I'd like to point out, first, that creating multiple elements with the same id is invalid HTML. Something you can do to address the iterator and duplicate id problems in one swoop is to use the row class that you're already placing on your input containers:

$("#add").click(function() {
        var intId = $(".row").length + 1;
        var fieldWrapper = $("<div class=\"row\"/>");
        var fptno = $("<div class=\"col-md-2\"><input type=\"text\" name=\"ptno" + intId + "\" class=\"form-control\" /></>");
        var fdesc = $("<div class=\"col-md-2\"><input type=\"text\" name=\"desc" + intId + "\" class=\"form-control\" /></>");
        ...

No global iterator, just utilizing classes you're already using.

Jquery, .length + 1 seems to be adding multiples of 7

3 Answers3