Rendering a person's time clock record in HTML, with many color-coded conditions

I have coded this mess of JavaScript and a bit of jQuery. Everything here works perfect as intended, however it is a real mess. What would be the best way to clean this up with best practices and such?

```
$.getJSON("People.json",
function(data) {
$.each(data.People, function(i, PersonObj) {
var Person = PersonObj[Object.keys(PersonObj)[0]];

content = '';
content += '';
content += '' + Person.Op + ' ';

// if the person has swiped off but not back on and one hour has elapsed
if (Person.DifHours > 1 && Person.ON === false) {
content += '' + Person.Name;
} else if (Person.OUT !== false) {
content += '' + Person.Name;
// If the person has swiped back on from dinner set to lime
} else if (Person.ON !== false) {
content += '' + Person.Name;
// If the person has swipped of for dinner but 1 hour has not elapsed set to yellow
} else if (Person.OFF !== false) {
content += '' + Person.Name;
} else if (Person.Clock !== false) {
// If the person has clocked on set to lime
content += '' + Person.Name;
} else {
content += '' + Person.Name;
}
// putting values in to the table without formatting

content += '' + Person.WorkHours + ' ';
content += '' + Person.Start + ' ';
content += '' + Person.End + ' ';

// if the person has not clocked in set the text to white
if (Person.Clock === false) {
content += '' + Person.Clock;
}
// if the person is late by 10 mins set to cyan
else if (Person.Late > 10) {
content += '' + Person.Clock;
// if the person is early by 10 mins set to orange
} els

There are a few good practices that will help this code out.
Close the HTML tags that you open - generate valid HTML

Newer browsers will probably render your content properly, but older browsers may not. You may also get strange formatting errors that are hard to debug. Save yourself the headache and always close your tags in the reverse order that you open them.

....

You may also want to put the ` tags outside of your foreach loop, as there should be one for the overall table, not one per row.

You can also use the W3C official HTML validator to make sure that the output you generate is valid HTML, and adjust your JavaScript to match.
Set a variable in the if block; construct the content only once

This makes it easier to update if you later decide to change how the raw HTML is built (like moving from tables to divs and spans).
Avoid CSS shorthands if you are not using all of the properties

Using background: XYZcolor; means that any other CSS background properties (background-image, background-repeat, background-attachment, background-position) already set are being wiped out. If this is your intention, keep it. If not, be more precise with your styles and use only background-color: XYZcolor;. This may not affect you now, but is a good habit to keep to avoid surprises later.
Use CSS classes; avoid using the style= attribute

This allows you to:

• Give meaningful names indicating why the cell is colored a particular way (so you can debug using "view source" in the browser)

• Adjust the styles independently of your JavaScript (so that you don't need to involve a JavaScript developer if the client wants the red cells to be just a bit redder)

• Avoid the problems mentioned in this SO question: What's so bad about in-line CSS?

Combining with the earlier suggestions gives us:

var swipeStatus;
 // if the person has swiped off but not back on and one hour has elapsed  
if (Person.DifHours > 1 && Person.ON === false) {
    swipeStatus = "lateByOneHour";
} else if (Person.OUT !== false) {
    swipeStatus = "notOutYet";
    // If the person has swiped back on from dinner set to lime 
} else if (Person.ON !== false) {
    swipeStatus = "backFromDinner";
    // If the person has swiped of for dinner but 1 hour has not elapsed set to yellow
} else if (Person.OFF !== false) {
    swipeStatus = "outForDinner";
} else if (Person.Clock !== false) {
    // If the person has clocked on set to lime
    swipeStatus = "inNormally";
} else {
    swipeStatus = "defaultStatus";
}
content += '' + Person.Name + '';

With the appropriate CSS:
.lateByOneHour {
background-color: red;
}
.notOutYet {
background-color: red;
}
.backFromDinner {
background-color: lime;
}
.outForDinner {
background-color: yellow;
}
.inNormally {
background-color: lime;
}
.defaultStatus {
background-color: red;
}


A similar strategy can be applied to the other logic blocks of your code.

You can also add multiple CSS classes to a single tag . Sometimes that is useful in simplifying if/else logic, but doesn't appear to be the case here. Good to keep in your back pocket though.
For better performance, call appendTo() outside of the loop

The jQuery site itself has a page about why append() is slow in loops, but the gist of it is that it is expensive to touch the DOM, as append() and appendTo()` do. Appending a giant chunk of HTML (your whole table) once at the end of the loop will perform faster than appending little bits of HTML (just a row) in each iteration of the loop.

StackExchange Code Review Q#60596, answer score: 42