CODEWITHSUNDEEP
jquery
Gordon Younge
Gordon Younge

Reputation: 23

JQuery click+this not working

I have copied a snippet of code over. I am very new to web languages and come from a java background.

JQuery seems great but I just can't figure out how to solve this.

http://jsfiddle.net/MMf9Q/2/

thank you.

Upvotes: 0

Views: 112

Answers (8)

Muhammad Raheel
Muhammad Raheel

Reputation: 19882

It should be like this

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

    var color = $(this).css('background-color');

    if(color == '#cc0000'){
        $(this).css('background-color', '#00CC00');
    }else{
        $(this).css('background-color', '#CC0000');
    }
})​;

Upvotes: 0

Answers21241
Answers21241

Reputation: 76

I've updated your fiddle to work as I think you are expecting it.

I removed the if/else statement and simple used jQuery toggle ability(http://api.jquery.com/toggle/). Made a class that I want the object to transform to. I find this keeps the code much cleaner and organized.

Fixed code: http://jsfiddle.net/MMf9Q/6/

jQuery:

    $('#clickme').click(function(){
        $(this).toggleClass('red');
    })

Upvotes: 6

SadullahCeran
SadullahCeran

Reputation: 2415

Answers21241 and iambriansreed have lovely answers which gives what you tried to accomplish.

But for your code, I would like to give couple of advices for start.

First of all, here is your original javascript code:

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

    var color = this.css('background');

    if(color == '#cc0000'){
        this.css('background', '#00CC00');
    }else{
        this.css('background', '#CC0000');
    }
})​

1) You have to wrap your code in $(function(){}); in order for jquery to execute it after page load. Otherwise, your javascript code will be executed before DOM is fully loaded, and it will simply not work for most cases:

$(function(){
    $('#clickme').click(function(){
        var color = this.css('background-color');
                    alert(color);
        if(color == '#CC0000'){
            this.css('background-color', '#00CC00');
        }else{
            this.css('background-color', '#CC0000');
        }
    });
});​

2) this.css() will not work. .css() is a function of a jquery object, and this is not a jquery object in that case. You must wrap it with $() to construct a jquery object from any dom element:

$(function(){
    $('#clickme').click(function(){
        var color = $(this).css('background-color');
        if(color == '#CC0000'){
            $(this).css('background-color', '#00CC00');
        }else{
            $(this).css('background-color', '#CC0000');
        }
    });
});​

3) color value may be something different than what you gave. Browsers convert hex color values to RGB values mostly, and what you get will differ. Try alert(color); at a point to get value, and change your code accordingly.

$(function(){
    $('#clickme').click(function(){
        var color = $(this).css('background-color');
                    alert(color);
        if(color == '#CC0000' || color == 'rgb(204, 0, 0)'){
            $(this).css('background-color', '#00CC00');
        }else{
            $(this).css('background-color', '#CC0000');
        }
    });
});​

Here is final fiddle:

This will help you to understand what was wrong. Besides, the way is not the best practice. Use toggleClass and style your elements within css. Answers21241 and iambriansreed have given good ways of doing that.

Upvotes: 1

veeTrain
veeTrain

Reputation: 2915

Gordon, your code had a couple of problems with it. The first is that you need to wrap this in $(). That represents the #clickme element.

Secondly, your request for background is a lot more detailed than what you were expecting. You could alert or console.log what is being returned to find out why your if statements are not behaving like you were expecting. As seen here.

Update: Here is the updated approach:

$('#clickme').click(function() {
    var color = $(this).css('backgroundColor');
    if(color == "rgb(0, 204, 0)") {
        $(this).css('backgroundColor', '#CC0000');
    }else{
        $(this).css('backgroundColor', '#00CC00');
    }
});​

Upvotes: 0

Muhammad Raheel
Muhammad Raheel

Reputation: 19882

Use $(this) instead of this. It represent the selector.

Upvotes: 0

LOLapalooza
LOLapalooza

Reputation: 2082

As others have said, it should be $(this) instead of this. I would also recommend saving $(this) to another variable for performance purposes. It's probably not necessary to do here, but it's a good habit to get in to, especially if you're looping through something. Something like this: 

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

    var clickedElement = $(this);
    var color = clickedElement.css('background');

    if(color == '#cc0000'){
        clickedElement.css('background', '#00CC00');
    }else{
        clickedElement.css('background', '#CC0000');
    }
})​

Upvotes: 0

iambriansreed
iambriansreed

Reputation: 22271

Fiddled

http://jsfiddle.net/nTXPC/

CSS

.red {
 background:#CC0000;
}
.green {
 background:#00CC00;
}

HTML

<div id="clickme" class="red"> Click me </div>​

JS

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

   $(this).toggleClass('red green');

});​

Upvotes: 0

nanobar
nanobar

Reputation: 66475

Use $(this) to access the this element as a JQuery object

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

    var color = $(this).css('background-color');

    if(color == '#cc0000'){
        $(this).css('background', '#00CC00');
    }else{
        $(this).css('background', '#CC0000');
    }
})

Upvotes: 0

Related Questions