1

Here is my code below. I am attempting to reverse the click that was already made. This is happening with a td tag. I have combined failed experiments from several questions here and other forums.

function changeColor(elem) {
    if (elem.style.backgroundColor = "#5AD9C1" || "transparent") {
        elem.style.backgroundColor = "#66FF66";
    } else if (element.style.backgroundColor = "#66FF66") {
        elem.style.backgroundColor = "#5AD9C1";
    }
};
phantom
  • 1,457
  • 7
  • 15

4 Answers4

5

First:

// With '=' you do assign a value to backgroundColor
if (elem.style.backgroundColor = "#5AD9C1" ...)

// Use '==' to check, if a equals b
if(elem.style.backgroundColor == "#5AD9C1" ...)

Second:

You can't chain if-statements like this:

if(myVar == 'A' || 'B' || 'C')

It's the same as asking if('B') which is always true

You have to do it like this:

if(myVar == 'A' || myVar == 'B' || myVAR == 'C')

For detailed information about if-statements and operators see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/if...else

The correct solution is:

function changeColor(elem) {
    if (elem.style.backgroundColor == "#5AD9C1" || elem.style.backgroundColor == "transparent") {
        elem.style.backgroundColor = "#66FF66";
    } else if (element.style.backgroundColor == "#66FF66") {
        elem.style.backgroundColor = "#5AD9C1";
    }
}

edit:

As mentioned in the comments, the main reason why this does not work is because style.backgroundColor is returning the color as RGB value

I found this solution for converting rgb to hex.

Community
  • 1
  • 1
kair
  • 946
  • 1
  • 10
  • 16
  • I see what you are saying, I did not understand this before, and I looked into the link provided. The same problem occurs accounting for the discrepancy with the equal signs. – user3510681 Nov 03 '14 at 01:38
  • 1
    The reason why this doesn't work is because when you get backgroundColor the native api gives you back a string in the rgb format "rgb(102, 100, 208)" as an example and not in HEX like you are expecting. I would still highly suggest using classes and giving these states a more meaningful value, as per my answer below. – TysonWolker Nov 03 '14 at 05:51
  • I edited my answer and added a link for a rgb to hex solution. I totaly forgot this behaviour since I do only use jQuery nowadays :) – kair Nov 03 '14 at 15:43
2

You should use a class so that it can be maintained from a stylesheet.

.state2 {
   background-color: #66FF66;
}
.state1{
   background-color: #5AD9C1;
}

Possibly one of the states may be redundant and should be applied to the base element allowing you to toggle the class instead.

If you have jQuery available use the following:

if($element.hasClass('state1')) {
   $element.removeClass('state1').addClass('state2');
else{
   $element.removeClass('state2').addClass('state1');
}

The above can be improved quite a bit especially if there was some example HTML.

If you don't have the luxury of using jQuery you can look at alternatives or use some of these replacements: http://toddmotto.com/creating-jquery-style-functions-in-javascript-hasclass-addclass-removeclass-toggleclass/

EDIT: I have added an answer that solves your problem. Although I still don't recommend doing this: http://jsfiddle.net/po16f5ec/4/

I also referenced this article for the hex to rgb: RGB to Hex and Hex to RGB

Community
  • 1
  • 1
TysonWolker
  • 436
  • 2
  • 9
2

Many browsers, certainly Chrome and Firefox (from experience) return their colors in rgb() format, rather than hexadecimal; regardless of the format in which they were supplied (#fff, white, hsl(0,100%,100%) all return rgb(255,255,255)).

That said, if you use css class-names then you don't need to worry about toggling between specific colours, or how to compensate for the vagaries in how specific browsers return those colours. In plain JavaScript, for example it's quite simple to implement a class-change function that achieves the same end-result:

function toggleClass(elem, cssClassOn) {
    var test = elem.classList.contains(cssClassOn);
    elem.classList[ test ? 'remove' : 'add' ](cssClassOn);
}

document.getElementById('test').addEventListener('click', function (e) {
    toggleClass(e.target, 'on');
});

JS Fiddle demo.

The above, of course, needs to be coupled with the appropriate CSS styling.

For those browsers that don't implement the Element.classList API, a simple alternative is:

function toggleClass(elem, cssClassOn) {
    var currentClasses = elem.className,
        test = currentClasses.indexOf(cssClassOn) > -1;

    if (test) {
        elem.className = currentClasses.replace(cssClassOn,'');
    }
    else {
        elem.className += currentClasses + ' ' + cssClassOn;
    }
}

JS Fiddle demo.

References:

David Thomas
  • 249,100
  • 51
  • 377
  • 410
0

Be careful with = and ==

function changeColor(elem) {
  if (elem.style.backgroundColor == "#5AD9C1" || elem.style.backgroundColor == "transparent") {
    elem.style.backgroundColor = "#66FF66";
  } else if (element.style.backgroundColor == "#66FF66") {
    elem.style.backgroundColor = "#5AD9C1";
  }
user1563056
  • 101
  • 3