Michiel
Michiel

Reputation: 8093

while loop in Javascript with concatenation

I'm trying to make a while-loop in my Javascript.

So far, I've got this:

<script>
    $(document).ready(function(){       
        var i=0;
        while (i<9999) {
            $(".add_new_item_field" + i).hide();    
        $(".add_new_item_button" + i).click(function(){
            $(".add_new_item_field" + i).slideToggle("slow");
        });
        i++;
        }
    });
</script>

Goal is to make this work:

<div class="add_new_item_button1"></div>
<div class="add_new_item_button2"></div>
<div class="add_new_item_button3"></div>
...
<div class="add_new_item_field1">Show me something</div>
<div class="add_new_item_field2">Show me something</div>
<div class="add_new_item_field3">Show me something</div>
...

But for some reason, it's not working. Am I missing something here?

Upvotes: 0

Views: 415

Answers (4)

Bergi
Bergi

Reputation: 664599

Your problem is that the concatenation in the line $(".add_new_item_field" + i).slideToggle("slow"); happens when you click one of the divs. Yet, the loop that had set up the handlers was run long ago then and i already has a value of 9999. Use a closure as @David demonstrated to avoid this.

However, I feel this is the wrong approach. Setting up 10000 click handlers, and executing 20000 jQuery selection does make your page very, very slow. Use one common class for the button, and one common class for the fields. If you can't depend on a certain document order, give them unique ids to refer to each other - but not classes.

Then hide all the fields with one single line of CSS, and use event delegation for the buttons to fire 1 single function that looks up the field by id from the data attached to the clicked button.

<style>
    .add_new_item_field { display:none; }
</style>
<!-- placing the stylesheet here also avoids flickering.
Even better would be of course if it was written dynamically by JS, for not 
hiding the fields in clients that do not support JavaScript -->
<script src=/"jquery.js"></script>
<script>
jQuery(function($) {
    $(document).on("click", ".add_new_item_button", function(e) {
        var id = "field"+$(this).data("field");
        $('#'+id).show();
    });
});
</script>
<div class="add_new_item_button" data-field="1"></div>
<div class="add_new_item_button" data-field="2"></div>
<div class="add_new_item_button" data-field="3"></div>
...
<div class="add_new_item_field" id="field1">Show me something</div>
<div class="add_new_item_field" id="field2">Show me something</div>
<div class="add_new_item_field" id="field3">Show me something</div>
...

Upvotes: 1

David Hellsing
David Hellsing

Reputation: 108500

I’m guessing that i is not what you expect it to be in the handler, because when the handler executes i has already maxed out to 9999. To fix that, you need to bring the variable into the handler closure, something like:

var i=0;
while (i<9999) {
    $(".add_new_item_field" + i).hide();  
    $(".add_new_item_button" + i).click((function(i) {
        // i is now saved in this closure
        return function() {
            $(".add_new_item_field" + i).slideToggle("slow");
        };
    }(i)));
    i++;
}

Sidenote: I’m not really sure this is the best way to solve your actual task here though, looping and attaching 9999 event handlers seems unnecessary...

Upvotes: 1

Stephen O&#39;Connor
Stephen O&#39;Connor

Reputation: 1475

You are creating a closure in your click handler which captures the i variable, not it's value. This means all the click functions will contain the value 9999 which the value of the variable at the end of the while loop.

You can fix it by creating a function that set's the click handler.

var setClick = function(index) {
   $(".add_new_item_button" + index).click(function(){
   $(".add_new_item_field" + index).slideToggle("slow");
   }
}

And use it in your while loop.

while (i<9999) {
    $(".add_new_item_field" + i).hide();    
    setClick(i);
    i++;
    }

Now the value of i is correctly captured by your click handler

Upvotes: -1

Mike de Klerk
Mike de Klerk

Reputation: 12328

I think you are using the class the wrong way. You can assign a click handler to all objects of the same class. But you are trying to use the class specifier as an ID and are trying to assign the handler to each seperate object. You wouldn't do the same for the behavior and layout of a link/url? (link) would you?

Read this: jQuery: How do I add a click handler to a class and find out which element was clicked?

You want to setup the handler for a class, specify your divs as that class. I not abusing the class specifier as some sort of ID.

Upvotes: 0

Related Questions