Click function inside for loop

Question

I am trying to loop my .hide() and .show() click handlers and I can't get this simple part to work correctly. My buttons are called "#towelcome" etc. I'm assuminig I'm missing something stupid and obvious. All of the divs are hidden in css.

var elements = ["welcome", "slides", "projects", "pages"];
for (i=0;i<=elements.length-1;i++){ 
    $("#to"+elements[i]).click(function() {
      $("#"+elements[i]).show();
    });
}

/UPDATE:------------------------------------------------------------------------

The following code is the code I am trying to minify.

$("#toprojects").click(function(){
    $("#projects").show();
    $("#slides").hide();
    $("#pages").hide();
    $("#welcome").hide();
});

$("#toslides").click(function(){
    $("#projects").hide();
    $("#slides").show();
    $("#pages").hide();
    $("#welcome").hide();
});

$("#topages").click(function(){
    $("#projects").hide();
    $("#slides").hide();
    $("#pages").show();
    $("#welcome").hide();
});

if(window.location.hash === "#slides"){
    $("#projects").hide();
    $("#slides").show();
    $("#pages").hide();
    $("#welcome").hide();
}

if(window.location.hash === "#projects"){
    $("#projects").show();
    $("#slides").hide();
    $("#pages").hide();
    $("#welcome").hide();
}

if(window.location.hash === "#pages"){
    $("#projects").hide();
    $("#slides").hide();
    $("#pages").show();
    $("#welcome").hide();
}

found a related post here and it solved it pretty nicely jQuery: Set click from array loop

Eli · Accepted Answer · 2011-04-26 17:41:23Z

3

I haven't tested this, but I think something along these lines should work for you:

var elements = ["welcome", "slides", "projects", "pages"];

var clickHandler = function(element) {
    var id = element.attr('id') || $(this).attr('id');

    $.each(elements, function(index, value) {
        var element = $('#' + value);
        if (id !== 'to' + element.attr('id')) {
            element.hide();
        } else {
            element.show();
        }
    });    
};

$.each(elements, function(index, value) {
    $('#to' + value).click(clickHandler);
});

if ($.inArray(window.location.hash, elements)) {
    var id = window.location.hash.substring(1);

    clickHandler($('#' + id));
}

edited Apr 26, 2011 at 17:41

answered Apr 18, 2011 at 18:37

Eli

17.8k4 gold badges40 silver badges49 bronze badges

Sign up to request clarification or add additional context in comments.

6 Comments

Spudley Over a year ago

.each is a JQuery method, so it won't work on elements unless you wrap it in a $(). I don't think there was anything wrong with the original loop anyway.

Richard Marskell - Drackir Over a year ago

I think you mean $.each(elements, function(index, value) { /*..*/ }, no?

Eli Over a year ago

You are right that there is nothing to gain from using each, just cleaner code is all. That's why for performance I would recommend the second approach.

Richard Marskell - Drackir Over a year ago

Out of curiosity, why this , len = elements.length; i < len; and not ; i < elements.length;?

Eli Over a year ago

It is a performance optimization. By caching the value of length, we do not re-evaluate the length every iteration of the loop. It is considered a best practice by many front-end engineers.

|

Naren Sisodiya · Accepted Answer · 2011-04-18 18:45:22Z

2

var elements = ["welcome", "slides", "projects", "pages"];
jQuery.each(elements,function(){ 
element = this;
    $("#to"+element).live("click",function() {
      $("#"+element).show(); //  use toggle for hiding/show as per current state

    });
});

answered Apr 18, 2011 at 18:45

Naren Sisodiya

7,2882 gold badges27 silver badges35 bronze badges

1 Comment

ThomasReggi Over a year ago

i set jQuery.each(elements, function(index, element) { and it works

Emmett · Accepted Answer · 2011-04-18 18:43:02Z

1

You are running into an issue with closure. The value of i, regardless of which element is clicked, will be 4. There are many ways around it, but jQuery offers a particularly convenient one, the each function:

var elements = ["welcome", "slides", "projects", "pages"];
$.each(elements, function(index, element) {
    $("#to" + element).click(function() {
        $("#" + element).show();
    });
});

answered Apr 18, 2011 at 18:43

Emmett

14.4k12 gold badges58 silver badges82 bronze badges

1 Comment

ThomasReggi Over a year ago

I think I need the for loop, please look at the updates to the post.

Spudley · Accepted Answer · 2011-04-18 18:59:32Z

1

We need to see more of your code to be sure, but the obvious things that might be missing are:

You're using JQuery functionality here: have you included the jQuery library in your page?
The code above needs to be run after the document has fully loaded, otherwise the click() event may not get registered to the button properly. The best way to do this is to use jQuery's $(document).ready() method. Simply wrap all the above code inside the $(document).ready() {} function, and it should work.

You also need to pass the i variable into the click function, as it won't actually get called until later, and the original value of i will have long since disappeared by then.

$(document).ready() {
    var elements = ["welcome", "slides", "projects", "pages"];
    for (i=0;i<=elements.length-1;i++){ 
        $("#to"+elements[i]).click(function(i) {
            $("#"+elements[i]).show();
        });
    }
}

As others have suggested, you could refactor the loop to use .each(), but I don't see any point in that. I might have used for(i in elements) {...} instead, but the syntax you've used is fine.

edited Apr 18, 2011 at 18:59

answered Apr 18, 2011 at 18:38

Spudley

169k39 gold badges240 silver badges309 bronze badges

3 Comments

ThomasReggi Over a year ago

var i inside the click function is [object Object]

ThomasReggi Over a year ago

everything is connected and wrapped in doc.ready it also needs to be $(document).ready(function() { no?

Spudley Over a year ago

@Thomas - yes, you're right; I typed it up in a hurry. But you get what I was trying to say at least. In any case, having seen your edits to the question, I think there's a better solution than any of the answers have given so far (including mine), so I've posted an alternative answer.

Spudley · Accepted Answer · 2011-04-19 08:20:50Z

Looking at your updates with the code you're 'trying to minify', I think a different approach may work just as well. Using the JQuery toggle() function and some class names as well as the IDs, you can put the whole thing into a few lines, without needing a loop at all. Something like this:

$(".to-button").click(function(){
    var name = $(this).attr('id').replace(/^to/,'');
    $(".infopanel").hide();
    $("#"+name).show();
});

Then your HTML code would have classes like so:

<div id='toprojects' class='to-button'>Projects</div>

and

<div id='projects' class='infopanel'>info about projects here...</div>

and similarly for slides, pages and welcome.

I was trying to come up with a way to combine the hide() and show() lines into one using .toggle() with the showorhide flag, but it's too early in the morning to be thinking too hard. ;-) I'm sure there is a way though.

Collectives™ on Stack Overflow

Click function inside for loop

5 Answers 5

6 Comments

1 Comment

1 Comment

3 Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

6 Comments

1 Comment

1 Comment

3 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related