jQuery for loop not recognizing variable in closure in Drupal behavior...?

Question

Can anyone tell me why this might not work

Drupal.behaviors.toggleGroups = {
  attach:function(context, settings) {
    for (var i = 1; i < 8; i++) {
      $('#edit-group-' + i.toString() + '-toggle').unbind('click').click(function(i) {
        $('#category-' + i.toString()).slideToggle();
      });
    }
  }
};

But this ugly thing works just fine

Drupal.behaviors.toggleGroups = {
  attach:function(context, settings) {
    $('#edit-group-1-toggle').unbind('click').click(function() {
      $('#category-1').slideToggle();
    });
    $('#edit-group-2-toggle').unbind('click').click(function() {
      $('#category-2').slideToggle();
    });
    $('#edit-group-3-toggle').unbind('click').click(function() {
      $('#category-3').slideToggle();
    });
    $('#edit-group-4-toggle').unbind('click').click(function() {
      $('#category-4').slideToggle();
    });
    $('#edit-group-5-toggle').unbind('click').click(function() {
      $('#category-5').slideToggle();
    });
    $('#edit-group-6-toggle').unbind('click').click(function() {
      $('#category-6').slideToggle();
    });
    $('#edit-group-7-toggle').unbind('click').click(function() {
      $('#category-7').slideToggle();
    });
  }
};

Ideally I'd like to do something like "while selector returns result do something" - the problem is that I need the incremented number, and each click will toggle a separate div. It possible I'm just thinking about it all wrong, but regardless, I can't figure out why what I have isn't valid...

And while your at it, any advise on the jQuery once() method so I don't have to unbind/bind the click handler would also be appreciated...

THANK YOU!

If you use 2 common classes instead of a thousand ids you can forget about for loops and closures. — elclanrs
– elclanrs, Commented Dec 13, 2012 at 23:25

Sushanth -- · Accepted Answer · 2012-12-13 23:21:18Z

3

Classic closure issue..

Try this

Drupal.behaviors.toggleGroups = {
  attach:function(context, settings) {
    for (var i = 1; i < 8; i++) {
      (function(num){
         $('#edit-group-' + num + '-toggle')
                                    .unbind('click').click(function(num) {
            $('#category-' + num).slideToggle();
         });
      })(i)
    }
  }
};

By the time the functions are assigned the variable i shares the same memory location. SO it will always point to the last instance of i

answered Dec 13, 2012 at 23:21

Sushanth --

55.8k9 gold badges70 silver badges109 bronze badges

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

6 Comments

Blender Over a year ago

Could you explain why this works? Also, do you really need the toString()s?

Sushanth -- Over a year ago

@Blender .. We are executing the function immediately for that particular instance of i .. So the event will be bound the current iteration rather than the last instance of the iteration

mraaroncruz Over a year ago

@Blender the (function (num) { ... })(i) part means execute this function now and use the current value of i as its argument

Aaron Lamb Over a year ago

Well, it looks really good, and is indeed fascinating, but I can't say I'm getting it to work:

attach:function(context, settings) {         for (var i = 1; i < 8; i++) {           (function(num) {             $('#edit-group-' + num + '-toggle').unbind('click').click(function(num) {               $('#category' + num).slideToggle();             });           })(i);         }       }

- this is producing the same non-result as my loop.

Aaron Lamb Over a year ago

Nope - correcting the $('#category' + num) selector to the correct $('#category-' + num) didn't help either. Is there something I'm missing?

|

elclanrs · Accepted Answer · 2012-12-13 23:31:44Z

2

This is a common case of "iditis". If you use two common classes everything will be much easier to manipulate. Remove ids and add classes, edit-group-toggle and category, then you can grab their corresponding targets by index:

attach: function( context, settings ) {
  $('.edit-group-toggle').click(function() {
    $('.category').eq( $(this).index() ).slideToggle();
  });
}

answered Dec 13, 2012 at 23:31

elclanrs

94.2k21 gold badges137 silver badges171 bronze badges

1 Comment

Aaron Lamb Over a year ago

Awesome - thank you! I didn't know about eq either, obviously, but this was exactly what I needed. I did need to traverse the dom a bit to get to a structure with suitable indexes thanks to D7 Froms API and my own idiosyncrasy, but yes, iditis was a correct diagnosis!

Collectives™ on Stack Overflow

jQuery for loop not recognizing variable in closure in Drupal behavior...?

2 Answers 2

6 Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

6 Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related