style.backgroundColor doesn't work?

Question

Tried to change the color of a cell by clicking on it.

The cells are normaly grey and by the first click they should turn red. When I click on a red cell it should turn grey again.

function changeColor(cell) {
  var red = '#FE2E2E';
  var grey = '#E6E6E6';

  if (cell.style.backgroundColor == grey) {
    cell.style.backgroundColor = red;
  } else {
    if (cell.style.backgroundColor == red) {
      cell.style.backgroundColor = grey;
    }
  };
};

#table tr td {
  width: 20px;
  height: 50px;
  cursor: pointer;
  background-color: #E6E6E6;
  border: 1px solid black;
}

<table class="table table-bordered" id="table">
  <tbody>
    <tr>
      <td onclick="changeColor(this)"></td>
      <td onclick="changeColor(this)"></td>
      <td onclick="changeColor(this)"></td>
      <td onclick="changeColor(this)"></td>
      <td onclick="changeColor(this)"></td>
    </tr>
  </tbody>
</table>

I also tried with .style.bgColor, rgb and if (cell.style.backgroundColor === but this didn't work either. The value of the cells background color is either by .backgroundColor : '' or by .bgColor : undefined.

One of the key things in programming is using a *debugger*. With a debugger, you'll be able to see things like the current values of variables, etc. If you use the powerful debugger built into your browser, you'll see why this code isn't working. — T.J. Crowder, Oct 04 '16 at 12:23
cell.style.backgroundColor == red is the problem because undefined is not equal to red. Still red should be in quotes. — Taiti, Oct 04 '16 at 12:25
Using and checking for classes is much nicer IMO than checking for and adding individual style properties. — , Oct 04 '16 at 12:28
Browsers do not all return HEX codes when you read color properties. You should not have the if inside the else. — epascarello, Oct 04 '16 at 12:29
No inline style is set to begin with, so none of the conditions are being met. You'll probably need to use getComputedStyle instead, or use classes, which is the recommended solution. — Robin Neal, Oct 04 '16 at 12:32

score 5 · Answer 1 · answered Oct 04 '16 at 12:31

Your code isn't working because the style property does not have a backgroundColor set when your code first runs: style represents an element's inline style attribute, and your elements do not have one to start. When you check to see if the element's background is red or gray, it's neither since it has no inline style (style.backgroundColor is actually the empty string).

You have a few options:

Use getComputedStyle to see what the element's background-color really is, regardless if it's set inline or not.
Provide a default case that will set your element's background-color regardless of whether or not it's already been set. (If it's red, switch it to gray; otherwise, set it to red.)

Either would work and do what you need it to; it depends on how flexible you need to be in your solution, and I'll leave that up to you.

score 5 · Accepted Answer · answered Oct 04 '16 at 12:35

The value you get back from style.backgroundColor isn't likely to be in the same format you set it in; it's in whatever format the browser wants to make it.

One minimal change approach is to store a flag on the element (see comments):

function changeColor(cell) {
  var red = '#FE2E2E';
  var grey = '#E6E6E6';
  
  // Get a flag; will be falsy if not there at all
  var flag = cell.getAttribute("data-grey");

  if (!flag) {
    // Make it grey
    cell.setAttribute("data-grey", "true");
    cell.style.backgroundColor = red;
  } else {
    // Not grey, make it red
    cell.setAttribute("data-grey", ""); // blank is falsy
    cell.style.backgroundColor = grey;
  }
}

#table tr td {
  width: 20px;
  height: 50px;
  cursor: pointer;
  background-color: #E6E6E6;
  border: 1px solid black;
}

<table class="table table-bordered" id="table">
  <tbody>
    <tr>
      <td onclick="changeColor(this)"></td>
      <td onclick="changeColor(this)"></td>
      <td onclick="changeColor(this)"></td>
      <td onclick="changeColor(this)"></td>
      <td onclick="changeColor(this)"></td>
    </tr>
  </tbody>
</table>

...but the right thing to do is add/remove classes and use CSS to style accordingly (see comments):

// See https://developer.mozilla.org/en-US/docs/Web/API/Element/classList for classList info
function changeColor(cell) {
  // adds or removes the active class
  cell.classList.toggle("active");
}

#table tr td {
  width: 20px;
  height: 50px;
  cursor: pointer;
  background-color: #E6E6E6;
  border: 1px solid black;
}
/* A class we can toggle to override the color above */
#table tr td.active {
  background-color: #fe2e2e;
}

