views:

28

answers:

1

Hi all,

I'm trying to get better at using Jquery and therefore would like feedback on ways to optimize my script below.

To briefly describe the functionality I have a "checkall" checkbox and a button for performing actions on the checked elements. If there's no elements checked, my button should be disabled and have the class disabled added aswell. If there's just one element checked the button should not be disabled, neither have the class disabled.

Thanks in advance

$(document).ready(function(){

$('#checkall').click(function () {
    $(this).parents('.table_form:eq(0)').find(':checkbox').attr('checked', this.checked);

    if($(this).parents('.table_form:eq(0)').find(':checkbox').is(':checked')) {
        $("#delete_selected").attr("disabled");
        $("#delete_selected").removeClass("disabled");
    } else {
        $("#delete_selected").removeAttr("disabled").addClass("disabled");
    }
});

$("#blog_posts tbody :checkbox").click(checked_status);

});

function checked_status() {
    var n = $("input:checked").length;

    if(n > 0) {
        $("#delete_selected").attr("disabled");
        $("#delete_selected").removeClass("disabled");
    } else {
        $("#delete_selected").removeAttr("disabled").addClass("disabled");
    }
}
+2  A: 

I don't have a lot to say about optimization here, but presumably you are trying to set this to disabled:

$("#delete_selected").attr("disabled"); // won't work, simply returns true or false

It should instead be:

$("#delete_selected").attr("disabled", true); // or "disabled"

and you could use chaining:

$("#delete_selected").attr("disabled", true)
                     .removeClass("disabled");

See http://api.jquery.com/attr/

Also, this condition:

if($(this).parents('.table_form:eq(0)').find(':checkbox').is(':checked')) {

can be changed to the slightly more concise:

if($(this).closest('.table_form').find(':checkbox:checked').length) {

But that's really a preference thing.

karim79
Right I just discovered that bug myself. Thanks for pointing out though!The code is now like this:$("#delete_selected").attr("disabled", false);$("#delete_selected").removeClass("disabled");
kris