Calendar in HTML + CSS + JS

Question

As I'm new to HTML + CSS + JS I've been building a calendar (which I think anyone can use as a basis) but would like to get your suggestions on how I could improve my code specifically from the perspectives of:

• Adherence to best practices in HTML + CSS + JS.
• Usability.
• Browser compatibility.

Note: The console logging in the JavaScript is only for debugging, not the final version.

var Calendar = function(o) {
  //Store div id
  this.divId = o.ParentID;

  // Days of week, starting on Sunday
  this.DaysOfWeek = o.DaysOfWeek;

  console.log("this.DaysOfWeek == ", this.DaysOfWeek)

  // Months, stating on January
  this.Months = o.Months;

  console.log("this.Months == ", this.Months)

  // Set the current month, year
  var d = new Date();

  console.log("d == ", d)

  this.CurrentMonth = d.getMonth();

  console.log("this.CurrentMonth == ", this.CurrentMonth);

  this.CurrentYear = d.getFullYear();

  console.log("this.CurrentYear == ", this.CurrentYear);

  var f=o.Format;

  console.log("o == ", o);

  console.log("f == ", f);

  //this.f = typeof(f) == 'string' ? f.charAt(0).toUpperCase() : 'M';

  if(typeof(f) == 'string') {
    this.f  = f.charAt(0).toUpperCase();
  } else {
    this.f = 'M';
  }

  console.log("this.f == ", this.f);
};

// Goes to next month
Calendar.prototype.nextMonth = function() {
  console.log("Calendar.prototype.nextMonth = function() {");

  if ( this.CurrentMonth == 11 ) {
    console.log("this.CurrentMonth == ", this.CurrentMonth);

    this.CurrentMonth = 0;

    console.log("this.CurrentMonth == ", this.CurrentMonth);

    console.log("this.CurrentYear == ", this.CurrentYear);

    this.CurrentYear = this.CurrentYear + 1;

    console.log("this.CurrentYear == ", this.CurrentYear);
  } else {
    console.log("this.CurrentMonth == ", this.CurrentMonth);

    this.CurrentMonth = this.CurrentMonth + 1;

    console.log("this.CurrentMonth + 1 == ", this.CurrentMonth);
  }

  this.showCurrent();
};

// Goes to previous month
Calendar.prototype.previousMonth = function() {
  console.log("Calendar.prototype.previousMonth = function() {");

  if ( this.CurrentMonth == 0 ) {
    console.log("this.CurrentMonth == ", this.CurrentMonth);

    this.CurrentMonth = 11;

    console.log("this.CurrentMonth == ", this.CurrentMonth);

    console.log("this.CurrentYear == ", this.CurrentYear);

    this.CurrentYear = this.CurrentYear - 1;

    console.log("this.CurrentYear == ", this.CurrentYear);
  } else {
    console.log("this.CurrentMonth == ", this.CurrentMonth);

    this.CurrentMonth = this.CurrentMonth - 1;

    console.log("this.CurrentMonth - 1 == ", this.CurrentMonth);
  }

  this.showCurrent();
};

// 
Calendar.prototype.previousYear = function() {
  console.log(" ");

  console.log("Calendar.prototype.previousYear = function() {");

  console.log("this.CurrentYear == " + this.CurrentYear);

  this.CurrentYear = this.CurrentYear - 1;

  console.log("this.CurrentYear - 1 i.e. this.CurrentYear == " + this.CurrentYear);

  this.showCurrent();
}

// 
Calendar.prototype.nextYear = function() {
  console.log(" ");

  console.log("Calendar.prototype.nextYear = function() {");

  console.log("this.CurrentYear == " + this.CurrentYear);

  this.CurrentYear = this.CurrentYear + 1;

  console.log("this.CurrentYear - 1 i.e. this.CurrentYear == " + this.CurrentYear);

  this.showCurrent();
}              

// Show current month
Calendar.prototype.showCurrent = function() {
  console.log(" ");

  console.log("Calendar.prototype.showCurrent = function() {");

  console.log("this.CurrentYear == ", this.CurrentYear);

  console.log("this.CurrentMonth == ", this.CurrentMonth);

  this.Calendar(this.CurrentYear, this.CurrentMonth);
};

