From 07cd9800e8cbd1b7822ead6b249d015650ef80c8 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Wed, 5 Feb 2014 18:26:44 -0500 Subject: [PATCH 01/10] Fix touch events. Make components listen to touch events themselves. Components can have a "listenToTouchMove" property that would report user activity on touch moves. Currently, the only problem left is that the MediaTechController emits tap events to show/hide the controlbar but that causes the control bar to not be hidden via a tap. --- src/js/component.js | 17 +++++++++++++++++ src/js/control-bar/control-bar.js | 2 ++ src/js/control-bar/progress-control.js | 2 ++ src/js/media/media.js | 16 ---------------- src/js/player.js | 8 -------- src/js/slider.js | 4 ++++ 6 files changed, 25 insertions(+), 24 deletions(-) diff --git a/src/js/component.js b/src/js/component.js index 4407fa8876..197a6a7f35 100644 --- a/src/js/component.js +++ b/src/js/component.js @@ -65,6 +65,23 @@ vjs.Component = vjs.CoreObject.extend({ this.ready(ready); // Don't want to trigger ready here or it will before init is actually // finished for all children that run this constructor + + + var touchmove = false; + this.on('touchstart', function() { + touchmove = false; + }); + this.on('touchmove', vjs.bind(this, function() { + if (this.listenToTouchMove) { + this.player_.reportUserActivity(); + } + touchmove = true; + })); + this.on('touchend', vjs.bind(this, function(event) { + if (!touchmove && !didSomething) { + this.player_.reportUserActivity(); + } + })); } }); diff --git a/src/js/control-bar/control-bar.js b/src/js/control-bar/control-bar.js index de655ccaf9..3d59f6a415 100644 --- a/src/js/control-bar/control-bar.js +++ b/src/js/control-bar/control-bar.js @@ -24,6 +24,8 @@ vjs.ControlBar.prototype.options_ = { } }; +vjs.ControlBar.prototype.listenToTouchMove = true; + vjs.ControlBar.prototype.createEl = function(){ return vjs.createEl('div', { className: 'vjs-control-bar' diff --git a/src/js/control-bar/progress-control.js b/src/js/control-bar/progress-control.js index 0dc42cc749..a6b98b1fbd 100644 --- a/src/js/control-bar/progress-control.js +++ b/src/js/control-bar/progress-control.js @@ -19,6 +19,8 @@ vjs.ProgressControl.prototype.options_ = { } }; +vjs.ProgressControl.prototype.listenToTouchMove = true; + vjs.ProgressControl.prototype.createEl = function(){ return vjs.Component.prototype.createEl.call(this, 'div', { className: 'vjs-progress-control vjs-control' diff --git a/src/js/media/media.js b/src/js/media/media.js index 7565abc3de..8910a21d6b 100644 --- a/src/js/media/media.js +++ b/src/js/media/media.js @@ -82,24 +82,8 @@ vjs.MediaTechController.prototype.addControlsListeners = function(){ this.on('touchstart', function(event) { // Stop the mouse events from also happening event.preventDefault(); - event.stopPropagation(); - // Record if the user was active now so we don't have to keep polling it - userWasActive = this.player_.userActive(); }); - preventBubble = function(event){ - event.stopPropagation(); - if (userWasActive) { - this.player_.reportUserActivity(); - } - }; - - // Treat all touch events the same for consistency - this.on('touchmove', preventBubble); - this.on('touchleave', preventBubble); - this.on('touchcancel', preventBubble); - this.on('touchend', preventBubble); - // Turn on component tap events this.emitTapEvents(); diff --git a/src/js/player.js b/src/js/player.js index 636e8797bb..ef88299dc2 100644 --- a/src/js/player.js +++ b/src/js/player.js @@ -1316,14 +1316,6 @@ vjs.Player.prototype.listenForUserActivity = function(){ this.on('keydown', onMouseActivity); this.on('keyup', onMouseActivity); - // Consider any touch events that bubble up to be activity - // Certain touches on the tech will be blocked from bubbling because they - // toggle controls - this.on('touchstart', onMouseDown); - this.on('touchmove', onMouseActivity); - this.on('touchend', onMouseUp); - this.on('touchcancel', onMouseUp); - // Run an interval every 250 milliseconds instead of stuffing everything into // the mousemove/touchmove function itself, to prevent performance degradation. // `this.reportUserActivity` simply sets this.userActivity_ to true, which diff --git a/src/js/slider.js b/src/js/slider.js index b8bb906af7..2d70fb40d7 100644 --- a/src/js/slider.js +++ b/src/js/slider.js @@ -50,6 +50,8 @@ vjs.Slider.prototype.createEl = function(type, props) { return vjs.Component.prototype.createEl.call(this, type, props); }; +vjs.Slider.prototype.listenToTouchMove = true; + vjs.Slider.prototype.onMouseDown = function(event){ event.preventDefault(); vjs.blockTextSelection(); @@ -230,3 +232,5 @@ vjs.SliderHandle.prototype.createEl = function(type, props) { return vjs.Component.prototype.createEl.call(this, 'div', props); }; + +vjs.SliderHandle.prototype.listenToTouchMove = true; From 4b254a07df8069e36803a8001234539c604916c2 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Wed, 5 Feb 2014 19:19:14 -0500 Subject: [PATCH 02/10] remove unused var --- src/js/component.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/js/component.js b/src/js/component.js index 197a6a7f35..b1460dd22d 100644 --- a/src/js/component.js +++ b/src/js/component.js @@ -78,7 +78,7 @@ vjs.Component = vjs.CoreObject.extend({ touchmove = true; })); this.on('touchend', vjs.bind(this, function(event) { - if (!touchmove && !didSomething) { + if (!touchmove) { this.player_.reportUserActivity(); } })); From a7d624affe53ead68bf70bf0bb6eedee34f728da Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Wed, 5 Feb 2014 19:22:43 -0500 Subject: [PATCH 03/10] stop immediate propagation on tap events Make media tech controller only hide control bar. --- src/js/component.js | 4 +++- src/js/media/media.js | 5 ++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/js/component.js b/src/js/component.js index b1460dd22d..4ca4de70c2 100644 --- a/src/js/component.js +++ b/src/js/component.js @@ -868,7 +868,9 @@ vjs.Component.prototype.emitTapEvents = function(){ // When the touch ends, measure how long it took and trigger the appropriate // event - this.on('touchend', function() { + this.on('touchend', function(event) { + event.stopImmediatePropagation(); + // Proceed only if the touchmove/leave/cancel event didn't happen if (couldBeTap === true) { // Measure how long the touch lasted diff --git a/src/js/media/media.js b/src/js/media/media.js index 8910a21d6b..a51d3b6372 100644 --- a/src/js/media/media.js +++ b/src/js/media/media.js @@ -134,7 +134,10 @@ vjs.MediaTechController.prototype.onClick = function(event){ */ vjs.MediaTechController.prototype.onTap = function(){ - this.player().userActive(!this.player().userActive()); + var userActivity = this.player().userActive(); + if (userActivity) { + this.player().userActive(!userActivity); + } }; vjs.MediaTechController.prototype.features = { From 26c8d3f92cd0937674fcc81d12ca46a57ace1e77 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Wed, 5 Feb 2014 20:41:52 -0500 Subject: [PATCH 04/10] enableUserActivity on component disableUserActivity on MediaTechController and Player. MediaTechController does it manually. --- src/js/component.js | 47 ++++++++++++++++++++++++++----------------- src/js/media/media.js | 28 +++++++++++++++++++------- src/js/player.js | 1 + 3 files changed, 51 insertions(+), 25 deletions(-) diff --git a/src/js/component.js b/src/js/component.js index 4ca4de70c2..939c06ea10 100644 --- a/src/js/component.js +++ b/src/js/component.js @@ -66,22 +66,7 @@ vjs.Component = vjs.CoreObject.extend({ // Don't want to trigger ready here or it will before init is actually // finished for all children that run this constructor - - var touchmove = false; - this.on('touchstart', function() { - touchmove = false; - }); - this.on('touchmove', vjs.bind(this, function() { - if (this.listenToTouchMove) { - this.player_.reportUserActivity(); - } - touchmove = true; - })); - this.on('touchend', vjs.bind(this, function(event) { - if (!touchmove) { - this.player_.reportUserActivity(); - } - })); + this.enableUserActivity(); } }); @@ -869,8 +854,6 @@ vjs.Component.prototype.emitTapEvents = function(){ // When the touch ends, measure how long it took and trigger the appropriate // event this.on('touchend', function(event) { - event.stopImmediatePropagation(); - // Proceed only if the touchmove/leave/cancel event didn't happen if (couldBeTap === true) { // Measure how long the touch lasted @@ -885,3 +868,31 @@ vjs.Component.prototype.emitTapEvents = function(){ } }); }; + +vjs.Component.prototype.enableUserActivity = function() { + var touchmove = false; + + this.enableUserActivity.touchstart = function() { + touchmove = false; + }; + this.enableUserActivity.touchmove = vjs.bind(this, function() { + this.player_.reportUserActivity(); + touchmove = true; + }); + this.enableUserActivity.touchend = vjs.bind(this, function() { + if (!touchmove) { + this.player_.reportUserActivity(); + } + }); + + this.on('touchstart', this.enableUserActivity.touchstart); + this.on('touchmove', this.enableUserActivity.touchmove); + this.on('touchend', this.enableUserActivity.touchend); +}; + +vjs.Component.prototype.disableUserActivity = function() { + this.off('touchstart', this.enableUserActivity.touchstart); + this.off('touchmove', this.enableUserActivity.touchmove); + this.off('touchend', this.enableUserActivity.touchend); +}; + diff --git a/src/js/media/media.js b/src/js/media/media.js index a51d3b6372..59d9e8d0ce 100644 --- a/src/js/media/media.js +++ b/src/js/media/media.js @@ -15,6 +15,7 @@ vjs.MediaTechController = vjs.Component.extend({ vjs.Component.call(this, player, options, ready); this.initControlsListeners(); + this.disableUserActivity(); } }); @@ -60,7 +61,8 @@ vjs.MediaTechController.prototype.initControlsListeners = function(){ }; vjs.MediaTechController.prototype.addControlsListeners = function(){ - var preventBubble, userWasActive; + var userWasActive, + touchmove = false; // Some browsers (Chrome & IE) don't trigger a click on a flash swf, but do // trigger mousedown/up. @@ -82,14 +84,29 @@ vjs.MediaTechController.prototype.addControlsListeners = function(){ this.on('touchstart', function(event) { // Stop the mouse events from also happening event.preventDefault(); + userWasActive = this.player().userActive(); + touchmove = false; + }); + + this.on('touchmove', function(event) { + touchmove = true; + }) + + this.on('touchend', function(event) { + if (userWasActive) { + this.player().reportUserActivity(); + } + if (!touchmove) { + this.player().userActive(!this.player().userActive()); + } }); // Turn on component tap events - this.emitTapEvents(); + //this.emitTapEvents(); // The tap listener needs to come after the touchend listener because the tap // listener cancels out any reportedUserActivity when setting userActive(false) - this.on('tap', this.onTap); + //this.on('tap', this.onTap); }; /** @@ -134,10 +151,7 @@ vjs.MediaTechController.prototype.onClick = function(event){ */ vjs.MediaTechController.prototype.onTap = function(){ - var userActivity = this.player().userActive(); - if (userActivity) { - this.player().userActive(!userActivity); - } + this.player().userActive(!this.player().userActive()); }; vjs.MediaTechController.prototype.features = { diff --git a/src/js/player.js b/src/js/player.js index ef88299dc2..f121584005 100644 --- a/src/js/player.js +++ b/src/js/player.js @@ -106,6 +106,7 @@ vjs.Player = vjs.Component.extend({ } this.listenForUserActivity(); + this.disableUserActivity(); } }); From e5294848f94b36ea05bf3e4ffba64f868cbc3b3c Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Wed, 5 Feb 2014 20:50:39 -0500 Subject: [PATCH 05/10] Remove listenToTouchMove. --- src/js/control-bar/control-bar.js | 2 -- src/js/control-bar/progress-control.js | 2 -- src/js/slider.js | 4 ---- 3 files changed, 8 deletions(-) diff --git a/src/js/control-bar/control-bar.js b/src/js/control-bar/control-bar.js index 3d59f6a415..de655ccaf9 100644 --- a/src/js/control-bar/control-bar.js +++ b/src/js/control-bar/control-bar.js @@ -24,8 +24,6 @@ vjs.ControlBar.prototype.options_ = { } }; -vjs.ControlBar.prototype.listenToTouchMove = true; - vjs.ControlBar.prototype.createEl = function(){ return vjs.createEl('div', { className: 'vjs-control-bar' diff --git a/src/js/control-bar/progress-control.js b/src/js/control-bar/progress-control.js index a6b98b1fbd..0dc42cc749 100644 --- a/src/js/control-bar/progress-control.js +++ b/src/js/control-bar/progress-control.js @@ -19,8 +19,6 @@ vjs.ProgressControl.prototype.options_ = { } }; -vjs.ProgressControl.prototype.listenToTouchMove = true; - vjs.ProgressControl.prototype.createEl = function(){ return vjs.Component.prototype.createEl.call(this, 'div', { className: 'vjs-progress-control vjs-control' diff --git a/src/js/slider.js b/src/js/slider.js index 2d70fb40d7..b8bb906af7 100644 --- a/src/js/slider.js +++ b/src/js/slider.js @@ -50,8 +50,6 @@ vjs.Slider.prototype.createEl = function(type, props) { return vjs.Component.prototype.createEl.call(this, type, props); }; -vjs.Slider.prototype.listenToTouchMove = true; - vjs.Slider.prototype.onMouseDown = function(event){ event.preventDefault(); vjs.blockTextSelection(); @@ -232,5 +230,3 @@ vjs.SliderHandle.prototype.createEl = function(type, props) { return vjs.Component.prototype.createEl.call(this, 'div', props); }; - -vjs.SliderHandle.prototype.listenToTouchMove = true; From 44d6cac1b7c853fb93ff196f2925e1c64f3539bd Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Wed, 5 Feb 2014 20:58:30 -0500 Subject: [PATCH 06/10] Test is for tap events and not user activity --- test/unit/component.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/unit/component.js b/test/unit/component.js index ff88ae4a5a..c3cad1d276 100644 --- a/test/unit/component.js +++ b/test/unit/component.js @@ -231,6 +231,8 @@ test('should emit a tap event', function(){ var comp = new vjs.Component(getFakePlayer(), {}); + comp.disableUserActivity(); + comp.emitTapEvents(); comp.on('tap', function(){ ok(true, 'Tap event emitted'); From 4d2267c1bc91bcc4acada17d730d431a7787d6a1 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Thu, 6 Feb 2014 15:40:20 -0500 Subject: [PATCH 07/10] use tap events. Report activity on touch move. --- src/js/media/media.js | 20 +++----------------- 1 file changed, 3 insertions(+), 17 deletions(-) diff --git a/src/js/media/media.js b/src/js/media/media.js index 59d9e8d0ce..49e935fbe8 100644 --- a/src/js/media/media.js +++ b/src/js/media/media.js @@ -61,9 +61,6 @@ vjs.MediaTechController.prototype.initControlsListeners = function(){ }; vjs.MediaTechController.prototype.addControlsListeners = function(){ - var userWasActive, - touchmove = false; - // Some browsers (Chrome & IE) don't trigger a click on a flash swf, but do // trigger mousedown/up. // http://stackoverflow.com/questions/1444562/javascript-onclick-event-over-flash-object @@ -84,29 +81,18 @@ vjs.MediaTechController.prototype.addControlsListeners = function(){ this.on('touchstart', function(event) { // Stop the mouse events from also happening event.preventDefault(); - userWasActive = this.player().userActive(); - touchmove = false; }); this.on('touchmove', function(event) { - touchmove = true; - }) - - this.on('touchend', function(event) { - if (userWasActive) { - this.player().reportUserActivity(); - } - if (!touchmove) { - this.player().userActive(!this.player().userActive()); - } + this.player().reportUserActivity(); }); // Turn on component tap events - //this.emitTapEvents(); + this.emitTapEvents(); // The tap listener needs to come after the touchend listener because the tap // listener cancels out any reportedUserActivity when setting userActive(false) - //this.on('tap', this.onTap); + this.on('tap', this.onTap); }; /** From 75a23135e62ad23fec6a6bb1d39a02af104fcf09 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Thu, 6 Feb 2014 15:59:10 -0500 Subject: [PATCH 08/10] Report user activity on touchmove if userWasActive --- src/js/media/media.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/js/media/media.js b/src/js/media/media.js index 49e935fbe8..b446b1cf4a 100644 --- a/src/js/media/media.js +++ b/src/js/media/media.js @@ -61,6 +61,8 @@ vjs.MediaTechController.prototype.initControlsListeners = function(){ }; vjs.MediaTechController.prototype.addControlsListeners = function(){ + var userWasActive; + // Some browsers (Chrome & IE) don't trigger a click on a flash swf, but do // trigger mousedown/up. // http://stackoverflow.com/questions/1444562/javascript-onclick-event-over-flash-object @@ -81,10 +83,13 @@ vjs.MediaTechController.prototype.addControlsListeners = function(){ this.on('touchstart', function(event) { // Stop the mouse events from also happening event.preventDefault(); + userWasActive = this.player_.userActive(); }); this.on('touchmove', function(event) { - this.player().reportUserActivity(); + if (userWasActive){ + this.player().reportUserActivity(); + } }); // Turn on component tap events From 9975ed407a55e9e5a213753cc3a2b54d47a54661 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Thu, 6 Feb 2014 15:59:57 -0500 Subject: [PATCH 09/10] Don't enable enableUserActivity on an option --- src/js/component.js | 4 +++- src/js/media/media.js | 4 +++- src/js/player.js | 4 +++- test/unit/component.js | 4 +--- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/js/component.js b/src/js/component.js index 939c06ea10..eb7aaea284 100644 --- a/src/js/component.js +++ b/src/js/component.js @@ -66,7 +66,9 @@ vjs.Component = vjs.CoreObject.extend({ // Don't want to trigger ready here or it will before init is actually // finished for all children that run this constructor - this.enableUserActivity(); + if (options.reportUserActivity) { + this.enableUserActivity(); + } } }); diff --git a/src/js/media/media.js b/src/js/media/media.js index b446b1cf4a..792d8f4c4a 100644 --- a/src/js/media/media.js +++ b/src/js/media/media.js @@ -12,10 +12,12 @@ vjs.MediaTechController = vjs.Component.extend({ /** @constructor */ init: function(player, options, ready){ + // we don't want the tech to report user activity automatically. + // This is done manually in addControlsListeners + options.reportUserActivity = false; vjs.Component.call(this, player, options, ready); this.initControlsListeners(); - this.disableUserActivity(); } }); diff --git a/src/js/player.js b/src/js/player.js index f121584005..232514a795 100644 --- a/src/js/player.js +++ b/src/js/player.js @@ -54,6 +54,9 @@ vjs.Player = vjs.Component.extend({ // May be turned back on by HTML5 tech if nativeControlsForTouch is true tag.controls = false; + // we don't want the player to report user activity + options.reportUserActivity = false; + // Run base component initializing with new options. // Builds the element through createEl() // Inits and embeds any child components in opts @@ -106,7 +109,6 @@ vjs.Player = vjs.Component.extend({ } this.listenForUserActivity(); - this.disableUserActivity(); } }); diff --git a/test/unit/component.js b/test/unit/component.js index c3cad1d276..4bb2abfe48 100644 --- a/test/unit/component.js +++ b/test/unit/component.js @@ -229,9 +229,7 @@ test('should emit a tap event', function(){ var origTouch = vjs.TOUCH_ENABLED; vjs.TOUCH_ENABLED = true; - var comp = new vjs.Component(getFakePlayer(), {}); - - comp.disableUserActivity(); + var comp = new vjs.Component(getFakePlayer(), {reportUserActivity: false}); comp.emitTapEvents(); comp.on('tap', function(){ From 28a4d29934525c6a0bce5fdbe08b2abed8af8615 Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Thu, 6 Feb 2014 17:04:58 -0500 Subject: [PATCH 10/10] enable user activity on components by default --- src/js/component.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/js/component.js b/src/js/component.js index eb7aaea284..05e65a4d13 100644 --- a/src/js/component.js +++ b/src/js/component.js @@ -66,7 +66,7 @@ vjs.Component = vjs.CoreObject.extend({ // Don't want to trigger ready here or it will before init is actually // finished for all children that run this constructor - if (options.reportUserActivity) { + if (options.reportUserActivity !== false) { this.enableUserActivity(); } }