MS Excel type of grid in jQuery

Question

I have created an MS Excel type of grid in jQuery. I want to learn best practices and I need your comments for more optimized code.

Please review the code and offer your suggestions.

Demo

jQuery:

var JS = JS || {};

JS.training = JS.training || {};
  JS.training.tableData = JS.training.tableData || {};
  JS.training.tableData = {  

    defaults: {
        $table: $('#myTable'),
        addTableRowBtn: $('#addRowBtn'),
        addTableColBtn: $('#addColBtn')
    },
    createTable: function () {
        var _this = this,
            table = "";
        table += "<thead>";
        for (i = 0; i < 5; i++) {
            var tableHeader = (i == 0) ? "<th style='border:1px solid #E5E5E5; background:#F1F1F1'></th>" : "<th style='border:1px solid #E5E5E5; background:#F1F1F1'>A" + i + "</th>";
            table += tableHeader;
        }
        table += "</thead>";
        table += "<tbody>";
        for (i = 0; i < 5; i++) {
            table += "<tr  id='row" + i + "'>";
            for (var j = 0; j < 5; j++) {
                var tableDataCells = (j == 0) ? "<td width='3%' style='border:1px solid #E5E5E5;'>" + (i + 1) + "</td>" : "<td width=100px style='border:1px solid #E5E5E5' id='td" + j + "'  contenteditable=true> </td>";
                table += tableDataCells;
            }
            table += '</tr>';
        }
        table += "</tbody>";
        //APPEND TABLE MARKUP
        $(_this.defaults.$table).append(table);
        //BIND EVENTS
        _this.bindEvents();
    },
    addTableRow: function () {
        var _this = this,
            colLen = $("#myTable tr:nth-child(1) td").length,
            colVal = parseInt($("#myTable tr:last-child td:first").text()) + 1;
        for (i = 0; i < 1; i++) {
            var table = "<tr id=row" + colVal + "'>";
            for (var j = 0; j < colLen; j++) {
                if (j == 0) {
                    table += '<td width="3%" style="border:1px solid #E5E5E5; background:#F1F1F1">' + colVal + ' </td>';
                } else {
                    table += "<td width=100px style='border:1px solid #E5E5E5;' contenteditable=true id='td" + j + "'> </td>";
                }
            }
            table += '</tr>';
        }
        $(_this.defaults.$table).append(table);
    },
    addTableColumn: function () {
        var _this = this,
            colVal = $("#myTable tr th:last-child").text(),
            colNum = parseInt(colVal.charAt(1)) + 1;
        console.log(colNum);
        $("#myTable thead tr:last-child").append("<th  style='border:1px solid #E5E5E5; background:#F1F1F1'>A" + colNum + "</th>");
        $("#myTable tbody tr").each(function () {
            $(this).append("<td width=100px style='border:1px solid #E5E5E5;' contenteditable=true id='td" + colNum + "'></td>")
        });
    },
    bindEvents: function () {
        var _this = this;
        //CAPTURE ADD ROW BUTTON CLICK
        _this.defaults.addTableRowBtn.on('click', function () {
            _this.addTableRow();
        });

        //CAPTURE ADD COLUMN BUTTON CLICK
        _this.defaults.addTableColBtn.on('click', function () {
            _this.addTableColumn();
        });
    },
    init: function () {
        var _this = this;
        _this.createTable();
    }
};

//INIT CALL
JS.training.tableData.init();

HTML:

<div id="wrapper">
    <button id="addRowBtn" name="addRowBtn" value="Add Row">Add Row</button>
    <button id="addColBtn" name="addColBtn" value="Add Col">Add Column</button>
    <div id="table">
        <table id="myTable" cellpadding="0" cellspacing="0" border="0" style="border:1px solid #E5E5E5"></table>
    </div>
</div>

CSS:

body {
    font:normal 14px/16px Arial, Helvetica, sans-serif
}
#wrapper {
    margin:100px auto 0;
    width:80%;
}
#myTable {
    width:100%;
    margin:20px auto 0
}
#myTable th, #myTable td {
    padding:5px;
}
#myTable tr td.first {
    background:#F1F1F1;
    border:1px solid #E5E5E5;
}

Answer 1

From a once over:

• The column and row count should be a parameter/variable/default
• The namespace JS.training seems like overkill, I doubt anybody would ever use 2 libraries for showing datagrids ;)

• The following could use extraction of the pattern, also you should look into a templating library I am using my personal template function here as an example:

      for (i = 0; i < 5; i++) {
          var tableHeader = (i == 0) ? 
          "<th style='border:1px solid #E5E5E5; background:#F1F1F1'></th>" : 
          "<th style='border:1px solid #E5E5E5; background:#F1F1F1'>A" + i + "</th>";
          table += tableHeader;
      }
  

  could be

  function fillTemplate( s )
  { //Replace ~ with further provided arguments
    for( var i = 1, a = s.split('~'), s = '' ; i < arguments.length ; i++ )
      s = s + a.shift() + arguments[i];
    return s + a.join("");
  }     
  
  var headerTemplate = `<th style='border:1px solid #E5E5E5; background:#F1F1F1'>~</th>`    
  for (i = 0; i < 5; i++) {
    var header = (i == 0) ? "" : i;
    table += fillTemplate( headerTemplate, header );
  }
  

• I would split up the creation of header and items HTML into 2 separate functions

• var _this = this is really only useful for closures, and you are not using those in createTable.
• You should extract <td width="3%" style="border:1px solid #E5E5E5; background:#F1F1F1"> into a template as well, you use it both in createTable and addTableRow.
• Same goes for the header template which you use as well in addTableColumn

• These comments can be filed under Captain Obvious and removed:

  //CAPTURE ADD ROW BUTTON CLICK
  _this.defaults.addTableRowBtn.on('click', function () {
      _this.addTableRow();
  });
  
  //CAPTURE ADD COLUMN BUTTON CLICK
  _this.defaults.addTableColBtn.on('click', function () {
      _this.addTableColumn();
  });
  

All in all I really like your code, I will leave it to @kleinfreund to review your CSS.

Answer 2

Only some quick notes on your HTML/CSS.

HTML:

1. You're repeating ID's in your generated markup. ID's must only occur once. If you need them in multiple locations, use classes instead.

CSS:

1. You declare a line-height of 16 pixels. I'd advice you using a unit-less relative value for line-height like this: font: 14px/1.2 Arial, Helvetica, sans-serif;

   Otherwise you would need to redefine the line-height on all elements with a higher font-size, because they currently have a line-height of 16 pixels as well. With a relative value you do this only once.

   I also ommited normal in your declaration, because the default values for font-style, font-weight and font-variation are normal anyway.

2. Although it's possible to ommit the last ; in a rule declaration, it's not a good practise. You will run into troubles while editing CSS. By always including it, you don't need to worry about issues like moving the declaration further to the top, etc.

3. You add some CSS rules as inline styles to all your generated td's. You can move these rules to your CSS to reduce this unnecessary repetition.

Answer 3

Some further points...

• You have inline CSS in your tds, this could easily be extracted into your stylesheet.

• I much prefer creating HTML using jQuery, rather than ugly concatenated strings:

  var $table = $('<tbody>')
  
  for (i = 0; i < 5; i++) {
      var $row = $('<tr>');
      for (var j = 0; j < 5; j++) {
          var $cell = (j == 0)
            ? $('<td>').text(i + 1) 
            : $('<td>').attr('contenteditable', true);
          $cell.appendTo($row);
      }
      $row.appendTo($table);
  }