// Show month (year, month)
Calendar.prototype.Calendar = function(y,m) {
  console.log(" ");

  console.log("Calendar.prototype.Calendar = function(y,m){");

  typeof(y) == 'number' ? this.CurrentYear = y : null;

  console.log("this.CurrentYear == ", this.CurrentYear);

  typeof(y) == 'number' ? this.CurrentMonth = m : null;

  console.log("this.CurrentMonth == ", this.CurrentMonth);

  // 1st day of the selected month
  var firstDayOfCurrentMonth = new Date(y, m, 1).getDay();

  console.log("firstDayOfCurrentMonth == ", firstDayOfCurrentMonth);

  // Last date of the selected month
  var lastDateOfCurrentMonth = new Date(y, m+1, 0).getDate();

  console.log("lastDateOfCurrentMonth == ", lastDateOfCurrentMonth);

  // Last day of the previous month
  console.log("m == ", m);

  var lastDateOfLastMonth = m == 0 ? new Date(y-1, 11, 0).getDate() : new Date(y, m, 0).getDate();

  console.log("lastDateOfLastMonth == ", lastDateOfLastMonth);

  console.log("Print selected month and year.");

  // Write selected month and year. This HTML goes into <div id="year"></div>
  //var yearhtml = '<span class="yearspan">' + y + '</span>';

  // Write selected month and year. This HTML goes into <div id="month"></div>
  //var monthhtml = '<span class="monthspan">' + this.Months[m] + '</span>';

  // Write selected month and year. This HTML goes into <div id="month"></div>
  var monthandyearhtml = '<span id="monthandyearspan">' + this.Months[m] + ' - ' + y + '</span>';

  console.log("monthandyearhtml == " + monthandyearhtml);

  var html = '<table>';

  // Write the header of the days of the week
  html += '<tr>';

  console.log(" ");

  console.log("Write the header of the days of the week");

  for(var i=0; i < 7;i++) {
    console.log("i == ", i);

    console.log("this.DaysOfWeek[i] == ", this.DaysOfWeek[i]);

    html += '<th class="daysheader">' + this.DaysOfWeek[i] + '</th>';
  }

  html += '</tr>';

  console.log("Before conditional operator this.f == ", this.f);

  //this.f = 'X';

  var p = dm = this.f == 'M' ? 1 : firstDayOfCurrentMonth == 0 ? -5 : 2;

  /*var p, dm;

  if(this.f =='M') {
    dm = 1;

    p = dm;
  } else {
    if(firstDayOfCurrentMonth == 0) {
      firstDayOfCurrentMonth == -5;
    } else {
      firstDayOfCurrentMonth == 2;
    }
  }*/

  console.log("After conditional operator");

  console.log("this.f == ", this.f);

  console.log("p == ", p);

  console.log("dm == ", dm);

  console.log("firstDayOfCurrentMonth == ", firstDayOfCurrentMonth);

  var cellvalue;

  for (var d, i=0, z0=0; z0<6; z0++) {
    html += '<tr>';

    console.log("Inside 1st for loop - d == " + d + " | i == " + i + " | z0 == " + z0);

    for (var z0a = 0; z0a < 7; z0a++) {
      console.log("Inside 2nd for loop");

      console.log("z0a == " + z0a);

      d = i + dm - firstDayOfCurrentMonth;

      console.log("d outside if statm == " + d);

      // Dates from prev month
      if (d < 1){
        console.log("d < 1");

        console.log("p before p++ == " + p);

        cellvalue = lastDateOfLastMonth - firstDayOfCurrentMonth + p++;

        console.log("p after p++ == " + p);

        console.log("cellvalue == " + cellvalue);

        html += '<td id="prevmonthdates">' + 
              '<span id="cellvaluespan">' + (cellvalue) + '</span><br/>' + 
              '<ul id="cellvaluelist"><li>apples</li><li>bananas</li><li>pineapples</li></ul>' + 
            '</td>';

      // Dates from next month
      } else if ( d > lastDateOfCurrentMonth){
        console.log("d > lastDateOfCurrentMonth");

        console.log("p before p++ == " + p);

        html += '<td id="nextmonthdates">' + (p++) + '</td>';

        console.log("p after p++ == " + p);

      // Current month dates
      } else {
        html += '<td id="currentmonthdates">' + (d) + '</td>';
        
        console.log("d inside else { == " + d);

        p = 1;

        console.log("p inside } else { == " + p);
      }
      
      if (i % 7 == 6 && d >= lastDateOfCurrentMonth) {
        console.log("INSIDE if (i % 7 == 6 && d >= lastDateOfCurrentMonth) {");

        console.log("i == " + i);

        console.log("d == " + d);

        console.log("z0 == " + z0);

        z0 = 10; // no more rows
      }

      console.log("i before i++ == " + i);

      i++;

      console.log("i after i++ == " + i);            
    }

    html += '</tr>';
  }

  // Closes table
  html += '</table>';

  // Write HTML to the div
  //document.getElementById("year").innerHTML = yearhtml;

  //document.getElementById("month").innerHTML = monthhtml;

  document.getElementById("monthandyear").innerHTML = monthandyearhtml;

  document.getElementById(this.divId).innerHTML = html;
};

