0

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?

12
  • which part doesn't work? and why not use for? Commented Nov 22, 2012 at 14:03
  • You want to generate the html with javascript ? Or the javascript is already here when you javascript is executed ? Commented Nov 22, 2012 at 14:04
  • It seems the concatenation in the javascript doesn't work. It just returns ".add_new_item_field" + i... Commented Nov 22, 2012 at 14:05
  • @Magus The HTML is in place. But I need to create a bunch of Javascript for each separate HTML item, so I was wondering if I couldn't fit this into a while loop... Commented Nov 22, 2012 at 14:06
  • 2
    By the time any click is executed, i will be 9999. So none of the click functions will find anything because they all try to find $(".add-new-item-field9999") Edit: lols off by one Commented Nov 22, 2012 at 14:07

4 Answers 4

1

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>
...
Sign up to request clarification or add additional context in comments.

7 Comments

Ah bummer, how would you fix this?
I have exactly what you have in your solution, but for some reason, the .add_new_item_button-div doesn't function properly (it doesn't do anything...)
@Michiel: Worksforme. Show us your code - what did you do different?
Ok, but what if I want to hide the content again when you click again? => ok, managed to do so. Still working out the kincks and I'll try to show you my code within minutes.
Ok, here is my code. Since it's a rather complex structure, I wasn't able to fit it in a jsFiddle. Here is a link, I hope this will suffice: chirowuustwezel.be/admin/index.php?page=verhuur
|
1

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...

6 Comments

And how would the while (i<9999) fit in this scenario?
@Michiel just replace your click listener with the one I posted above.
And this should be inside the while loop?
while looping is not the way.
@MikedeKlerk of course not, but I’m trying to explain why the code the OP posted doesn’t work as expected. I’m sure there are much better ways to solve the task. If the question was "how do I make this work", my answer would be different...
|
0

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.

Comments

-1

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

4 Comments

Do not use a while loop. Thats for the insane!
@MikedeKlerk The question is specifically about the while loop, not whether it’s insane or not. This answer doesn’t deserve a downvote because of someones opinion about a certiain kind of loops.
Someone else trying to achieve the same thing, adding a handler to a large number of objects, will not found the right way quickly if everybody is answering on the wrong phrased question. Instead of looking a bit further and working on a proper solution to accomplish what is needed.
@MikedeKlerk If you take the time to read the OP you will notice there is a while loop. My answer only refers to the original code and is not recommending while loops

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.