Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Fix issue with Jquery for ngMobile #3198

Closed
wants to merge 4 commits into from
Closed

Fix issue with Jquery for ngMobile #3198

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
81f99c0
issue:
Jul 11, 2013
ecbe81b
issue:
Jul 11, 2013
986adcb
fix for broken unit test, and add two more unit tests for tap event
ethanli83 Jul 13, 2013
798e880
Improve ngClick.js base on discussions in issue #3198 (https://github…
Aug 2, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion src/ngMobile/directive/ngClick.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,10 @@ ngMobile.directive('ngClick', ['$parse', '$timeout', '$rootElement',
return; // Too old.
}

// retrieve original event if it is wrapped by jquery
// the original event will have an array of touches
event = event.originalEvent || event;

var touches = event.touches && event.touches.length ? event.touches : [event];
var x = touches[0].clientX;
var y = touches[0].clientY;
Expand All @@ -141,6 +145,9 @@ ngMobile.directive('ngClick', ['$parse', '$timeout', '$rootElement',
// Global touchstart handler that creates an allowable region for a click event.
// This allowable region can be removed by preventGhostClick if we want to bust it.
function onTouchStart(event) {
// retrieve original event which has touches
event = event.originalEvent || event;

var touches = event.touches && event.touches.length ? event.touches : [event];
var x = touches[0].clientX;
var y = touches[0].clientY;
Expand Down Expand Up @@ -187,6 +194,8 @@ ngMobile.directive('ngClick', ['$parse', '$timeout', '$rootElement',

element.on('touchstart', function(event) {
tapping = true;
event = event.originalEvent || event;

tapElement = event.target ? event.target : event.srcElement; // IE uses srcElement.
// Hack for Safari, which can target text nodes instead of containers.
if(tapElement.nodeType == 3) {
Expand All @@ -213,6 +222,7 @@ ngMobile.directive('ngClick', ['$parse', '$timeout', '$rootElement',

element.on('touchend', function(event) {
var diff = Date.now() - startTime;
event = event.originalEvent || event;

var touches = (event.changedTouches && event.changedTouches.length) ? event.changedTouches :
((event.touches && event.touches.length) ? event.touches : [event]);
Expand All @@ -221,7 +231,7 @@ ngMobile.directive('ngClick', ['$parse', '$timeout', '$rootElement',
var y = e.clientY;
var dist = Math.sqrt( Math.pow(x - touchStartX, 2) + Math.pow(y - touchStartY, 2) );

if (tapping && diff < TAP_DURATION && dist < MOVE_TOLERANCE) {
if (diff < TAP_DURATION && dist < MOVE_TOLERANCE) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This condition is important. touchend events can come following moves and other things that invalidate a straight-up-and-down tap, and the tapping flag is set on touchstart and cleared for those illegal motions.

As an example, without this, it's possible to put a finger down, move it two inches away, move it back so it's within the MOVE_TOLERANCE again, and release, triggering a tap.

Was this deliberately removed? If so, why? If not, please put it back when you squash into one commit.

// Call preventGhostClick so the clickbuster will catch the corresponding click.
preventGhostClick(x, y);

Expand Down
26 changes: 26 additions & 0 deletions test/ngMobile/directive/ngClickSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,18 @@ describe('ngClick (mobile)', function() {
expect($rootScope.tapped).toBeUndefined();
}));

it('should click if the touchend is within mvoe tolerance', inject(function($rootScope, $compile, $rootElement) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: mvoe

element = $compile('<div ng-click="tapped = true"></div>')($rootScope);
$rootElement.append(element);
$rootScope.$digest();

expect($rootScope.tapped).toBeUndefined();

browserTrigger(element, 'touchstart', [], 10, 10);
browserTrigger(element, 'touchend', [], 15, 15);

expect($rootScope.tapped).toEqual(true);
}));

it('should not click if a touchmove comes before touchend', inject(function($rootScope, $compile, $rootElement) {
element = $compile('<div ng-click="tapped = true"></div>')($rootScope);
Expand All @@ -95,6 +107,20 @@ describe('ngClick (mobile)', function() {
expect($rootScope.tapped).toBeUndefined();
}));

it('should click if the touchend is within mvoe tolerance, even there is a touchmove before touchend', inject(function($rootScope, $compile, $rootElement) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here too.

element = $compile('<div ng-click="tapped = true"></div>')($rootScope);
$rootElement.append(element);
$rootScope.$digest();

expect($rootScope.tapped).toBeUndefined();

browserTrigger(element, 'touchstart', [], 10, 10);
browserTrigger(element, 'touchmove');
browserTrigger(element, 'touchend', [], 15, 15);

expect($rootScope.tapped).toEqual(true);
}));

it('should add the CSS class while the element is held down, and then remove it', inject(function($rootScope, $compile, $rootElement) {
element = $compile('<div ng-click="tapped = true"></div>')($rootScope);
$rootElement.append(element);
Expand Down