jQuery: Can't remove class

Question

I made a like / dislike script that works like this: 2 buttons with rate_deal class. When you click the like or dislike button it removes the rate_deal class from both buttons to prevent clicking again and then adds some classes to look better.

I have a problem. I don't know what I did wrong but I can't delete the rate_deal class for both buttons when I click on a button.

Here is the code:

HTML:

<div class="content_r_d_left_rate" id="deal-1">
<span class="rate_deal content_r_d_l_rate_img_up"></span>
<span class="rate_deal content_r_d_l_rate_img_down"></span>
</div>

jQuery:

$(".rate_deal").click(function(){

        var dealIdRaw = $(this).parent().attr('id');
        var dealId = dealIdRaw.replace('deal-', '');

        $('#'+dealIdRaw).children().removeClass('rate_deal');

        if($(this).hasClass('content_r_d_l_rate_img_up'))
        {
            $('#'+dealIdRaw+' > .content_r_d_l_rate_img_down').addClass('content_r_d_l_rate_img_down_i');
            $('#'+dealIdRaw+' > .content_r_d_l_rate_img_down').removeClass('content_r_d_l_rate_img_down');
        }
        else
        {
            $('#'+dealIdRaw+' > .content_r_d_l_rate_img_up').addClass('content_r_d_l_rate_img_up_i');
            $('#'+dealIdRaw+' > .content_r_d_l_rate_img_up').removeClass('content_r_d_l_rate_img_up');
        }
    });

Answer 1

You have a structural problem with the way you've written your code. When you do this:

$(".rate_deal").click(function(){

That locks in those DOM objects for click handlers from now on, regardless of whether they continue to have the rate_deal class attached to them. So, structurally, your click handler is NOT removed when you remove the class.

You can solve that problem by using delegated event handling like this:

$(".content_r_d_left_rate").on("click", ".rate_deal", function(){

This will then only fire the click handler when the ".rate_deal" class is on the clicked upon object.

This block:

    var dealIdRaw = $(this).parent().attr('id');
    var dealId = dealIdRaw.replace('deal-', '');

    $('#'+dealIdRaw).children().removeClass('rate_deal');

can also be replaced with any one of these options:

    $(this).siblings().add(this).removeClass("rate_deal");

    $(this).parent().find(".rate_deal").removeClass("rate_deal");

    $(this).closest(".content_r_d_left_rate").find(".rate_deal").removeClass("rate_deal");

    $(this).parent.children().removeClass("rate_deal");

I prefer the 3rd variation because it's the most resistant to breaking if you slightly modify your HTML.

The whole block of simplified code would be this:

$(".content_r_d_left_rate").on("click", ".rate_deal", function(){

    // get our common parent
    var parent = $(this).closest(".content_r_d_left_rate");

    // remove existing .rate_deal classes in this block    
    // so no more clicks get processed
    parent.find(".rate_deal").removeClass("rate_deal");

    // toggle classes based on what was clicked
    if($(this).hasClass("content_r_d_l_rate_img_up")) {
        parent.find(".content_r_d_l_rate_img_down")
            .addClass("content_r_d_l_rate_img_down_i")
            .removeClass("content_r_d_l_rate_img_down");
    } else {
        parent.find(".content_r_d_l_rate_img_up")
            .addClass("content_r_d_l_rate_img_up_i")
            .removeClass("content_r_d_l_rate_img_up");
    }
});