Toggle effect problem when using for-loop in IE7

Question

I'm a webdesigner that's trying to get the hang of JavaScript and jQuery, and I want to learn how to write shorter, more concise code - to avoid being ridiculed by the developers at work ;)

I have this snippet:

// toggle divs when checkbox is checked
$('.product-optional-checkbox1').click(function () {
    $('.product-optional-toggle1').toggle('fast');  
});

$('.product-optional-checkbox2').click(function () {
    $('.product-optional-toggle2').toggle('fast');  
});

$('.product-optional-checkbox3').click(function () {
    $('.product-optional-toggle3').toggle('fast');  
});

// hide divs
$('.product-optional-toggle1').hide(); 
$('.product-optional-toggle2').hide(); 
$('.product-optional-toggle3').hide();

...that I want to reduce using a for-loop, like this:

for( var i = 1; i < 4; ++i ) {
    $('.product-optional-checkbox' + i).click(function () {
        $(this).parent('div').find('div').toggle('fast');  
    });
    $('.product-optional-toggle' + i).css({ display: 'none'}); 
};

It works fine in FF, however in IE7 it toggles twice. Anyone know who to solve a problem like this?

Answer 1

It's hard to say without seeing the HTML structure but maybe going to the parent then descendants is finding multiple divs?

In your example, you could replace:

$(this).parent('div').find('div').toggle('fast');

With code more similar to the original examples:

$('.product-optional-toggle' + i).toggle('fast');

However, it would be much better to ditch all the numbers and just use a class of .product-optional-checkbox. This way you can add a click function to all elements of that class in one go and avoid the loop:

$('.product-optional-checkbox').click(function () {
    // do stuff using $(this)
});

Answer 2

Given that your click events seem to be tied to checkboxes (based on the class names you have in your example), why not actually provide code to handle click events for those checkboxes? For example:

<div id='my_group_of_checkboxes'>
  <input type="checkbox" id="checkbox1">
  <input type="checkbox" id="checkbox2">
  ...
</div>

And your jQuery code is then stripped down to three lines:

$('#my_group_of_checkboxes :checkbox').click(function(){
  $(this).parent('div').hide('fast');
});

You also seem to need to hide the <div> elements related to each checkbox:

$('div[class^="product-optional"]').hide();

Although ID selectors would be a better option here and, depending on their position within your page, you may even be able to get away with something like:

$('#my_container_div_id div').hide();

If you can post some of your HTML, that might help provide more accurate answers as well.