-
Notifications
You must be signed in to change notification settings - Fork 149
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
fix: player controls #96
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,115 @@ | ||
import videojs from 'video.js'; | ||
/** | ||
* This class reacts to interactions with the canvas and | ||
* triggers appropriate functionality on the player. Right now | ||
* it does two things: | ||
* | ||
* 1. A `mousedown`/`touchstart` followed by `touchend`/`mouseup` without any | ||
* `touchmove` or `mousemove` toggles play/pause on the player | ||
* 2. Only moving on/clicking the control bar or toggling play/pause should | ||
* show the control bar. Moving around the scene in the canvas should not. | ||
*/ | ||
class CanvasPlayerControls extends videojs.EventTarget { | ||
constructor(player, canvas) { | ||
super(); | ||
|
||
this.player = player; | ||
this.canvas = canvas; | ||
|
||
this.onMoveEnd = videojs.bind(this, this.onMoveEnd); | ||
this.onMoveStart = videojs.bind(this, this.onMoveStart); | ||
this.onMove = videojs.bind(this, this.onMove); | ||
this.onControlBarMove = videojs.bind(this, this.onControlBarMove); | ||
|
||
this.player.controlBar.on([ | ||
'mousedown', | ||
'mousemove', | ||
'mouseup', | ||
'touchstart', | ||
'touchmove', | ||
'touchend' | ||
], this.onControlBarMove); | ||
|
||
// we have to override these here because | ||
// video.js listens for user activity on the video element | ||
// and makes the user active when the mouse moves. | ||
// We don't want that for 3d videos | ||
this.oldReportUserActivity = this.player.reportUserActivity; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not a huge fan of these but I don't think there's currently a way to disable it. Maybe we should have one? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I thought about adding one, but it seems like this plugin would be the only use case. |
||
this.player.reportUserActivity = () => {}; | ||
|
||
// canvas movements | ||
this.canvas.addEventListener('mousedown', this.onMoveStart); | ||
this.canvas.addEventListener('touchstart', this.onMoveStart); | ||
this.canvas.addEventListener('mousemove', this.onMove); | ||
this.canvas.addEventListener('touchmove', this.onMove); | ||
this.canvas.addEventListener('mouseup', this.onMoveEnd); | ||
this.canvas.addEventListener('touchend', this.onMoveEnd); | ||
|
||
this.shouldTogglePlay = false; | ||
} | ||
|
||
togglePlay() { | ||
if (this.player.paused()) { | ||
this.player.play(); | ||
} else { | ||
this.player.pause(); | ||
} | ||
} | ||
|
||
onMoveStart(e) { | ||
|
||
// if the player does not have a controlbar or | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can this use Video.js's There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. let me see if that works, looks like its probably a better solution. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK so I think that it would be too large of a refactor to convert this to a component and use the tap event. I think this will have to happen in another pr. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sounds good. |
||
// the move was a mouse click but not left click do not | ||
// toggle play. | ||
if (!this.player.controls() || (e.type === 'mousedown' && !videojs.dom.isSingleLeftClick(e))) { | ||
this.shouldTogglePlay = false; | ||
return; | ||
} | ||
|
||
this.shouldTogglePlay = true; | ||
this.touchMoveCount_ = 0; | ||
} | ||
|
||
onMoveEnd(e) { | ||
if (!this.shouldTogglePlay) { | ||
return; | ||
} | ||
this.togglePlay(); | ||
} | ||
|
||
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) { | ||
this.touchMoveCount_++; | ||
return; | ||
} | ||
this.shouldTogglePlay = false; | ||
} | ||
|
||
onControlBarMove(e) { | ||
this.player.userActive(true); | ||
} | ||
|
||
dispose() { | ||
this.canvas.removeEventListener('mousedown', this.onMoveStart); | ||
this.canvas.removeEventListener('touchstart', this.onMoveStart); | ||
this.canvas.removeEventListener('mousemove', this.onMove); | ||
this.canvas.removeEventListener('touchmove', this.onMove); | ||
this.canvas.removeEventListener('mouseup', this.onMoveEnd); | ||
this.canvas.removeEventListener('touchend', this.onMoveEnd); | ||
|
||
this.player.controlBar.off([ | ||
'mousedown', | ||
'mousemove', | ||
'mouseup', | ||
'touchstart', | ||
'touchmove', | ||
'touchend' | ||
], this.onControlBarMove); | ||
|
||
this.player.reportUserActivity = this.oldReportUserActivity; | ||
} | ||
} | ||
|
||
export default CanvasPlayerControls; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,7 @@ import VRControls from 'three/examples/js/controls/VRControls.js'; | |
import VREffect from 'three/examples/js/effects/VREffect.js'; | ||
import OrbitOrientationContols from './orbit-orientation-controls.js'; | ||
import * as utils from './utils'; | ||
import CanvasPlayerControls from './canvas-player-controls'; | ||
|
||
// import controls so they get regisetered with videojs | ||
import './cardboard-button'; | ||
|
@@ -497,7 +498,9 @@ class VR extends Plugin { | |
canvas: this.renderedCanvas, | ||
orientation: videojs.browser.IS_IOS || videojs.browser.IS_ANDROID || false | ||
}); | ||
this.canvasPlayerControls = new CanvasPlayerControls(this.player_, this.renderedCanvas); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we add this as a child of the player rather than just instantiating it here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh, it's not a real component, just an object. |
||
} | ||
|
||
this.animationFrameId_ = this.requestAnimationFrame(this.animate_); | ||
}); | ||
} else if (window.navigator.getVRDevices) { | ||
|
@@ -533,6 +536,11 @@ class VR extends Plugin { | |
if (this.controls3d) { | ||
this.controls3d.dispose(); | ||
} | ||
|
||
if (this.canvasPlayerControls) { | ||
this.canvasPlayerControls.dispose(); | ||
} | ||
|
||
if (this.effect) { | ||
this.effect.dispose(); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comments out https://github.com/mrdoob/three.js/blob/dev/examples/js/controls/OrbitControls.js#L226 so that we don't preventDefault on the contextmenu. It makes sense to do this if you are using pan controls with right click, but we are not.