Toggle classes with radio button in jQuery

Question

I have a set of radio buttons where when I click a radio button, I want the label to change color or whatever. But when I click another radio button, the color goes away. Therefore I have something like this:

jQuery('label').mouseup(function(){
   jQuery(this).prev().attr('checked', 'checked');
   jQuery('input').next().removeClass('selected');
   jQuery('input:checked').next().addClass('selected');

});

if you need to see some html:

<input type="radio" id="radio1" name="myRadio" value="option-1" />
<label for="radio1">Label 1</label>


<input type="radio" id="radio2" name="myRadio" value="option-2" />
<label for="radio2">Label 2</label>

This first removes 'selected' class from all the labels and then re-applies to only the checked labels.

It works and is simple, but I was thinking this might not be the most efficient way of doing this. I imagine that javascript is iterating through each input element and using more resources than necessary.

I'm curious if anyone knows of a common way of doing this more efficiently. I seem to be doing this type of thing quite often in my jQuery code. I've just been using jQuery for the past 3 months or so btw.

Answer 1

Besides the use of mouseup who seems a little bit unusual in this case (at least, to me), your code is fine.

I'd use click or change.

Answer 2

There are a few things I think are worth mentioning.

Clicking on a <label> will automatically change the value of the <input>. You don't need to set the checked attr manually, and therefore could bind to the change event on the radios instead. This will also allow keyboard events to select/deselect the radios, and will work anytime the radio values change, not just when someone raises their mouse over a label.

Also, you can save the whole collection of radio inputs in its own variable to make referencing them later not have to search through the DOM again.

Suggested code (w/ jsfiddle preview)

var $radios = jQuery('input[type=radio]');
$radios.change(function() {
   $radios.next().removeClass('selected');
   $radios.filter(':checked').next().addClass('selected');
});

Answer 3

It actually looks like it works when clicking the label. I wouldn't worry too much about the number of DOM elements searched as long as the performance is ok. Optimize when it becomes a problem and not before. Clarity/readability is probably more important. You might be able to improve it, though, by using some information from the label or its related input to narrow down the selectors. Using end and filter would also allow you to reuse the second query.

$('label').click(function(){
   var radio = $(this).prev();
   radio.attr('checked', 'checked'); 
   var name = radio.attr('name');
   $('input[name=' + name ']').next()
                              .removeClass('selected')
                              .end()
                              .filter(':checked')
                              .next()
                              .addClass('selected'); 

}); 

Note that using change on the actual radios, as some others suggest might be better. You could use the same techniques with that.

$('input[type=radio]').change( function() {
     $('input[type=radio]').next()
                           .removeClass('selected')
                           .end()
                           .filter(':checked')
                           .next()
                           .addClass('selected');
});

Use live handlers if you are adding radios dynamically to the page.