1
\$\begingroup\$

I have a table with 'nested' inputs that need to be checked or unchecked when a button 'select all' is pressed. There are several levels that can be toggled with their respective 'select all' button. In my code I have a repetitive .each() statement where the second one tests if there are differing numbers of checked/unchecked inputs and if there are it overwrites what the first .each() statement did. How can I write the following code better?

$(document).ready(function(){
    $(".checkAll").click(function(e){
        e.preventDefault();
        var thisRow = $(this).parents("tr");
        var thisRowNumber = parseInt(thisRow.attr("id").replace("row",""));
        var thisLevel = parseInt(thisRow.children().first().attr("class").replace("textlevel",""));
        var checked = 0;
        var unchecked = 0;
        $(".tablefull tr").each(function(){
            if($(this).attr("class") !== "heading"){
                r = parseInt($(this).attr("id").replace("row",""));
                if(r > thisRowNumber){
                    n = parseInt($(this).children().first().attr("class").replace("textlevel",""));
                    if(n === thisLevel){
                        return false;
                    }
                    if(n > thisLevel){
                        $(this).find("input[name='ci_id']").prop("checked",function(index, oldAttr){
                            if(oldAttr){
                                checked++;
                            }
                            else {
                                unchecked++;
                            }
                            return !oldAttr;
                        });
                    }
                }
            }
        });
        if(checked > 0 && unchecked > 0){
            $(".tablefull tr").each(function(){
                if($(this).attr("class") !== "heading"){
                    r = parseInt($(this).attr("id").replace("row",""));
                    if(r > thisRowNumber){
                        n = parseInt($(this).children().first().attr("class").replace("textlevel",""));
                        if(n === thisLevel){
                            return false;
                        }
                        if(n > thisLevel){
                            $(this).find("input[name='ci_id']").prop("checked",true);
                        }
                    }
                }
            });
        }
    });
});

here's the working code in a fiddle: http://jsfiddle.net/KnU4X/1/

here's the code without my lengthy fix in a fiddle: http://jsfiddle.net/KnU4X/2/

To see the problem, click 'select all' with level 1, then 'select all' with level 2, then 'select all' with level 1 again. You will notice it unchecks everything except the level 2 which becomes checked.

Is there a way to remove all that seemingly redundant code and keep that functionality?

\$\endgroup\$
2
\$\begingroup\$

In general, you would have an easier time doing this if your "nested rows" were actually nested. However - that leads to a variety of other layout problems, etc.

That said - here is a simpler way to accomplish what I think you're looking for:

jQuery code:

$(".checkAll").click(function (e) {
    e.preventDefault();

    // Find the row whose button was clicked.
    var $row = $(this).closest('tr');

    // Capture the associated "level" class.
    var cls = $row.children('td:first-child').attr('class');

    // Create a collection of rows, in which you can store the child-rows
    var $children = $row;

    // Start with the next row, and...
    var $current = $row.next();

    // ... for every subsequent row which is *not* on the same level as the
    // one you started with...
    while ($current.length && ($current.children('td:first-child').attr('class') !== cls)) {
        // Add the row to the collection.
        $children = $children.add($current);
        // Then move on to the next row.
        $current = $current.next();
    }

    // Capture all of the *checkboxes* within the matching rows
    var $checks = $($children).find('input[type="checkbox"]');

    // If ALL checkboxes are checked, uncheck them.
    // Otherwise, check them all.
    if ($checks.length == $checks.filter(':checked').length)
        $checks.prop('checked', false);
    else
        $checks.prop('checked', true);
});

Here is a working jsFiddle showing the code in-action.

\$\endgroup\$
1
\$\begingroup\$

Interesting scenario.

First an foremost, you're making it harder for yourself by relying on the class name. This forced you to parse class name strings then parse the value to integers.

Hence, I've simply added data-level attributes for each <tr>, signifying their levels. Now you have a value which you do not need to parse.

That is all I've changed in the HTML (which probably means now your HTML has redundant things you can remove, I'll leave that to you)

Next, is the script. Without the need to parse strings and values, you can focus on the logic. IMHO, the code logic is now much more readable and less distracting by all the string/value parsing you've been doing.

DEMO: http://jsfiddle.net/terryyounghk/vdEe8/

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.