CODEWITHSUNDEEP
javascriptjqueryhtml
RationalRabbit
RationalRabbit

Reputation: 1067

jQuery nested functions looping twice

In my limited understanding of jQuery, I don't understand what is happening here. I have two divs, each with a class of "required". The 1st div contains a set of checkboxes, while the second contains radio buttons. The object is to highlight the appropriate div background if no objects in either set are selected.

The 2nd set, however (the radio buttons), does have a button pre-selected by default, so should never be highlighted.

Here is what is happening: Both sets are being looped through twice. At the end of the 1st div (checkbox) loop, the div is correctly highlighted. The radio button items are them checked and the default checked button is recognized and that div is not highlighted. Then, the first div items are again gone through. At the end of this - and BEFORE the 2nd div is looped through for the second time, the 2nd div is highlighted. I am assuming that the cause here, even though it is correctly looping through the checkboxes, it is associating them with the id of the radiobuttons. What I need to do is prevent this second loop-through.

$(document).ready(function () {
    $('#Submit').click(function () {
        $('div.Required', '#JobForm').each(function () {
            var FieldName = this.id;
            var FieldLength = FieldName.length
            var CheckError = true;
            $("input:checkbox").each(function () {
                if (this.checked) {
                    CheckError = false;
                }
            });
            if (CheckError) {
                ErrorList += FieldName + ' requires a selection\n';
                this.style.backgroundColor = "#FFC0C0";
            }
            CheckError = true;
            $("input:radio").each(function () {
                if (this.checked) {
                    CheckError = false;
                }
            });
            if (CheckError) {
                ErrorList += FieldName + ' requires a selection\n';
                this.style.backgroundColor = "#FFC0C0";
            }
        });
        if (Error) {
            $('#Submit').val('Submit');
            alert(ErrorList)
            return false;
        } else {
            document.JobForm.submit();
        }
    });
});

Thanks to adeneo's suggestions, I was able to limit the double div looping, which also allowed me to eliminate the additional error message. As adeneo stated, since there are two div's, as originally written, the code was looping through both input types each time. As written below, the first loop loops through the checkboxes, while the second loops through the radio buttons.

$(document).ready(function()
{
    $('#Submit').click(function ()
    {
        $('div.Required','#JobForm').each(function()
        {
            var FieldName = this.id;
            var FieldLength = FieldName.length
            var CheckError = true;
            $("input:checkbox", this).each(function ()
            {
                if(this.checked){CheckError=false;}
            });
            $("input:radio", this).each(function ()
            {
                if(this.checked){CheckError=false;}
            });
            if(CheckError){ErrorList += FieldName+' requires a selection\n'; this.style.backgroundColor="#FFC0C0";}
        });
        if(Error)
        {
            $('#Submit').val('Submit');
            alert(ErrorList)
            return false;
        }
        else
        {
            document.JobForm.submit();
        }
    });
});

Upvotes: 0

Views: 302

Answers (1)

adeneo
adeneo

Reputation: 318182

You're not iterating over each DIV twice, but you're iterating over all the inputs the same number of times as you have matching DIV's

$('#Submit').click(function () {

    // loop over the DIV's, lets say two times if there are two elements
    $('div.Required', '#JobForm').each(function () { 
        // ..... other code

        // the code below selects all the checkboxes, both times

        $("input:checkbox").each(function () {
            if (this.checked) {
                CheckError = false;
            }
        });

        // the code below selects all the radiobuttons, both times

        $("input:radio").each(function () {
            if (this.checked) {
                CheckError = false;
            }
        });

     // .......more code
    });
});

Limit the selection of inputs with context or find()

$("input:checkbox", this).each(function () { ...

or

$(this).find("input:checkbox").each(function () { ...

also, your conditions don't make sense, you keep setting CheckError to a boolean, and on the next line you're checking for that boolean, and you'll never get a different result

CheckError = true;
if (CheckError) { ...

is like doing

if (true) { ...

Upvotes: 1

Related Questions