3
votes

How can I change the cell background color based on the cell's content in an onEdit() function?

I've had many versions of code that I tested for this - some working almost right, some not working at all. But I have yet to get this to work the way I need it to.

Please forgive the lack of elegance in the way that this is written, but I actually need to keep the code as straightforward as possible since there will be many cell changes, many conditionals, and many differing numbers of cells that will be changed depending on what gets changed on the worksheet.

Ok, so here goes...

function onEdit(event)
{
 var ss = SpreadsheetApp.getActiveSpreadsheet().getSheetByName("Sheet2");   
 var changedCell= event.source.getActiveRange().getA1Notation();

 if (changedCell == 'B3') {
 var c = ss.getRange("B3").getValue();
  if (c < 2); {
  ss.getRange("B3").setBackgroundColor('#ff0000');  
  ss.getRange("B12").setBackgroundColor('#ff0000'); 
  }
  if (c > 1); {
  ss.getRange("B3").setBackgroundColor('#000000');  
  ss.getRange("B12").setBackgroundColor('#000000'); 
  }
 }
}
2

2 Answers

6
votes

A few things to note

1.The name of the method is setBackground and not setBackgroundColor

2.How is the cell B3 formatted. The comparison works only if it is formatted as an integer. In most cases, Google Spreadsheet automatically formats the cells based on the data type, but if I'm writing code, I'd double check. So use something like

 var c = parseInt(ss.getRange("B3").getValue()) ;

3.The semicolon is not needed after the if condition. That will terminate the if condition immediately. So use

if (c < 2) {

and

if (c > 1) {

4.Finally, I don't know how data comes into B3, but if you have 1.5 in B3, then both the if conditions become true and your background color is overwritten. So, I suggest that you use a if..elseif

For better readability, I'd use setBackground('red') and setBackground('white') for the common colours.

4
votes

In addition to the advice from Srik, you should consider the efficiency of your onEdit() function. The general idea is to determine whether you should bail out as soon and as cheaply as possible, and then optimize the rest of the code according to the Best Practices (minimize Service calls, mainly).

It looks like you want the onEdit() to perform only on "Sheet2", but the way you have written it, it will trigger for a change on any sheet, but mess around with colors on "Sheet2". (Not a visible problem, since the conditions on "Sheet2" are the ones being used to decide on coloring, but you'll be burning up script execution time needlessly.) By using the event info to figure out what sheet has been updated, and exiting if it's not "Sheet2", the cost of the onEdit() drops.

I agree with Srik about the if statements... I just can't tell what you wanted. I don't think the suggestion of else if alone will solve it, though, because it would treat c > 1 as c > 2... if you wanted that, why not write it? Maybe you wanted no color for 1 < c < 2, with red for c < 1 and black for c > 2? You should sort out the logic in that part.

Here's how an optimized onEdit() would look, making maximum use of the event info:

function onEdit(event)
{
  var ss = event.range.getSheet();
  if (ss.getName() !== "Sheet2") return; // Get out quickly
  var changedCell = event.source.getActiveRange();
  var changedCellA1 = changedCell.getA1Notation();
  if (changedCellA1 !== 'B3') return;

  var c = event.value;  // We know we edited cell B3, just get the value
  var background = 'white'; // Assume 1 <= c <= 2
  if (c > 2) {
    background = 'red';
  }
  else if (c < 1) {
    background = 'black'; 
  }
  changedCell.setBackground(background);
  ss.getRange("B12").setBackground(background);
}