Trav McKinney
Trav McKinney

Reputation: 998

jQuery click function acting strange when adding/removing classes

I have two list items that, when clicked, should change classes from '.off' to '.on'. Only one element should be '.on' at a time so when one is already turned on and the other is clicked both elements should change classes from '.off' to '.on' and vice versa. If a list item with a class of '.on' is clicked it should change classes to '.off'

The problem I am having is when a list item with class '.on' is clicked it still runs the click function as if it had a class of '.off'

My html:

<ul>
    <li><a href="about" class="off">ABOUT</a></li>
    <li><a href="upload" class="off">SUBMIT</a></li>
</ul>

My javascript (running on jQuery 1.7.1)

$('.off').click(function(event) {
    event.preventDefault();
    $(".on").addClass("off").removeClass("on");
    $(this).addClass("on").removeClass("off");
});
$('.on').click(function(event) {
    event.preventDefault();
    $(this).addClass("off").removeClass("on");
});

Does anyone know what is going on here? Is there something wrong in my code or have I encountered some sort of bug here?

http://jsfiddle.net/ZC3CW/6/

Upvotes: 3

Views: 1116

Answers (5)

Matt
Matt

Reputation: 75317

The selectors you're using to bind the event using click() are used to select the elements to add the event handler to. The selector is not considered when the handler is run.

You should be looking for something more like this:

$('li').click(function(event) {
    event.preventDefault();

    if ($(this).hasClass('off')) {
        $(".on").addClass("off").removeClass("on");
        $(this).addClass("on").removeClass("off");
    } else { // $(this).hasClass('on');
        $(this).addClass("off").removeClass("on");
    }
});

You might want to make the li selector more explicit by adding a class/id to the ul or li's.

To confuse things further, you could also do this (if you're using jQuery > 1.7);

$(document).on('click', '.off', function(event) {
    event.preventDefault();
    $(".on").addClass("off").removeClass("on");
    $(this).addClass("on").removeClass("off");
});
$(document).on('click', '.on', function(event) {
    event.preventDefault();
    $(this).addClass("off").removeClass("on");
});

This is because the .on() function works by attaching the event handler to the selected elements (document), and will only execute the handler (the function) on the event specified (click) if the element that the event originated from matches the selector .off at the time the event fired, not at binding time.

Upvotes: 4

jlue
jlue

Reputation: 35

The click is bound to the element not the class. Maybe you can attach the events to the

  • elements and detect/toggle the elements classes:

    $('li').click(function(event) {
        event.preventDefault();
        if( $(this).hasClass('on') ) {
            $(this).removeClass('on');
        }
        else {
            $(this).addClass('on');
            $(this).siblings().removeClass('on');
        }
    });
    

    Upvotes: 0

  • musefan
    musefan

    Reputation: 48415

    I would suggest adding a click handle to a different selector, this should work for you...

    $("ul li a").click(function(e){
       e.preventDefault();
    
       if($(this).hasClass("off")){
          $("ul li a").addClass("off").removeClass("on");
          $(this).addClass("on").removeClass("off");
       }
       else{
          $(this).addClass("off").removeClass("on");
       }
    });
    

    Upvotes: 2

    pete
    pete

    Reputation: 25081

    The problem as I see it is that your "on" class is not in play at the time of the click event, so your $('.on').click method is never being called.

    Try re-assigning your events after changing classes (example follows) :

    var assignClicks = function () {
        $('.off').click(function(event) {
            event.preventDefault();
            $(".on").addClass("off").removeClass("on");
            $(this).addClass("on").removeClass("off");
            assignClicks();
        });
        $('.on').click(function(event) {
            event.preventDefault();
            $(this).addClass("off").removeClass("on");
            assignClicks();
        });
    };
    assignClicks();
    

    Hope this helps,

    Pete

    Upvotes: 0

    kolufild
    kolufild

    Reputation: 732

    The problem is that jQuery handlers get attached at page load and remain the same regardless of changing their classes. Use live('click', handler) on('click', handler) instead of click().

    Edit: just noticed that .live() is deprecated in jQuery 1.7.

    Upvotes: 1

    Related Questions