CODEWITHSUNDEEP
javascriptjqueryjquery-selectors
cmplieger
cmplieger

Reputation: 7351

jquery selector problem

I'm using this code:

$('fieldset input[type=checkbox]').each(function () {if($(this).attr('checked','checked')){
    var switches = $(this).parent().find('.switch');
    $(switches).attr('state','on')
    $(switches).css({'left':"52px"});
    $(switches).parent().css({'background-position': "147px -37px"});
}})

But somehow it sets all my checkboxes to checked="checked" Am I just stupid or is something else in the code interfering?

Thanks for your help :)

EDIT: here is the HTML

<fieldset>
        <input checked="checked" />
        <label></label>
        <div class="toggle_box">
            <div class="switch" state="on"></div>
        </div>
    </fieldset>
    <fieldset>
        <input/>
        <label></label>
        <div class="toggle_box">
            <div class="switch"></div>
        </div>
    </fieldset>

Upvotes: 0

Views: 128

Answers (7)

BoltClock
BoltClock

Reputation: 723618

You're passing the checked value to attr() as the second argument. That causes it to be set. What's then returned is the jQuery object with that input, which is always a truthy value.

Your if condition should look like this (use the :checked pseudo-class instead):

if ($(this).is(':checked')) {

On a side note, your inner code can be refactored to this:

$(this).parent().find('.switch')
    .attr('state', 'on')
    .css({ 'left': "52px" })
    .parent()
        .css({ 'background-position': "147px -37px" });

If you have to use a cached variable (i.e. switches), you can, but don't wrap it in the jQuery function. It's a jQuery object itself, so needs not be wrapped.

Upvotes: 1

Kyle Sletten
Kyle Sletten

Reputation: 5413

There is actually a selector specifically for the checked boxes.

$('fieldset input[type=checkbox]:checked').each(function(){
    var switches = $(this).parent().find('.switch');
    $(switches).attr('state','on')
    $(switches).css({'left':'52px'});
    $(switches).parent().css({'background-position':'147px -37px'});
});

If you use that you won't have to do any of that logic by yourself.

Upvotes: 0

Joe Enzminger
Joe Enzminger

Reputation: 11190

$('fieldset input[type=checkbox]').each(function ()  {if($(this).attr('checked') == 'checked')){
var switches = $(this).parent().find('.switch');
$(switches).attr('state','on')
$(switches).css({'left':"52px"});
$(switches).parent().css({'background-position': "147px -37px"});
}})

Upvotes: 0

tyronegcarter
tyronegcarter

Reputation: 3956

The statement in the if statement is setting your checkboxes to checked. Particularly, this statement: $(this).attr('checked','checked').

Instead you can do if($(this).prop('checked'))

Read more about the jQuery Prop method.

Alternatively, you can do, this.checked which will access the DOM directly or with jQuery, $(this).is(':checked').

Upvotes: 1

Alexander Kahoun
Alexander Kahoun

Reputation: 2488

At first glance I see you're using .is('checked') when I beleive you want .is(':checked') The preceding colon being the difference.

UPDATE: I see you updated the code. The line

if ($(this).attr('checked', 'checked'))

is actually setting all 'input:checkbox' elements to checked. It should be:

if ($(this).is(':checked'))

Upvotes: 0

Tyler
Tyler

Reputation: 2126

if($(this).attr('checked','checked')) would be your problem. You're assigning checked to each checkbox instead of checking if it's checked.

Upvotes: 0

alex
alex

Reputation: 490243

This piece of code is doing it...

$(this).attr('checked','checked')

...which returns the set in order for the cascade to work. An Object is always truthy in JavaScript.

Try...

this.checked

...which returns whether the checkbox is checked or not. If you wanted to do it the jQuery way, use $(this).is(':checked'). I wouldn't though, it's more verbose and less performant.

Upvotes: 2

Related Questions