From 640ce997804b8b649ffd8968e9b3d5513622094e Mon Sep 17 00:00:00 2001 From: Jon Zepernick Date: Tue, 25 Mar 2014 12:07:54 -0400 Subject: [PATCH 1/6] Buttons can be clicked even if touchmoves are thrown Previous behavior would suppress Button clicks if any touchmove events are thrown. Some devices are super sensitive and throw touchmove events for even slight taps. So instead, let's track movement during touchmoves and allow clicks to happen if the touch does not move very far. Fixes #1108 --- src/js/button.js | 41 ++++++++++++++++++++++++++++++++++------- 1 file changed, 34 insertions(+), 7 deletions(-) diff --git a/src/js/button.js b/src/js/button.js index 71d8d346de..75c42937f5 100644 --- a/src/js/button.js +++ b/src/js/button.js @@ -15,18 +15,45 @@ vjs.Button = vjs.Component.extend({ init: function(player, options){ vjs.Component.call(this, player, options); - var touchstart = false; + var treatTouchAsClick = false; + var touchLocation = null; + this.on('touchstart', function(event) { - // Stop click and other mouse events from triggering also - event.preventDefault(); - touchstart = true; + // If more than one finger, we don't want to consider treating this as a click + if (event.touches.length == 1) { + // Stop click and other mouse events from triggering also + event.preventDefault(); + treatTouchAsClick = true; + touchLocation = { + x: event.touches[0].pageX, + y: event.touches[0].pageY + }; + } }); - this.on('touchmove', function() { - touchstart = false; + + this.on('touchmove', function(event) { + if (event.touches.length > 1) { + treatTouchAsClick = false; + } else if (touchLocation) { + var newTouch = { + x: event.touches[0].pageX, + y: event.touches[0].pageY + }; + var xdiff = newTouch.x - touchLocation.x; + var ydiff = newTouch.y - touchLocation.y; + var touchDistance = Math.sqrt(xdiff * xdiff + ydiff * ydiff); + // Some devices are really sensitive and will throw touchmoves for all but the slightest + // of taps. So, if we moved only a small distance, let's still consider this a click. + if (touchDistance > 44) { + treatTouchAsClick = false; + } + } }); + var self = this; this.on('touchend', function(event) { - if (touchstart) { + touchLocation = null; + if (treatTouchAsClick) { self.onClick(event); } event.preventDefault(); From 4ac0795eac77e08f2ef8b7c094d19f2570eb9126 Mon Sep 17 00:00:00 2001 From: Jon Zepernick Date: Wed, 26 Mar 2014 19:13:37 -0400 Subject: [PATCH 2/6] Clean up Button touch logic --- src/js/button.js | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/src/js/button.js b/src/js/button.js index 75c42937f5..043275b454 100644 --- a/src/js/button.js +++ b/src/js/button.js @@ -16,7 +16,7 @@ vjs.Button = vjs.Component.extend({ vjs.Component.call(this, player, options); var treatTouchAsClick = false; - var touchLocation = null; + var firstTouch = null; this.on('touchstart', function(event) { // If more than one finger, we don't want to consider treating this as a click @@ -24,23 +24,16 @@ vjs.Button = vjs.Component.extend({ // Stop click and other mouse events from triggering also event.preventDefault(); treatTouchAsClick = true; - touchLocation = { - x: event.touches[0].pageX, - y: event.touches[0].pageY - }; + firstTouch = event.touches[0]; } }); this.on('touchmove', function(event) { if (event.touches.length > 1) { treatTouchAsClick = false; - } else if (touchLocation) { - var newTouch = { - x: event.touches[0].pageX, - y: event.touches[0].pageY - }; - var xdiff = newTouch.x - touchLocation.x; - var ydiff = newTouch.y - touchLocation.y; + } else if (firstTouch) { + var xdiff = event.touches[0].pageX - firstTouch.pageX; + var ydiff = event.touches[0].pageY - firstTouch.pageY; var touchDistance = Math.sqrt(xdiff * xdiff + ydiff * ydiff); // Some devices are really sensitive and will throw touchmoves for all but the slightest // of taps. So, if we moved only a small distance, let's still consider this a click. @@ -52,7 +45,7 @@ vjs.Button = vjs.Component.extend({ var self = this; this.on('touchend', function(event) { - touchLocation = null; + firstTouch = null; if (treatTouchAsClick) { self.onClick(event); } From 9899b928f78d9bf107fabba3e23b1c049b49f782 Mon Sep 17 00:00:00 2001 From: Jon Zepernick Date: Thu, 27 Mar 2014 10:52:32 -0400 Subject: [PATCH 3/6] Move Button tap logic to Component.emitTapEvents --- src/js/button.js | 38 ++------------------------------------ src/js/component.js | 35 +++++++++++++++++++++++++++++------ 2 files changed, 31 insertions(+), 42 deletions(-) diff --git a/src/js/button.js b/src/js/button.js index 043275b454..76c09efe1e 100644 --- a/src/js/button.js +++ b/src/js/button.js @@ -15,43 +15,9 @@ vjs.Button = vjs.Component.extend({ init: function(player, options){ vjs.Component.call(this, player, options); - var treatTouchAsClick = false; - var firstTouch = null; - - this.on('touchstart', function(event) { - // If more than one finger, we don't want to consider treating this as a click - if (event.touches.length == 1) { - // Stop click and other mouse events from triggering also - event.preventDefault(); - treatTouchAsClick = true; - firstTouch = event.touches[0]; - } - }); - - this.on('touchmove', function(event) { - if (event.touches.length > 1) { - treatTouchAsClick = false; - } else if (firstTouch) { - var xdiff = event.touches[0].pageX - firstTouch.pageX; - var ydiff = event.touches[0].pageY - firstTouch.pageY; - var touchDistance = Math.sqrt(xdiff * xdiff + ydiff * ydiff); - // Some devices are really sensitive and will throw touchmoves for all but the slightest - // of taps. So, if we moved only a small distance, let's still consider this a click. - if (touchDistance > 44) { - treatTouchAsClick = false; - } - } - }); - - var self = this; - this.on('touchend', function(event) { - firstTouch = null; - if (treatTouchAsClick) { - self.onClick(event); - } - event.preventDefault(); - }); + this.emitTapEvents(); + this.on('tap', this.onClick); this.on('click', this.onClick); this.on('focus', this.onFocus); this.on('blur', this.onBlur); diff --git a/src/js/component.js b/src/js/component.js index 82dde8fb23..e449ea0e3b 100644 --- a/src/js/component.js +++ b/src/js/component.js @@ -833,29 +833,52 @@ vjs.Component.prototype.onResize; * @private */ vjs.Component.prototype.emitTapEvents = function(){ - var touchStart, touchTime, couldBeTap, noTap; + var touchStart, firstTouch, touchTime, couldBeTap, noTap, + xdiff, ydiff, touchDistance; // Track the start time so we can determine how long the touch lasted touchStart = 0; + firstTouch = null; this.on('touchstart', function(event) { - // Record start time so we can detect a tap vs. "touch and hold" - touchStart = new Date().getTime(); - // Reset couldBeTap tracking - couldBeTap = true; + // If more than one finger, don't consider treating this as a click + if (event.touches.length === 1) { + firstTouch = event.touches[0]; + // Record start time so we can detect a tap vs. "touch and hold" + touchStart = new Date().getTime(); + // Reset couldBeTap tracking + couldBeTap = true; + } + }); + + this.on('touchmove', function(event) { + // If more than one finger, don't consider treating this as a click + if (event.touches.length > 1) { + couldBeTap = false; + } else if (firstTouch) { + // Some devices will throw touchmoves for all but the slightest of taps. + // So, if we moved only a small distance, this could still be a tap + // 44 pixels is somewhat abitrary, based on Apple HIG 44px min button size + xdiff = event.touches[0].pageX - firstTouch.pageX; + ydiff = event.touches[0].pageY - firstTouch.pageY; + touchDistance = Math.sqrt(xdiff * xdiff + ydiff * ydiff); + if (touchDistance > 44) { + couldBeTap = false; + } + } }); noTap = function(){ couldBeTap = false; }; // TODO: Listen to the original target. http://youtu.be/DujfpXOKUp8?t=13m8s - this.on('touchmove', noTap); this.on('touchleave', noTap); this.on('touchcancel', noTap); // When the touch ends, measure how long it took and trigger the appropriate // event this.on('touchend', function(event) { + firstTouch = null; // Proceed only if the touchmove/leave/cancel event didn't happen if (couldBeTap === true) { // Measure how long the touch lasted From f0e6b70687f80262b06fcefa23d97fdd4fb12503 Mon Sep 17 00:00:00 2001 From: Jon Zepernick Date: Thu, 27 Mar 2014 17:09:47 -0400 Subject: [PATCH 4/6] Tests for emitTapEvents movement threshold --- test/unit/component.js | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/test/unit/component.js b/test/unit/component.js index fd31df56e4..4060b2d1e0 100644 --- a/test/unit/component.js +++ b/test/unit/component.js @@ -229,7 +229,7 @@ test('should use a defined content el for appending children', function(){ }); test('should emit a tap event', function(){ - expect(1); + expect(2); // Fake touch support. Real touch support isn't needed for this test. var origTouch = vjs.TOUCH_ENABLED; @@ -241,13 +241,27 @@ test('should emit a tap event', function(){ comp.on('tap', function(){ ok(true, 'Tap event emitted'); }); - comp.trigger('touchstart'); + + // A touchstart followed by touchend should trigger a tap + vjs.trigger(comp.el(), {type: 'touchstart', touches: [{}]}); + comp.trigger('touchend'); + + // A touchmove with a lot of movement should not trigger a tap + vjs.trigger(comp.el(), {type: 'touchstart', touches: [ + { pageX: 0, pageY: 0 } + ]}); + vjs.trigger(comp.el(), {type: 'touchmove', touches: [ + { pageX: 100, pageY: 100 } + ]}); comp.trigger('touchend'); - // This second test should not trigger another tap event because - // a touchmove is happening - comp.trigger('touchstart'); - comp.trigger('touchmove'); + // A touchmove with not much movement should still allow a tap + vjs.trigger(comp.el(), {type: 'touchstart', touches: [ + { pageX: 0, pageY: 0 } + ]}); + vjs.trigger(comp.el(), {type: 'touchmove', touches: [ + { pageX: 10, pageY: 10 } + ]}); comp.trigger('touchend'); // Reset to orignial value From 254eb1b1ff6a60ff4caf4582e4bc310217eca432 Mon Sep 17 00:00:00 2001 From: Jon Zepernick Date: Wed, 2 Apr 2014 08:15:58 -0400 Subject: [PATCH 5/6] Use a var for tap movement threshold --- src/js/component.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/js/component.js b/src/js/component.js index e449ea0e3b..ceb119a9f1 100644 --- a/src/js/component.js +++ b/src/js/component.js @@ -834,12 +834,15 @@ vjs.Component.prototype.onResize; */ vjs.Component.prototype.emitTapEvents = function(){ var touchStart, firstTouch, touchTime, couldBeTap, noTap, - xdiff, ydiff, touchDistance; + xdiff, ydiff, touchDistance, tapMovementThreshold; // Track the start time so we can determine how long the touch lasted touchStart = 0; firstTouch = null; + // Maximum movement allowed during a touch event to still be considered a tap + tapMovementThreshold = 22; + this.on('touchstart', function(event) { // If more than one finger, don't consider treating this as a click if (event.touches.length === 1) { @@ -858,11 +861,10 @@ vjs.Component.prototype.emitTapEvents = function(){ } else if (firstTouch) { // Some devices will throw touchmoves for all but the slightest of taps. // So, if we moved only a small distance, this could still be a tap - // 44 pixels is somewhat abitrary, based on Apple HIG 44px min button size xdiff = event.touches[0].pageX - firstTouch.pageX; ydiff = event.touches[0].pageY - firstTouch.pageY; touchDistance = Math.sqrt(xdiff * xdiff + ydiff * ydiff); - if (touchDistance > 44) { + if (touchDistance > tapMovementThreshold) { couldBeTap = false; } } From 771bbe74349cdf84b6244d67557209d181487780 Mon Sep 17 00:00:00 2001 From: Jon Zepernick Date: Wed, 2 Apr 2014 09:06:22 -0400 Subject: [PATCH 6/6] Prevent taps triggering clicks --- src/js/component.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/js/component.js b/src/js/component.js index ceb119a9f1..51ea4ac269 100644 --- a/src/js/component.js +++ b/src/js/component.js @@ -887,6 +887,7 @@ vjs.Component.prototype.emitTapEvents = function(){ touchTime = new Date().getTime() - touchStart; // The touch needs to be quick in order to consider it a tap if (touchTime < 250) { + event.preventDefault(); // Don't let browser turn this into a click this.trigger('tap'); // It may be good to copy the touchend event object and change the // type to tap, if the other event properties aren't exact after