0

I have have multiple divs' with similar code within it, but also has a unique id within the main div that calls a toggleClass & slideToggle function. I'm trying to create a loop so that I do not have to write similar code for each element.

--- working code --- (where numeric value after the id would change)

$('#er1').click(function() {
    $('#er1').toggleClass('but-extra-on');
    $('#cr1').toggleClass('but-closerow-on');
    $('#er1').next('div').slideToggle('slow');
    return false;
});

-- not working code -- (I want to have functions for the click of #er1, #er2, #er3 etc.)

var count = 1;
while (count < 10){
    var curER = 'er'+count; 
    var curCR = 'cr'+count; 
    $('#'+curER).click(function() {
        $('#'+curER).toggleClass('but-extra-on');
        $('#'+curCR).toggleClass('but-closerow-on');
        $('#'+curER).next('div').slideToggle('slow');
    });
    count++;
}

* for some reason, when I use the while code, whenever I click #er1, #er2, #er3 etc.. only the event for #er9 toggles.

Felix Kling
  • 795,719
  • 175
  • 1,089
  • 1,143
bobe
  • 3
  • 1
  • 1
  • 2
  • Can you post a [JS Fiddle demo](http://jsfiddle.net/) demo, showing the working/non-working code? I assume that the last iteration of the `while` loop sets everything to the final value of `count`. – David Thomas Nov 03 '11 at 17:58
  • 1
    possible duplicate of [Access outside variable in loop from Javascript closure](http://stackoverflow.com/questions/1331769/access-outside-variable-in-loop-from-javascript-closure) and [Javascript closure inside loops - simple practical example](http://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example) – Felix Kling Nov 03 '11 at 17:59
  • The problem is that you've declared curCR and curER within the loop. You need to move the variables outside of the loop. – Cory Danielson Nov 03 '11 at 18:20
  • I found 2 solutions for you, check them out :D – Cory Danielson Nov 03 '11 at 18:53

2 Answers2

2

You can solve this problem, by using the $(this) selector for the one that you are clicking, and attaching an html data attribute to the div, specifying the other div that you want to change when you click it and selecting the other one with that... make sense.. probably not? Check out Solution 1 below.

The second solution is to use jQuery Event Data to pass the count variable into the event listener.

Solution 1: http://jsfiddle.net/Es4QW/8/ (this bloats your html a bit)
Solution 2: http://jsfiddle.net/CoryDanielson/Es4QW/23/

I believe the second solution is slightly more efficient, because you have slightly less HTML code, and the $(this) object is slightly smaller. Creating the map and passing it as event data, I believe, is less intensive... but... realistically... there's no difference... the second solution has cleaner HTML code, so you should use that.

Solution 3 w/ .slideToggle(): http://jsfiddle.net/CoryDanielson/Es4QW/24/

Edit: Updated solutions by passing in the selected elements instead of the selectors. Now each click event will not do a DOM lookup as it did before.

Cory Danielson
  • 14,314
  • 3
  • 44
  • 51
0

I've run into this problem before. I fixed it by extracting the event listener code into its own function like so:

for (var i = 1; i < 10; i++) {
    attachClickEvent('er'+i, 'cr'+i);
)

function attachClickEvent(cr, er)
{
    $('#'+er).click(function() {
        $(this).toggleClass('but-extra-on');
        $('#'+cr).toggleClass('but-closerow-on');
        $(this).next('div').slideToggle('slow');
    });
}
Yes Barry
  • 9,514
  • 5
  • 50
  • 69