Hans Pede Hans Pede - 5 months ago 7
jQuery Question

This script works as I want to, but its ugly

I have a script which I worked hard on, but it is pretty ugly but it works. It can check all check boxes (with .select-all), and if one of those check boxes (.chk-box) is unchecked, the .select-all will also become unchecked. Moreover the .select-all will also hide a div (.hidethis). I am in the learning state, so if someone (if easy) can show me a nicer code with this functionality it is very welcome! It can learn me a alot!

<script type="text/javascript">

$("document").ready(function()
{
if($(".select-all").is(":checked"))
{
$(".hidethis").hide();
}
else
{
$(".hidethis").show();
}
});

$(".select-all").change(function ()
{
if (this.checked)
{
$(".chk-box").prop('checked', $(this).prop("checked"));
$(".hidethis").hide();
}
else
{
$(".chk-box").removeAttr("checked");
$(".hidethis").show();
}
});

$(".chk-box").change(function ()
{
if($(".chk-box").length == $(".chk-box:checked").length)
{
$(".select-all").prop('checked', $(this).prop("checked"));
$(".hidethis").hide();
}
else
{
$(".select-all").removeAttr("checked");
$(".hidethis").show();
}
});

</script>

Answer

Your code isn't ugly - but if($(".select-all").length == $(".select-all:checked").length) isn't immediately clear on what it does (if your goal here is readability). Also, you don't need the if/else on the select-all change, since you always want to set all chk-boxes to whatever the value of select-all is anyway.

Here's a fiddle I made that's a bit cleaner. I just run a loop over all the check boxes to see if they are all set or not. The check all handler just applies its value to all check boxes. The hide-all can just be handled in those two events.

The JS:

$(".check").on("change", function() {
    var allChecked = true;
    $(".check").each(function(i) {
        allChecked = allChecked && $(this).prop("checked");
    });
    $(".check-all").prop("checked", allChecked);

    allChecked ? $(".hidethis").hide() : $(".hidethis").show();
});

$(".check-all").on("change", function() {
    var checked = $(this).prop("checked");
    $(".check").prop("checked", checked);

    checked ? $(".hidethis").hide() : $(".hidethis").show();
});
Comments