<table class="table table-bordered" id="table">
  <tbody>
    <tr>
      <td onclick="changeColor(this)"></td>
      <td onclick="changeColor(this)"></td>
      <td onclick="changeColor(this)"></td>
      <td onclick="changeColor(this)"></td>
      <td onclick="changeColor(this)"></td>
    </tr>
  </tbody>
</table>

score 1 · Answer 3 · edited May 23 '17 at 10:30

So this is how your if/else should be written, you should not have a check inside of your else. Now the console.log will show why your code fails. The backgroundColor check will return rgb in most browsers and not the hex.

...This does not work on purpose...

function changeColor(cell) { 
  var red = '#FE2E2E';
  var grey = '#E6E6E6';
  console.log(cell.style.backgroundColor);  //rgb(230, 230, 230)
  if (cell.style.backgroundColor == grey) {
    cell.style.backgroundColor = red;
  } else {
      cell.style.backgroundColor = grey; 
  }
};

#table tr td {
  width: 20px;
  height: 50px;
  cursor: pointer;
  background-color: #E6E6E6;
  border: 1px solid black;
}

<table class="table table-bordered" id="table">
  <tbody>
    <tr>
      <td onclick="changeColor(this)"></td>
      <td onclick="changeColor(this)"></td>
      <td onclick="changeColor(this)"></td>
      <td onclick="changeColor(this)"></td>
      <td onclick="changeColor(this)"></td>
    </tr>
  </tbody>
</table>

So what can you do? Use a function like RGB to Hex and Hex to RGB to convert the hex/rgb, Or you can use rgb instead and make it work in most browsers rgb(230, 230, 230) and rgb(254, 46, 46 OR the easy thing with a lot less code is to use a class and do not worry about colors at all.

function changeColor(cell) { console.log(cell);
  cell.classList.toggle("selected");
};

#table tr td {
  width: 20px;
  height: 50px;
  cursor: pointer;
  background-color: #E6E6E6;
  border: 1px solid black;
}

#table tr td.selected {
  background-color: #FE2E2E;
}

<table class="table table-bordered" id="table">
  <tbody>
    <tr>
      <td onclick="changeColor(this)"></td>
      <td onclick="changeColor(this)"></td>
      <td onclick="changeColor(this)"></td>
      <td onclick="changeColor(this)"></td>
      <td onclick="changeColor(this)"></td>
    </tr>
  </tbody>
</table>

score 0 · Answer 4 · answered May 06 '20 at 09:17

now 2020, and i find the really solution:

function changeColor(cell) {
  var red = "rgba(255, 4, 10, 1)"; //rgba or rgb, so if you really want to use this, you need use regexp. 
  var grey = "rgba(230, 230, 230, 1)"; //pls change your css to this too.


  var style = cell.computedStyleMap().get('background-color').toString();

  if (style == grey) {
    cell.style.backgroundColor = red;
  } else  if (style == red) {
    cell.style.backgroundColor = grey; 
  };

for chrome this is ok, but, maybe other browser with other color format, you need test it.

score -2 · Answer 5 · answered Oct 04 '16 at 12:39

-2

You need quotation marks here.

function changeColor(cell) {

  if (cell.style.backgroundColor == "grey") {
    cell.style.backgroundColor = "red";
  } else if (cell.style.backgroundColor == "red") {
    cell.style.backgroundColor = "grey";
  };
};

(I also took the liberty to replace the if in the else block and replaced it with a else if)

answered Oct 04 '16 at 12:39

Joel

17
1
5

2

The quotation isn't the problem, because `red` and `grey` are variables. – Marco Oct 04 '16 at 12:44
Yes that is right. I forgot to mention that it works with quotation if you remove the variables. Then your js will compare Strings and then set the value to a string instead of a hex number. I tried this and it worked. Naturally only with colors that can be named. – Joel Oct 04 '16 at 12:52

style.backgroundColor doesn't work?

5 Answers5

Linked

Related