Commit 67be50ca authored by Sébastien Chopin's avatar Sébastien Chopin

**First, thanks to do this awesome job with TodoMVC, It's great to see some projects like this :)**

Here my modifications to respect the "best-pratices" in Backbone.js :

**Model** - js/models/todo.js

 - remove `initialize` method in Model (useless because `defaults` do already the job)
 - remove `clear` method, we should not change `clear` behavior of Backbone.Model -> http://backbonejs.org/#Model-clear

**Views** - js/views/app.js and js/views/todos.js

 - Call model.destroy() (model.clear(); is not logic)
 - Use this.$el instead of $(this.el) to improve readability and caching

Correct this issue: https://github.com/addyosmani/todomvc/issues/223 with adding `window.app.Todos.on( 'change:completed', this.addAll, this );` and `add` event call now `addAll` method in *js/views/app.js*

Issue related too: https://github.com/addyosmani/todomvc/issues/253
parent 29732c99
...@@ -8,30 +8,17 @@ ...@@ -8,30 +8,17 @@
window.app.Todo = Backbone.Model.extend({ window.app.Todo = Backbone.Model.extend({
// Default attributes for the todo. // Default attributes for the todo.
// Ensure that each todo created has `title` and `completed` keys
defaults: { defaults: {
title: '', title: '',
completed: false completed: false
}, },
// Ensure that each todo created has `title`.
initialize: function() {
if ( !this.get('title') ) {
this.set({
'title': this.defaults.title
});
}
},
// Toggle the `completed` state of this todo item. // Toggle the `completed` state of this todo item.
toggle: function() { toggle: function() {
this.save({ this.save({
completed: !this.get('completed') completed: !this.get('completed')
}); });
},
// Remove this Todo from *localStorage* and delete its view.
clear: function() {
this.destroy();
} }
}); });
......
...@@ -9,7 +9,7 @@ $(function( $ ) { ...@@ -9,7 +9,7 @@ $(function( $ ) {
// Instead of generating a new element, bind to the existing skeleton of // Instead of generating a new element, bind to the existing skeleton of
// the App already present in the HTML. // the App already present in the HTML.
el: $('#todoapp'), el: '#todoapp',
// Our template for the line of statistics at the bottom of the app. // Our template for the line of statistics at the bottom of the app.
statsTemplate: _.template( $('#stats-template').html() ), statsTemplate: _.template( $('#stats-template').html() ),
...@@ -28,12 +28,13 @@ $(function( $ ) { ...@@ -28,12 +28,13 @@ $(function( $ ) {
this.input = this.$('#new-todo'); this.input = this.$('#new-todo');
this.allCheckbox = this.$('#toggle-all')[0]; this.allCheckbox = this.$('#toggle-all')[0];
window.app.Todos.on( 'add', this.addOne, this ); window.app.Todos.on( 'add', this.addAll, this );
window.app.Todos.on( 'reset', this.addAll, this ); window.app.Todos.on( 'reset', this.addAll, this );
window.app.Todos.on( 'change:completed', this.addAll, this );
window.app.Todos.on( 'all', this.render, this ); window.app.Todos.on( 'all', this.render, this );
this.$footer = $('#footer'); this.$footer = this.$('#footer');
this.$main = $('#main'); this.$main = this.$('#main');
window.app.Todos.fetch(); window.app.Todos.fetch();
}, },
...@@ -112,7 +113,7 @@ $(function( $ ) { ...@@ -112,7 +113,7 @@ $(function( $ ) {
// Clear all completed todo items, destroying their models. // Clear all completed todo items, destroying their models.
clearCompleted: function() { clearCompleted: function() {
_.each( window.app.Todos.completed(), function( todo ) { _.each( window.app.Todos.completed(), function( todo ) {
todo.clear(); todo.destroy();
}); });
return false; return false;
......
...@@ -32,10 +32,8 @@ $(function() { ...@@ -32,10 +32,8 @@ $(function() {
// Re-render the titles of the todo item. // Re-render the titles of the todo item.
render: function() { render: function() {
var $el = $( this.el ); this.$el.html( this.template( this.model.toJSON() ) );
this.$el.toggleClass( 'completed', this.model.get('completed') );
$el.html( this.template( this.model.toJSON() ) );
$el.toggleClass( 'completed', this.model.get('completed') );
this.input = this.$('.edit'); this.input = this.$('.edit');
return this; return this;
...@@ -48,7 +46,7 @@ $(function() { ...@@ -48,7 +46,7 @@ $(function() {
// Switch this view into `"editing"` mode, displaying the input field. // Switch this view into `"editing"` mode, displaying the input field.
edit: function() { edit: function() {
$( this.el ).addClass('editing'); this.$el.addClass('editing');
this.input.focus(); this.input.focus();
}, },
...@@ -62,7 +60,7 @@ $(function() { ...@@ -62,7 +60,7 @@ $(function() {
this.clear(); this.clear();
} }
$( this.el ).removeClass('editing'); this.$el.removeClass('editing');
}, },
// If you hit `enter`, we're through editing the item. // If you hit `enter`, we're through editing the item.
...@@ -72,9 +70,9 @@ $(function() { ...@@ -72,9 +70,9 @@ $(function() {
} }
}, },
// Remove the item, destroy the model. // Remove the item, destroy the model from *localStorage* and delete its view
clear: function() { clear: function() {
this.model.clear(); this.model.destroy();
} }
}); });
}); });
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment