Items model #170

lc-thomasberger · 2017-04-04T15:33:01Z

_isActive attribute is added to _items Array. This attributes are managed by funcitons defined in the itemsModel.
The setter functions trigger a change event change_items:_isActive. Views listen to this event and hande Dom updates. Plugins can use the functions on the itemsModel to manipulate the state of the Component.

removed unused functions

updated required Framework version

oliverfoster · 2017-04-04T17:59:15Z

js/narrativeModel.js

+        defaults: function() {
+            return _.extend({
+                _activeItem: 0
+            }, ItemsModel.prototype.defaults);


If you assume nothing is known about ItemsModel.prototype.defaults apart from what is described in the Backbone API, it could be a function or an object.

Can you instead use _.result(ItemsModel.prototype, "defaults"); here just in case we have to change the way defaults is defined on the itemsModel later?

It's equivalent of doing typeof ItemsModel.prototype.defaults == "function" ? ItemsModel.prototype.defaults() : ItemsModel.prototype.defaults;

http://underscorejs.org/#result

oliverfoster · 2017-04-04T18:04:04Z

js/narrativeModel.js

+    var NarrativeModel = ItemsModel.extend({
+
+        defaults: function() {
+            return _.extend({


_.extend overwrites object attributes from right to left. If the ItemsModel was to define an _activeItem:1 it would overwrite this NarrativeModel _activeItem:0. These arguments should be inverted so that this model overwrites the defaults from the ItemsModel:

_.extend(ItemsModel, NarrativeModel);

_.extend({}, ItemsModel, NarrativeModel);

remove oncomplete as it not used anymore

use _.result for itemsModel defaults

oliverfoster · 2017-04-05T10:23:51Z

js/narrativeModel.js

+        },
+
+        checkCompletionStatus: function() {
+            ItemsModel.prototype.checkCompletionStatus.apply(this, arguments);


This line can go as this function overrides the behaviour of ItemsModel, the completion for Narrative is handled by the narrative, either inview or allItems

oliverfoster · 2017-04-05T10:24:55Z

js/narrativeModel.js

@@ -6,9 +6,9 @@ define([
    var NarrativeModel = ItemsModel.extend({

        defaults: function() {
-            return _.extend({
+            return _.extend(_.result(ItemsModel.prototype, "defaults"), {


return _.extend({}, _.result(ItemsModel.prototype, "defaults"), {

Extra object needed so that the we don't overwrite the ItemsModel.prototype.defaults object with our Narrative defaults.

added empty object to extend funciton for defaults

oliverfoster · 2017-04-05T13:13:57Z

js/narrativeView.js

+        },
+
+        preRender: function() {
+            this.listenTo(Adapt, 'device:changed', this.reRender, this);


this.listenTo(Adapt, { 'device:changed': this.reRender, 'device:resize', this.resizeControl, 'notify:closed', this.closeNotify });

^ if you feel like it?

oliverfoster · 2017-04-06T13:05:45Z

js/narrativeModel.js

        },

        prepareNarrativeModel: function() {
            this.set('_component', 'narrative');
            this.set('_wasHotgraphic', true);
            this.set('originalBody', this.get('body'));
            this.set('originalInstruction', this.get('instruction'));
-            this.set('_activeItem', (this.get('_activeItem') === -1) ? 0 : this.get('_activeItem'));
+
+            var activeItem = this.getActiveItemsIndexes()[0] || 0;


Maybe on ItemsModel :

getFirstActiveItemIndex: function() { return this.getActiveItemsIndexes()[0]; }, getFirstActiveItem: function() { return this.model.getActiveItems()[0]; }

Seeing as this is used reasonably often in narrative (and probably hotgraphic, i haven't read it yet) I think it'll make these references easier.

oliverfoster · 2017-04-06T13:07:31Z

js/narrativeView.js

-            this.model.set('_activeItem', stage);
+            nextStage = (nextStage + numberOfItems) % numberOfItems;
+            this.model.setItemAtIndexAsInactive(activeStage, false);
+            this.model.setItemAtIndexAsActive(nextStage, true);


did you need the explicit true here? (and in onProgressClicked)

remove default trigger true falg for readability assign listener to Adapt as object

oliverfoster · 2017-04-07T13:28:55Z

js/narrativeModel.js

+                this.set('_marginDir', 'right');
+            }
+            this.set('_itemCount', this.get('_items').length);
+


Can your change this to defaults as with accordion:

defaults: function() { return _.extend({}, _.results(ItemsModel.prototype, "defaults"), { _marginDir: (Adapt.config.get('_defaultDirection') == 'rtl') ? 'left': 'right', _itemCount: this.getItemCount() }); }

moloko

tested in IE11 and Firefox

oliverfoster · 2017-06-16T14:45:27Z

please do not merge until @tomgreenfield comes back with review

lc-thomasberger · 2018-02-07T17:20:48Z

close this one in favor for #179

lc-thomasberger added 5 commits April 4, 2017 14:33

use Model and View

2c74ef1

added model code

63328dd

refactored to use model

c10b6ff

removed unused functions

Version bump

9b09873

updated required Framework version

remove this from the view as it is handled in css

5e9e47d

oliverfoster reviewed Apr 4, 2017

View reviewed changes

lc-thomasberger added 3 commits April 5, 2017 11:21

handle completionCheck in model

b5f45c2

remove oncomplete as it not used anymore

update extend order

de8b2bd

use _.result for itemsModel defaults

hande reset in model

ad9cc0f

oliverfoster reviewed Apr 5, 2017

View reviewed changes

lc-thomasberger added 3 commits April 5, 2017 12:52

add force paramter

43e396c

fixed completion handling

f96677b

added empty object to extend funciton for defaults

renamed getCurrentItem to getItemAtIndex

985a69e

oliverfoster reviewed Apr 5, 2017

View reviewed changes

updated to use _isActive attribute on _items

4231cef

oliverfoster reviewed Apr 6, 2017

View reviewed changes

lc-thomasberger added 2 commits April 7, 2017 09:38

use getFirstActiveItemIndex functions

5bd8c5c

remove default trigger true falg for readability assign listener to Adapt as object

use getFirstActiveItemIndex function

5c6b302

oliverfoster reviewed Apr 7, 2017

View reviewed changes

lc-thomasberger added 5 commits April 10, 2017 12:59

split up into defaults and init

5166702

rename function checkAllItemsCompleted -> areAllItemsCompleted

980cefc

verison bump to 2.1.0

3c7df20

Merge branch 'master' into itemsModel

3660021

updated method names

b7029d8

lc-thomasberger requested review from moloko, brian-learningpool and dancgray May 9, 2017 08:12

lc-thomasberger requested a review from oliverfoster May 9, 2017 08:12

oliverfoster approved these changes May 9, 2017

View reviewed changes

moloko approved these changes May 10, 2017

View reviewed changes

moloko mentioned this pull request Jul 10, 2017

Resolves #1596, adds support for swipe functionality on narrative #174

Closed

lc-thomasberger closed this Feb 7, 2018

moloko deleted the itemsModel branch June 26, 2018 09:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Items model #170

Items model #170

lc-thomasberger commented Apr 4, 2017 •

edited

Loading

oliverfoster Apr 4, 2017

oliverfoster Apr 4, 2017

oliverfoster Apr 5, 2017

oliverfoster Apr 5, 2017

oliverfoster Apr 5, 2017 •

edited

Loading

oliverfoster Apr 5, 2017

oliverfoster Apr 6, 2017 •

edited

Loading

oliverfoster Apr 6, 2017 •

edited

Loading

oliverfoster Apr 7, 2017 •

edited

Loading

moloko left a comment

oliverfoster commented Jun 16, 2017

lc-thomasberger commented Feb 7, 2018

Items model #170

Items model #170

Conversation

lc-thomasberger commented Apr 4, 2017 • edited Loading

oliverfoster Apr 4, 2017

Choose a reason for hiding this comment

oliverfoster Apr 4, 2017

Choose a reason for hiding this comment

oliverfoster Apr 5, 2017

Choose a reason for hiding this comment

oliverfoster Apr 5, 2017

Choose a reason for hiding this comment

oliverfoster Apr 5, 2017 • edited Loading

Choose a reason for hiding this comment

oliverfoster Apr 5, 2017

Choose a reason for hiding this comment

oliverfoster Apr 6, 2017 • edited Loading

Choose a reason for hiding this comment

oliverfoster Apr 6, 2017 • edited Loading

Choose a reason for hiding this comment

oliverfoster Apr 7, 2017 • edited Loading

Choose a reason for hiding this comment

moloko left a comment

Choose a reason for hiding this comment

oliverfoster commented Jun 16, 2017

lc-thomasberger commented Feb 7, 2018

lc-thomasberger commented Apr 4, 2017 •

edited

Loading

oliverfoster Apr 5, 2017 •

edited

Loading

oliverfoster Apr 6, 2017 •

edited

Loading

oliverfoster Apr 6, 2017 •

edited

Loading

oliverfoster Apr 7, 2017 •

edited

Loading