// On Load of the window
window.onload = function() {
  
  // Start calendar
  var c = new Calendar({
    ParentID:"divcalendartable",

    DaysOfWeek:[
    'MON',
    'TUE',
    'WED',
    'THU',
    'FRI',
    'SAT',
    'SUN'
    ],

    Months:['Jan', 'Feb', 'Mar', 'Apr', 'May', 'Jun', 'Jul', 'Aug', 'Sep', 'Oct', 'Nov', 'Dec' ],

    Format:'dd/mm/yyyy'
  });

  c.showCurrent();
  
  // Bind next and previous button clicks
  getId('btnPrev').onclick = function(){
    c.previousMonth();
  };

  getId('btnPrevYr').onclick = function(){
    c.previousYear();
  };

  getId('btnNext').onclick = function(){
    c.nextMonth();
  };

  getId('btnNextYr').onclick = function(){
    c.nextYear();
  };                        
}

// Get element by id
function getId(id) {
  return document.getElementById(id);
}

html, body { margin: 0; padding: 0; }

table {
  border-collapse: collapse;
  font-family: Georgia, Times, serif;
}

th {
  border: 1px solid #A8A8A8;
  vertical-align: top;
}

td {
  height: 150px;
  width: 150px;
  padding: 10px;
  border: 1px solid #A8A8A8;
  vertical-align: top;
}

.divcalendar {
  padding: 15px;
    float:left;
    /*background-color: #FFCC00;*/
}

/* Wrapper div. That makes the inner div into an inline element that can be centered with text-align.*/
#calendaroverallcontrols {
  text-align: center;
}

/* This is a fluid div as width will be changing */
#calendarmonthcontrols {
  display: inline-block;
  /*background-color: #FF0000;*/
}

#calendarmonthcontrols > div, #calendarmonthcontrols > a {
    display: inline-block;
}    

#btnPrevYr {
  text-decoration: none;
  font-size: 35px;
  vertical-align: middle;
  /*background: #00FFCC;*/      
}

#btnPrev {
  text-decoration: none;
  font-size: 35px;
  margin-left: 20px;
  vertical-align: middle;
  /*background: #00FFCC;*/
}    

/*.yearspan {
  font-size: 20px;
  font-weight: bold;
  float: left;
  margin-left: 5px;
  margin-right: 5px;
}

.monthspan {
  font-size: 20px;
  font-weight: bold;
  float: left;
  margin-left: 5px;
  margin-right: 5px;
}*/

#monthandyearspan {
  width: 50px;
  font-size: 25px;
  font-weight: bold;
  margin-left: 20px;
  margin-right: 20px;
  vertical-align: middle;
  /*background: #00FFCC;*/
}    

#monthandyear {
  vertical-align: middle;
}

#btnNext {
  text-decoration: none;
  font-size: 35px;
  margin-right: 20px;
  vertical-align: middle;
  /*background: #00FFCC;*/
}

#btnNextYr {
  text-decoration: none;
  font-size: 35px;
  vertical-align: middle;
  /*background: #00FFCC;*/
}        

#divcalendartable {
  clear: both;
}

.daysheader {
  background: #C0C0C0;
  height: auto;
  text-align: center;
}

