adaptlearning / adapt-contrib-narrative

A component that displays an image gallery with accompanying text
GNU General Public License v3.0
5 stars 39 forks source link

fixes ABU-1114 #154

Closed sarveshwar-gavhane closed 8 years ago

sarveshwar-gavhane commented 8 years ago

Fixes #146

moloko commented 8 years ago

sorry @sarveshwar-gavhane this doesn't seem to fix the issue - which is that you get a JS error if _items is missing from the .json

The error is currently occurring on L63 (in your modified version).

moloko commented 8 years ago

+1 certainly prevents the JavaScript error for me now. it is maybe also worth adding a console.warn() to point out that there are no _items? @brian-learningpool as you logged the original ticket, what do you think?

brian-learningpool commented 8 years ago

@sarveshwar-gavhane, can you change the code to this? if (!this.model.has('_items') || !this.model.get('_items').length) return;

Otherwise if the _items property is set, such as in the authoring tool, but it doesn't contain any items this throws another JavaScript error. We should probably think about doing something with the template too, as some <div> controls are rendered.

tomgreenfield commented 8 years ago

I've found that if (_.isEmpty(items)) return; reads really nicely in these sorts of cases.

sarveshwar-gavhane commented 8 years ago

yeah sure @brian-learningpool other wise it throws an error on L187, @tomgreenfield if (_.isEmpty('items') || !this.model.get('_items').length) return; also works

ghost commented 8 years ago

Both @brian-learningpool solution and @tomgreenfield solution worked for me:

if (!this.model.has('_items') || !this.model.get('_items').length) return;

OR

if (_.isEmpty(this.model.get('_items'))) return;

ghost commented 8 years ago

+1

brian-learningpool commented 8 years ago

+1

moloko commented 8 years ago

+1