patternjavascriptMinor
JavaScript implementation of a TODO manager
Viewed 0 times
javascriptimplementationtodomanager
Problem
I was asked to create a TODO manager using JavaScript and CSS. I did not get a good review on the code nor specific comments on how to improve it.
Can someone here help by give me tips on what is bad with my implementation?
```
a img {border:none;}
#todo { float: left; width: 10%; min-height: 12em; } html #todo { height: 12em; } / IE6 */
.todo.custom-state-active { background: #eee; }
.todo li { float: left; width: 96px; padding: 0.4em; margin: 0 0.4em 0.4em 0; text-align: center; }
.todo li h5 { margin: 0 0 0.4em; cursor: move; }
.todo li a { float: right; }
.todo li a.ui-icon-zoomin { float: left; }
.todo li img { width: 100%; cursor: move; }
#newtask { float: left; width: 30%; min-height: 60em; margin-left: 20px; margin-top:15px;} html #newtask { height: 12em; } / IE6 */
.newtask.custom-state-active { background: #eee; }
.newtask li { float: left; width: 96px; padding: 0.4em; margin: 0 0.4em 0.4em 0; text-align: center; }
.newtask li h5 { margin: 0 0 0.4em; cursor: move; }
.newtask li a { float: right; }
.newtask li a.ui-icon-zoomin { float: left; }
.newtask li img { width: 100%; cursor: move; }
#inprogress { float: left; width: 30%; min-height: 60em; margin-left: 20px;margin-top:15px;} html #inprogress { height: 12em; } / IE6 */
.inprogress.custom-state-active { background: #eee; }
.inprogress li { float: left; width: 96px; padding: 0.4em; margin: 0 0.4em 0.4em 0; text-align: center; }
.inprogress li h5 { margin: 0 0 0.4em; cursor: move; }
.inprogress li a { float: right; }
.inprogress li a.ui-icon-zoomin { float: left; }
.inprogress li img { width: 100%; cursor: move; }
#done { float: left; width: 30%; min-height: 60em; margin-left: 20px;margin-top:15px;} html #inprogress { height: 12em; } / IE6 */
.d
Can someone here help by give me tips on what is bad with my implementation?
```
a img {border:none;}
#todo { float: left; width: 10%; min-height: 12em; } html #todo { height: 12em; } / IE6 */
.todo.custom-state-active { background: #eee; }
.todo li { float: left; width: 96px; padding: 0.4em; margin: 0 0.4em 0.4em 0; text-align: center; }
.todo li h5 { margin: 0 0 0.4em; cursor: move; }
.todo li a { float: right; }
.todo li a.ui-icon-zoomin { float: left; }
.todo li img { width: 100%; cursor: move; }
#newtask { float: left; width: 30%; min-height: 60em; margin-left: 20px; margin-top:15px;} html #newtask { height: 12em; } / IE6 */
.newtask.custom-state-active { background: #eee; }
.newtask li { float: left; width: 96px; padding: 0.4em; margin: 0 0.4em 0.4em 0; text-align: center; }
.newtask li h5 { margin: 0 0 0.4em; cursor: move; }
.newtask li a { float: right; }
.newtask li a.ui-icon-zoomin { float: left; }
.newtask li img { width: 100%; cursor: move; }
#inprogress { float: left; width: 30%; min-height: 60em; margin-left: 20px;margin-top:15px;} html #inprogress { height: 12em; } / IE6 */
.inprogress.custom-state-active { background: #eee; }
.inprogress li { float: left; width: 96px; padding: 0.4em; margin: 0 0.4em 0.4em 0; text-align: center; }
.inprogress li h5 { margin: 0 0 0.4em; cursor: move; }
.inprogress li a { float: right; }
.inprogress li a.ui-icon-zoomin { float: left; }
.inprogress li img { width: 100%; cursor: move; }
#done { float: left; width: 30%; min-height: 60em; margin-left: 20px;margin-top:15px;} html #inprogress { height: 12em; } / IE6 */
.d
Solution
You code look ok but I suggust using a MVC library, since this will abstract and simplify your logic.
Check out this project on github, called TodoMVC.
TodoMVC shows how to use popular MVC libraries to implement the popular 'todo list' project.
Examples can be found here.
The easiest MVC library to use is stapes.js
Here are a few tips for your code.
1) Be consistent. Use jQuery instead of raw DOM manipulation.
Example:
Previous Code:
New Code:
2)
3) Also, place all your CSS in external separate css files.
For more tips check out.
https://github.com/rwldrn/idiomatic.js
Check out this project on github, called TodoMVC.
TodoMVC shows how to use popular MVC libraries to implement the popular 'todo list' project.
Examples can be found here.
The easiest MVC library to use is stapes.js
Here are a few tips for your code.
1) Be consistent. Use jQuery instead of raw DOM manipulation.
Example:
Previous Code:
document.getElementById("inProgressCount").innerHTML = tasksInProgress + " Projects";
document.getElementById("doneCount").innerHTML = tasksDone + " Projects";
document.getElementById("todoCount").innerHTML = tasksInTodo + " Projects";
document.getElementById("totalCount").innerHTML = "Total: " + total + " Projects";New Code:
$("#inProgressCount").html( tasksInProgress + " Projects" );
$("#doneCount").html( tasksDone + " Projects");
$("#todoCount").html( tasksInTodo + " Projects");
$("#totalCount").html( "Total: " + total + " Projects");2)
onAddTask() should be split up into multiple functions since it's longer then 8 - 12 lines.3) Also, place all your CSS in external separate css files.
For more tips check out.
https://github.com/rwldrn/idiomatic.js
Code Snippets
document.getElementById("inProgressCount").innerHTML = tasksInProgress + " Projects";
document.getElementById("doneCount").innerHTML = tasksDone + " Projects";
document.getElementById("todoCount").innerHTML = tasksInTodo + " Projects";
document.getElementById("totalCount").innerHTML = "Total: " + total + " Projects";$("#inProgressCount").html( tasksInProgress + " Projects" );
$("#doneCount").html( tasksDone + " Projects");
$("#todoCount").html( tasksInTodo + " Projects");
$("#totalCount").html( "Total: " + total + " Projects");Context
StackExchange Code Review Q#14718, answer score: 2
Revisions (0)
No revisions yet.