#prevmonthdates {
  background-color: #E0E0E0;
}

#nextmonthdates {
  background-color: #E0E0E0;
}

#currentmonthdates {
  background-color: #FFFFFF;
}

#cellvaluespan {
  background: #FF0000;
}

#cellvaluelist {
  background: #FFCC00;
}        

.swim {
  font-family: Arial, Helvetica, sans-serif;
  font-size: 80%;
  text-align: center;
  background: #445511;
  color: #F5F5F5;
  margin-bottom: 5px;
  padding: 5px;
}

.chrono {
  font-family: Arial, Helvetica, sans-serif;
  font-size: 80%;
  text-align: center;
  background: #778899;
  color: #F5F5F5;
  margin-bottom: 5px;
  padding: 5px;
}

<div class="divcalendar">

<div id="calendaroverallcontrols">
  <!-- <div id="year"></div> -->

  <div id="calendarmonthcontrols">
    <a id="btnPrevYr" href="#" title="Previous Year"><span><<</span></a>

    <a id="btnPrev" href="#" title="Previous Month"><span><</span></a>

    <!-- <input type="button" src="images/btnprevmonth.png" alt="Submit" id="btnPrev"/>-->

    <!-- <div id="month"></div>-->

    <div id="monthandyear"></div>

    <!--<input type="button" src="images/btnnextmonth.png" alt="Submit" id="btnNext"/>-->

    <a id="btnNext" href="#" title="Next Month"><span>></span></a>

    <a id="btnNextYr" href="#" title="Next Year"><span>>></span></a>      
  </div>
</div>

<div id="divcalendartable"></div>

Answer 1

HTML

The HTML looks mostly fine. Just a few minor stylistic comments:

• Variable names: You use dromedaryCase for some ids, and lowercasenames for others. It’s nice to be consistent. Also, don’t prefix variable names with “div”; it’s redundant info.
• Comments: It’s not obvious to anybody reading the HTML what the point of the divcalendartable div is for, as it appears to be empty. You should add some comments explaining that you’re using JavaScript to populate this div. (I think you should try to do more of the rendering in HTML and just embellish it with JS, but doing so probably requires rethinking the entire calendar.)
• No JS: this will look very kooky in a browser that doesn’t have JS enabled. It’s enough to wrap a message in a <noscript> tag to explain that you need JS enabled for it to work; you just have to do something so that it fails gracefully.

I’ll come back to the HTML when I comment on the JS.

CSS

You can make some improvements here:

• Comments: Your CSS needs more comments. (This is not just you: most CSS has very sparse commenting.) In particular, I’d suggest comments that:

  • Organise your CSS into the different parts of the calendar. e.g. a section for the base table definitions, the buttons, the calendar cells. If you want to change a specific part of the app, this makes it much easier to find the exact style definition you want.
  • Explain why you chose particular CSS rules. In particular, anything that’s non-obvious or was tricky to pin down – this will save you loads of time when you come to debug this later.

• Consolidate: You have lots of selectors with very similar attributes that differ in only a few ways. I’d put multiple selectors on the common rules, with distinct selectors for the few rules where they differ. Not only is this more compact, but it highlights the differences between them.

  For example, for the final pair of rules, write it as:

  .swim, .chrono {
    font-family: Arial, Helvetica, sans-serif;
    font-size: 80%;
    text-align: center;
    color: #F5F5F5;
    margin-bottom: 5px;
    padding: 5px;
  }
  
  .swim {
    background: #445511;
  }
  
  .chrono {
    background: #778899;
  }
  

JS

This is by no means a comprehensive review, but I’ve listed a variety of comments below. But first, two general comments:

• Use better variable names. Single-letter variable names make it really hard to understand what a variable represents, or to follow it through the script. (And it’s a pain to grep for.) Replace all your variable names with something that describes what that variable should store.
• Write more comments. Although the exact code is fairly easy to follow, you could definitely explain what you’re doing a bit more. (And to be clear: explain the why of what the code does, not what the code does.)

Okay, onto some comments on the individual comments:

• Calendar. You should tell us what input this function takes, in particular what attributes the function expects it to have (and what attributes this will have), and what those attributes correspond to.

