Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add items collection #1638

Merged
merged 4 commits into from
Oct 25, 2017
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions src/core/js/models/itemModel.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
define(function() {

var ItemModel = Backbone.Model.extend({

defaults: { _isActive: false, _isVisited: false },

reset: function() {
this.set({ _isActive: false, _isVisited: false });
},

toggleActive: function(isActive) {
if (isActive === undefined) {
isActive = !this.get('_isActive');
}

this.set('_isActive', isActive);
},

toggleVisited: function(isVisited) {
if (isVisited === undefined) {
isVisited = !this.get('_isVisited');
}

this.set('_isVisited', isVisited);
}

});

return ItemModel;

});
76 changes: 76 additions & 0 deletions src/core/js/models/itemsComponentModel.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
define([
'core/js/models/componentModel',
'core/js/models/itemModel'
], function(ComponentModel, ItemModel) {

var ItemsComponentModel = ComponentModel.extend({

toJSON: function() {
var json = _.clone(this.attributes);

json._items = this.get('_items').toJSON();

return json;
},

init: function() {
this.setUpItems();

this.listenTo(this.get('_items'), {
'change:_isVisited': this.checkCompletionStatus
});
},

setUpItems: function() {
var items = this.get('_items');
var collection = new Backbone.Collection(null, { model: ItemModel });

items.forEach(function(item, index) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could actually call .map() here and call the Collection add() with an array of items after.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you write out the code? my brain has gone to sleep... :/

Copy link
Member

@brian-learningpool brian-learningpool Aug 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like this:

var items = items.map(function(item, index) {
  item._index = index;
  return item;
});

collection.add(items);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't make use of the array returned by calling .map so .forEach could still be used. If immutability is desirable though
var items = items.slice().map(
would prevent mutation

Also since there's now only one .add call why not just use

var collection = new Backbone.Collection(items, { model: itemModel })

After the array has been mapped.

Sorry for the lack of MD I'm on a phone currently.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point.

item._index = index;
collection.add(item);
});

this.set('_items', collection);
},

getItem: function(index) {
return this.get('_items').findWhere({ _index: index });
},

getVisitedItems: function() {
return this.get('_items').where({ _isVisited: true });
},

getActiveItems: function() {
return this.get('_items').where({ _isActive: true });
},

getActiveItem: function() {
return this.get('_items').findWhere({ _isActive: true });
},

areAllItemsCompleted: function() {
return this.getVisitedItems().length === this.get('_items').length;
},

checkCompletionStatus: function() {
if (this.areAllItemsCompleted()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return early?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha, I think defiite overkill / needless expansion for single-line if statements. Only an issue if dealing with multiple nested lines and/or actions after the conditional.

this.setCompletionStatus();
}
},

reset: function(type, force) {
this.get('_items').each(function(item) { item.reset(); });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be more readable on a single line if we were using arrow functions.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:O * gasp *

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't do that with IE 11 still in the mix though. :-/

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to have a chat with you about that kind of stuff.


ComponentModel.prototype.reset.call(this, type, force);
},

resetActiveItems: function() {
this.get('_items').each(function(item) { item.toggleActive(false); });
}

});

return ItemsComponentModel;

});
126 changes: 0 additions & 126 deletions src/core/js/models/itemsModel.js

This file was deleted.