Checking all checkboxes

Question

Problems:

1. Wanting to check all checkboxes when the checkbox w/id=checkall is checked.
2. Wanting to uncheck the "checkall" box when any other checkbox is unchecked.

Solution:

$(document).ready(function () {
$('#checkall').on('click', function () {
if ($(this).is(':checked')) {
 $('input[type="checkbox"]').each(function () {
 console.log(this);
 this.checked = true;
 });
} else {
 $('input[type="checkbox"]').each(function () {
 this.checked = false;
 });
}
// Uncheck "Checkall" if any other checkbox is unchecked
if ($('#checkall').is(':checked')) {
 $('input[type="checkbox"]:not("#checkall")').click(function () {
 console.log(this);
 $('#checkall').attr('checked', false);
 });
}
});
});

I'm new with jQuery and I'm not sure if what I've written is really and truly "production value" code, but it is operational. It's not a matter of life and death, it's really just a personal project I'm working on that will likely be used by myself and some friends only. I am in the game to learn, though, and if the code can be improved upon please point me in the right direction to read about it more in depth. I've only been able to find bits and pieces on this topic.

Answer 1

I've written my changes in the code with comments, it explains it easier :) Most of the changes are "good to knows" and you dont need the each() if you want them all to do the same.

$(document).ready(function () {
 // We use the checkboxes multiple times on a page, save the reference
 // This will save DOM lookups, thus smoother performance (though very little in this case)
 var $allCheckBoxes = $('input[type="checkbox"]').not('#checkall'); // prefix with $ to indicate jQuery object (this is a preference, not a must)
 // We use it multiple times, save the reference
 var $checkAllTrigger = $('#checkall'); 
 // We only need to calc this once, not needed every click, so save it in a var
 var allCheckBoxesLength = $allCheckBoxes; // this gets set once, and that's exactly enough
 $checkAllTrigger.on('click', function () {
 // Use this.checked, "this" is the clicked element, ".checked" is true/false if it's checked or not
 $allCheckBoxes.prop('checked', this.checked); 
 })
 // as per Paul's comment, blur the element to remove.
 // We chain the blur() to the on(), on() returns the object, you can think $(this) 
 // gets returned, which you then blur. 
 .blur();
 // Uncheck "Checkall" if any other checkbox is unchecked
 // We do this by comparing the total amount of checkboxes vs the checked ones.
 // If e.g. 10 out of 10 are checked, they're all checked, check this on
 $allCheckBoxes.on('change', function(){
 $checkAllTrigger.prop('checked', allCheckBoxesLength == $allCheckBoxes.filter(":checked").length);
 });
});

• Paul S: One thing that kind of bugs me about form elements is the "focus" - for instance when you check a checkbox you're left with an ugly dotted square around the checkbox. I chose to add a variable to the above code by Martijn. -- $anyCheckBox = $('input['type="checkbox"]'); and added $anyCheckBox.on('click', function () { $(this).trigger('blur'); so all focus is removed.
• Martijn: Added it in my code, but "reviewed" style ;)
• Paul S: Great Stuff! Pretty much have a basic "Checkall" checkbox controller. I tried out the .blur(); addition and it's seemingly not working. For the moment, I still have the variable $anyCheckBox defined as the same as $allCheckBoxes without the .not('#checkAll') and using $anyCheckBox.on('click', function () { $(this).trigger('blur'); }); code to blur the checkbox. From how the code reads - it looks like it should work but it would still keep the focus on the checkall box, no?
• Martijn: That would require to make a jsFiddle to test. You could change it to .trigger('blur'), but that's based on only the all-checker to be focussed. I'm assuming the others don;t get focussed.

Answer 2

You can streamline the code by caching the selectors; this will improve performance as well. Next, you should use prop instead of attr as the jQuery documentation states:

The .prop() method should be used to set disabled and checked instead of the .attr() method.

Lastly, instead of using an if..else statement, you can write the condition inline, since checked expects a boolean, and the expression resolves to a boolean. In other words:

// Cache selectors
var $all = $('#checkall');
var $checks = $('input[type="checkbox"]').not($all);
$all.on('click', function () {
 $checks.prop('checked', $(this).is(':checked'));
});
$checks.on('click', function() {
 var checked = $checks.filter(':checked').length;
 $all.prop('checked', checked === $checks.length);
});

Here is a demo: http://jsfiddle.net/z7qy10nn/

• Paul S: I looked at the jQuery documentation regarding the .prop method and your explanation makes more sense as we're working with properties as opposed to attributes. I really like the declaration of variables at the beginning -- coming from PHP that's more my style haha -- and it keeps the code looking nice. Thanks for the fiddle as well - that was really helpful!
• Martijn: Your var checked, I find it dangerous. While it might work and isn't a reserved keyword, I'd use another name to make sure (now it's the same name as an possible property of an object)