patternjavascriptMinor
MS Excel type of grid in jQuery
Viewed 0 times
typejquerygridexcel
Problem
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 += "";
for (i = 0; i " : "A" + i + "";
table += tableHeader;
}
table += "";
table += "";
for (i = 0; i ";
for (var j = 0; j " + (i + 1) + "" : " ";
table += tableDataCells;
}
table += '';
}
table += "";
//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 ";
for (var j = 0; j ' + colVal + ' ';
} else {
table += " ";
}
}
table += '';
}
$(_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("A" + colNum + "");
$("#myTable tbody tr").each(function () {
$(this).append("")
});
},
bindEvents: function () {
var _this = this;
//CAPTURE ADD ROW BUTTON CLICK
_this.defaults.addTableRowBtn.on('click', fun
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 += "";
for (i = 0; i " : "A" + i + "";
table += tableHeader;
}
table += "";
table += "";
for (i = 0; i ";
for (var j = 0; j " + (i + 1) + "" : " ";
table += tableDataCells;
}
table += '';
}
table += "";
//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 ";
for (var j = 0; j ' + colVal + ' ';
} else {
table += " ";
}
}
table += '';
}
$(_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("A" + colNum + "");
$("#myTable tbody tr").each(function () {
$(this).append("")
});
},
bindEvents: function () {
var _this = this;
//CAPTURE ADD ROW BUTTON CLICK
_this.defaults.addTableRowBtn.on('click', fun
Solution
From a once over:
-
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:
could be
-
I would split up the creation of header and items HTML into 2 separate functions
-
These comments can be filed under Captain Obvious and removed:
All in all I really like your code, I will leave it to @kleinfreund to review your CSS.
- The column and row count should be a parameter/variable/default
- The namespace
JS.trainingseems 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 " :
"A" + i + "";
table += tableHeader;
}could be
function fillTemplate( s )
{ //Replace ~ with further provided arguments
for( var i = 1, a = s.split('~'), s = '' ; i ~`
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 = thisis really only useful for closures, and you are not using those increateTable.
- You should extract `
into a template as well, you use it both increateTableandaddTableRow.
- 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.
Code Snippets
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;
}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 );
}//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();
});Context
StackExchange Code Review Q#41130, answer score: 7
Revisions (0)
No revisions yet.