atalantus
atalantus

Reputation: 806

style.backgroundColor doesn't work?

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.

Upvotes: 6

Views: 21711

Answers (5)

defend orca
defend orca

Reputation: 743

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.

Upvotes: 0

epascarello
epascarello

Reputation: 207501

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>

Upvotes: 1

Joel
Joel

Reputation: 17

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)

Upvotes: -2

T.J. Crowder
T.J. Crowder

Reputation: 1074268

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>

Upvotes: 5

ajm
ajm

Reputation: 20105

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.

Upvotes: 5

Related Questions