• nextMonth. This looks fine, but here’s a way you can slightly tidy up the CurrentMonth iterator:

  this.CurrentMonth = (this.currentMonth + 1) % 12;
  

  and then you can remove the else branch from your if statement.

  I would replace incrementing the month directly with a call to nextYear. Although nextYear is a very similar function, it helps to make the intent of the code clear.

  So here’s how I’d rewrite it:

  Calendar.prototype.previousMonth = function() {
      this.CurrentMonth = (this.CurrentMonth + 12) % 12;
  
      // If we have wrapped around to the first month of the next
      // year, then we need to update the year attribute appropriately.
      if (this.CurrentMonth == 0) {
          this.nextYear();
      }
  
      this.showCurrent();
  }
  

  You can obviously do similar for previousMonth. (You may need to move the nextYear and previousYear definitions before these functions.)

• previousYear. You can tidy up the year increments to make them shorter and easier to read:

  this.CurrentYear--;
  

• showCurrent. No comments.

• Calendar.prototype.Calendar. More descriptive input variables.

  At the top of the function, you have a pair of ternary operators. Although that’s not bad per se, since they both have the same condition, I think it’s clearer to write them as an if … else block:

  if (typeof(y) == 'number') {
    this.currentYear = y;
    this.currentMonth = m;
  } else {
    this.currentYear = null;
    this.currentMonth = null;
  }
  

  The ternary operators starting var lastDateOfLastMonth and var p = dm are more complicated and should definitely be broken up. You should also add some comments to explain whatever it is they’re doing, particularly the magic numbers (1, 0, -5 and 2) on the latter.

  The z0* loop variables make these loops very hard to follow.

• window.onload. See previous comments about using better variable names.

  (Also, I suspect that if you initialise your var c = new Calendar() before the HTML, then you can add onclick="c.previousYear()" attributes to your buttons, which makes the getId() stuff entirely redundant. It also makes it easier to see what the buttons do when reading the HTML. That’s a guess though; I haven’t actually tested it.)

Answer 2

I see a few potential issues/possible improvements in your code.

1. Although you noted that you are using the console.logs for debugging, if you ever need to do production logging (for example logging an error so the user can see what it is), you could create a function something like this:

   function myLogger(content) {
       if (window.console && window.console.log) {
           console.log(content);
       }
   }
   

   which will ensure that it won't break your page if the console.log function is not available in the runtime environment.

2. Nested ternary expressions such as this one:

   var p = dm = this.f == 'M' ? 1 : firstDayOfCurrentMonth == 0 ? -5 : 2;
   

   are valid, but I recommend putting parentheses around the "inside" ternary for readability:

   var p = dm = this.f == 'M' ? 1 : (firstDayOfCurrentMonth == 0 ? -5 : 2);
   

3. Lines such as this:

   this.CurrentYear = this.CurrentYear + 1;
   

   can be shortened using ++ for adding one or -- for subtracting one. The line above would look like this with the shorthand:

   this.CurrentYear++;
   

   UPDATE:

   As @ScottSauyet said, this is a bit controversial. I tend to differ from Douglas Crockford on this and use this rule when determining whether using ++/-- is a good idea or not:

   Are you ONLY incrementing/decrementing the value? => Use ++/--.

   for loops (for (var i = 0; i < 10; i++) {...}) are one example that come to mind where ++/-- would be fine.

   Are you incrementing/decrementing the value and then using it on the same line? => Don't use ++/--.

   Example:

   var foo = 1;
   var bar = foo++;
   

   in this case, bar will be 1. Why? This is what happens:

   1. bar is assigned to the value of foo
   2. foo is incremented (but bar isn't)

   but if you use bar = ++foo, bar will be 2. It's confusing, so don't do use ++/-- in assignments.

   If you do have to do something like this use += 1:

   var foo = 1;
   var bar = foo += 1;
   

   which will give the expected result of bar = 2.

   More Information: There is an good SO question which discusses this topic.

Answer 3

There is a small bug in your code. In the 257th line of your js file change the code from p++ to++p i.e cellvalue = lastDateOfLastMonth - firstDayOfCurrentMonth + p++; A small bug in displaying the dates of previous months.

Rest of your code is fine. I hope you know the difference between p++ and ++p. If not then see this Stack Overflow question about the difference.