Fix/ VR360 player's behavior different to standar player on mobile devices #190
Conversation
| } | ||
|
|
||
| onMove(e) { | ||
| // its hard to tap without a touchmove, if there have been less | ||
| // than one, we should still toggle play | ||
| if (e.type === 'touchmove' && this.touchMoveCount_ < 1) { |
There was a problem hiding this comment.
why not just increase the limit here?
There was a problem hiding this comment.
We want to check when the movement has finished in the touchend event if it was a "touch for a click" or it was to move the camera in the canvas.
There was a problem hiding this comment.
It sounds like this is more or less what the shouldTogglePlay was supposed to do. Set a flag if there was enough touchmoves to exclude it this touch interaction from being a tap. I'd be interested to know if that will work as it'll be a much smaller change and won't require toggling user activity manually.
There was a problem hiding this comment.
The problem here is we need the change since we want to have the play/pause logic for desktop/mouse and the userActive logic for mobile devices/touch screen when the screen is touched. The current behavior that we have with my changes are:
360°/VR Player iOS/Android
Click/touch - Toggle userActive() - Show and hide control bar.
360°/VR Player Desktop
Click/mouse -- Toggle play/pause since you have the userActive() in mouseover
These ones are in the Standard player:
Standard Player iOS/Android
Click/touch - Toggle userActive() - Show and hide control bar.
** Standard Player Desktop**
Click/mouse -- Toggle play/pause since you have the userActive() in mouseover
src/canvas-player-controls.js
Outdated
| // We want to have the same behavior in VR360 Player and standar player. | ||
| // in touchend we want to know if was a touch click, for a click we show the bar, | ||
| // otherwise continue with the mouse logic. | ||
| if (e.type === 'touchend' && this.touchMoveCount_ < 7) { |
There was a problem hiding this comment.
Commonly when you touch the screen on and android device looking for "touch for a click" not "move touch" you get between 1 - 6 touchmove
There was a problem hiding this comment.
Looks like Video.js uses 10 as the threshold, might be better to just use that: https://github.com/videojs/video.js/blob/master/src/js/component.js#L1147-L1150.
There was a problem hiding this comment.
In addition to the previous test, I wonder if we can just rely on Video.js's tap events rather than re-implementing it here https://github.com/videojs/video.js/blob/master/src/js/component.js#L1142
This might require doing something like
Component.prototype.emitTapEvents.call(this)in the constructor.
src/canvas-player-controls.js
Outdated
| // We want to have the same behavior in VR360 Player and standar player. | ||
| // in touchend we want to know if was a touch click, for a click we show the bar, | ||
| // otherwise continue with the mouse logic. | ||
| if (e.type === 'touchend' && this.touchMoveCount_ < 7) { |
There was a problem hiding this comment.
Looks like Video.js uses 10 as the threshold, might be better to just use that: https://github.com/videojs/video.js/blob/master/src/js/component.js#L1147-L1150.
Fix different behavior between standard and vr360 player in iOS/Android devices