jquery function is not working on loop

Question

I am trying to make a custom validation for my form. We won't know in future how many boltons we are going to add / remove from our form.

It should check if radio buttons are not checked add "error class" to section.

The problem is any radio button I click just effect the last section.

function checkBoltons(){

    var boltons = [1,2,3];

    for (var i of boltons) {
    
		
			var $bolton = $('#bolton-' + i);
			
			if ( $('input[name="data[ObeBolton]['+i+']"]').is(':checked') == true ) {
				
				$bolton.removeClass('section-error');
				
			} else {
				$bolton.addClass('section-error');
				
			}
			
			$('input[name="data[ObeBolton]['+i+']"]:radio').change(function(){
				$('#bolton-' + i).removeClass('section-error');
			});
    }		
}

$(".checkBoltons").click(function(){
    checkBoltons();
})

.section-error {
	background-color: rgb(255, 218, 218);
	border: 1px solid red;
}

<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<div id="bolton-1">
    <p>bolton-1</p>
    <input type="radio" name="data[ObeBolton][1]" value="1" />
    <input type="radio" name="data[ObeBolton][1]" value="2" /> 
</div>

<div id="bolton-2">
    <p>bolton-2</p>
    <input type="radio" name="data[ObeBolton][2]" value="3" />
    <input type="radio" name="data[ObeBolton][2]" value="4" /> 
</div>

<div id="bolton-3">
    <p>bolton-3</p>
    <input type="radio" name="data[ObeBolton][3]" value="5" />
    <input type="radio" name="data[ObeBolton][3]" value="6" /> 
</div>


<button class="checkBoltons">Check Boltons</button>

The short answer is an adding a scoping function so that i is still valid when the events occur much later. The better approach is a re-write to use a single handler (with data- attributes as required). — iCollect.it Ltd
– iCollect.it Ltd, Commented Apr 22, 2015 at 10:53
@TrueBlueAussie I have tried your solution but doesn't work with data- parent attribute — Fury
– Fury, Commented Apr 22, 2015 at 10:59

Community · Accepted Answer · 2017-05-23 12:05:55Z

2

As @TrueBlueAussie suggested, the solution here is to use a single handler

<div id="bolton-1" class="bolton">
    <p>bolton-1</p>
    <input type="radio" name="data[ObeBolton][1]" value="1" />
    <input type="radio" name="data[ObeBolton][2]" value="2" />
</div>
<div id="bolton-2" class="bolton">
    <p>bolton-2</p>
    <input type="radio" name="data[ObeBolton][2]" value="3" />
    <input type="radio" name="data[ObeBolton][2]" value="4" />
</div>
<div id="bolton-3" class="bolton">
    <p>bolton-3</p>
    <input type="radio" name="data[ObeBolton][3]" value="5" />
    <input type="radio" name="data[ObeBolton][3]" value="6" />
</div>
<button class="checkBoltons">Check Boltons</button>

then

$(".checkBoltons").click(function () {
    $('.bolton').each(function(){
        var checked = $(this).find(':radio').is(':checked');
        $(this).toggleClass('section-error', !checked);        
    })
})
$('.bolton input:radio').change(function () {
    $(this).closest('.bolton').removeClass('section-error');
});

Demo: Fiddle

edited May 23, 2017 at 12:05

CommunityBot

11 silver badge

answered Apr 22, 2015 at 11:06

Arun P Johny

389k68 gold badges532 silver badges532 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

iCollect.it Ltd Over a year ago

Now this is the way to do it +1 :)

Shlomi Hassid · Accepted Answer · 2015-04-23 09:32:59Z

2

Quick and easy without changing your HTML:

$(function(){
    //Validate
    $(".checkBoltons").click(function() {
        var elements = $('div[id^="bolton-"]');
        $.each(elements, function(i,ele) {
            if($(ele).find("input:checked" ).length) {
                $(ele).removeClass('section-error');
            } else {
                $(ele).addClass('section-error');
            }
        });
    });
    
    //Remove when fixed
    $('div[id^="bolton-"] input:radio').change(function () {
        $(this).parent().removeClass('section-error');
    });
});

.section-error {
	background-color: rgb(255, 218, 218);
	border: 1px solid red;
}

<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.11.1/jquery.min.js"></script>
<div id="bolton-1">
    <p>bolton-1</p>
    <input type="radio" name="data[ObeBolton][1]" value="1" />
    <input type="radio" name="data[ObeBolton][1]" value="2" /> 
</div>

<div id="bolton-2">
    <p>bolton-2</p>
    <input type="radio" name="data[ObeBolton][2]" value="3" />
    <input type="radio" name="data[ObeBolton][2]" value="4" /> 
</div>

<div id="bolton-3">
    <p>bolton-3</p>
    <input type="radio" name="data[ObeBolton][3]" value="5" />
    <input type="radio" name="data[ObeBolton][3]" value="6" /> 
</div>
<button class="checkBoltons">Check Boltons</button>

edited Apr 23, 2015 at 9:32

answered Apr 22, 2015 at 11:18

Shlomi Hassid

6,6063 gold badges30 silver badges50 bronze badges

3 Comments

iCollect.it Ltd Over a year ago

You missed a couple of shortcuts (using toggleClass with a second boolean parameter, and a simple each() on elements directly), but this is an equally valid improvement to the original mess. +1

Shlomi Hassid Over a year ago

@TrueBlueAussie Thank you - I know it can be made allot more clean Exactly as the accepted answer is - I tried to give a more simpler to understand for new developers - also my goal was not changing the HTML structure.

iCollect.it Ltd Over a year ago

I applaud your keeping the DOM unchanged. That was appropriate (and was in the answer I threw away). elements.each() is actually simpler in this case for newbies. Side-note: Grammatically you do not prefix "simpler" with "more" as it is already an emphasised version of "simple".

Collectives™ on Stack Overflow

jquery function is not working on loop

2 Answers 2

1 Comment

3 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

1 Comment

3 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related