Resolves #1596, adds support for swipe functionality on narrative

[brian-learningpool]

@AaronmcL, this actually might need to change if/when https://github.com/adaptlearning/adapt_framework/pull/1640 goes in.

[oliverfoster]

yea, probs just need to remove the 'libraries/jquery.mobile.custom' line

[simondate]

May I suggest:

• Hiding the buttons on touch devices
• Having a touch specific instructional text

[lc-thomasberger]

Thanks @AaronmcL Tested on iOS9 and Android and works like a charm.

The suggestions raised by @simondate make a lot of sense to me. Maybe add a option to hide buttons on touch instead of always hiding them on touch devices. When the buttons are hidden, we will need a different instruction Text.

"_hideButtonOnTouch": true, "touchInstruction": "This instruction text is rendered when the buttons are disabled",

[moloko]

I agree with @lc-thomasberger - hiding buttons on touch should be an option not the default

[simondate]

Hiding buttons for .touch only could actually be quite problematic. Lots of Windows laptops have touch screens but that doesn't necessarily mean the learners will be using their touch screen at all.

Maybe we could make some distinction based upon touch and screen size too?

[brian-learningpool]

Removing the buttons entirely makes it slightly less intuitive to me. I've seen instances where users attempt to swipe from the arrows and the buttons are a visual indicator that the UI scrolls horizontally left, right, or in both directions.

Can we give the removal of the buttons and touch specific instruction text more thought and raise as a separate ticket?

[sarveshwar-gavhane]

can we reuse the code https://github.com/adaptlearning/adapt-contrib-narrative/blob/master/js/adapt-contrib-narrative.js#L298-L307 ?? instead of writing same code again

[moloko]

agree with @sarveshwar-gavhane - a certain amount of refactoring could be done here just having a think about how best to do it

[NayanKhedkar]

👍 @sarveshwar-gavhane

[moloko]

does anybody know what the intention of the code stage = (stage + numberOfItems) % numberOfItems; is? I mean I can kind of see the intention but in reality I don't think it's ever actually doing anything useful!

[NayanKhedkar]

you are right @moloko i think no longer required the code we can simply passed stage to setStage.

[moloko]

OK so maybe what we need to do is refactor so that we have navigateForward and navigateBack methods. The swipe events could be bound directly to the appropriate method; the click .narrative-controls should still call onNavigationClicked but could be greatly simplified to something like :

if ($(event.currentTarget).hasClass('narrative-control-right')) {
    this.navigateForward();
} else {
    this.navigateBack();
}

Either that or we make the event selector for click .narrative-controls much more specific so that we can bind directly to navigateForward and navigateBack and thereby dispense with onNavigationClicked altogether?

e.g.

'click .narrative-control-right': 'navigateForward',
'click .narrative-control-left': 'navigateBack',
'swipeleft .narrative-slider-graphic' : 'navigateForward',
'swiperight .narrative-slider-graphic': 'navigateBack'

I'm also wondering what the purpose of the _active model attribute is... anyone know?

All of this will of course need to be redone in #170

[brian-learningpool]

Been some time since I looked at this plugin but does _active relate to whether the item is rendering as a narrative or a hot graphic?

[moloko]

@brian-learningpool I had that thought too, the only two references to it are in setupNarrative (where it gets to set to true and onNavigationClicked where it will return early if not set. So it's not very clear why exactly it's needed.

[sarveshwar-gavhane]

@moloko either above approaches or we can simply add || isSwipeLeft in existing code

[moloko]

closing due to inactivity and being considerably out of date. still be nice to get this feature in though, just need to revisit properly