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

Allowing angular-touch to work with jQuery. #8584

Closed
wants to merge 2 commits into from
Closed

Conversation

mcmar
Copy link

@mcmar mcmar commented Aug 12, 2014

Angular includes a small subset of jQuery out of the box called jqLite. Normally, angular wraps all the elements it passes to its directives in jqLite. When jQuery is loaded ahead of angular, angular instead wraps its elements in the full implementation of jQuery. Additionally, angular wraps its events in jQuery. These jQuery events work throughout most of angular. Angular-touch currently was not working with those jQuery-wrapped events. Touches were registering inconsistently and rarely when using the ngClick directive. This commit should fix that. I took it from another person I found on github. I'm not sure why his changes were never merged into the main angular.js repo, but I've verified that it works and I decided to submit a pull request for it. There is a bug/issue thread I found relating to this. It is linked below.
#4001

jasonals and others added 2 commits October 2, 2013 18:30
When jQuery is included touches are found on event.originalEvent
@caitp
Copy link
Contributor

caitp commented Aug 12, 2014

I'm not sure where you found @jasonals' patch, but it doesn't look unreasonable (to my eyes --- @mzgol might have a better idea).

I'm not sure how we can write a test for this, though

@caitp
Copy link
Contributor

caitp commented Aug 12, 2014

(if we do merge this, we will take @jasonals' patch, add tests, and rebase it properly against the master branch)

@petebacondarwin
Copy link
Member

@mcmar or @jasonals - is it possible that you could create a unit test that fails before this patch? We cannot merge this PR without one?

@pomerantsev
Copy link
Contributor

@petebacondarwin - this issue has been annoying for me too, so I decided to try creating some tests for it. I could unfortunately come up with just one that would fail without the patch, and I created PR #10797 that has the test in it.
The problem is the way browserTrigger triggers touch events. It creates a mouseEvent, so the behavior of ngClick.js differs in tests and on real devices. For example, on touchstart, event.clientX and event.clientY would be undefined without the patch, so on touchend, click would never get triggered because dist will be undefined too.

I'll be happy to write more tests, but I haven't been able to find a way to emulate touch events properly: document.createEvent('TouchEvent') works fine in Chrome in mobile emulation mode, but throws an exception in normal mode. And I can't see a way to run Karma tests in mobile emulation mode. Some people are suggesting to use Chrome extensions, but I'm not sure if it's a viable solution for Angular.

Any way you could help me with it? Or maybe just accept the PR in its current state?

@pomerantsev pomerantsev mentioned this pull request Jan 18, 2015
@mgol
Copy link
Member

mgol commented Jan 22, 2015

LGTM. Generally, in jQuery the original event is always available at event.originalEvent, event copies some properties and normalizes some that differ between browsers. Touches are not copied so originalEvent should be used everywhere.

Why didn't current unit tests catch this? They run on jQuery as well.

@mgol
Copy link
Member

mgol commented Jan 22, 2015

@mcmar you have linting errors, see why the Travis build failed.

@pomerantsev
Copy link
Contributor

@mzgol - I've fixed the linting errors in PR #10797 that contains changes from this PR.
I wasn't sure how to add a commit to a PR that was not mine.
Current unit tests didn't catch this because, as I commented earlier, browserTrigger doesn't actually issue a touch event.

@mgol
Copy link
Member

mgol commented Jan 22, 2015

@pomerantsev I commented under #10797.

mgol added a commit that referenced this pull request Mar 31, 2015
If jQuery was used with Angular the touch logic was looking for touches
under the original event objects. However, jQuery wraps all events, keeping
the original one under the originalEvent property and copies/normalizes some
of event properties. Not all properties are copied, e.g. touches which caused
them to not be recognized properly.

Thanks to @mcmar & @pomerantsev for original patch ideas.

Fixes #4001
Closes #8584
Closes #10797
@mgol
Copy link
Member

mgol commented Mar 31, 2015

I went in another direction as just assigning $event.originalEvent to $event breaks the general assumption that $event is a jQuery/jqLite event object, not a native one. Please see #11471.

mgol added a commit to mgol/angular.js that referenced this pull request Mar 31, 2015
If jQuery was used with Angular the touch logic was looking for touches
under the original event objects. However, jQuery wraps all events, keeping
the original one under the originalEvent property and copies/normalizes some
of event properties. Not all properties are copied, e.g. touches which caused
them to not be recognized properly.

Thanks to @mcmar & @pomerantsev for original patch ideas.

Fixes angular#4001
Closes angular#8584
Closes angular#10797
mgol added a commit to mgol/angular.js that referenced this pull request Mar 31, 2015
If jQuery was used with Angular the touch logic was looking for touches
under the original event object. However, jQuery wraps all events, keeping
the original one under the originalEvent property and copies/normalizes some
of event properties. Not all properties are copied, e.g. touches which caused
them to not be recognized properly.

Thanks to @mcmar & @pomerantsev for original patch ideas.

Fixes angular#4001
Closes angular#8584
Closes angular#10797
mgol added a commit to mgol/angular.js that referenced this pull request Mar 31, 2015
If jQuery was used with Angular the touch logic was looking for touches
under the original event object. However, jQuery wraps all events, keeping
the original one under the originalEvent property and copies/normalizes some
of event properties. Not all properties are copied, e.g. touches which caused
them to not be recognized properly.

Thanks to @mcmar & @pomerantsev for original patch ideas.

Fixes angular#4001
Closes angular#8584
Closes angular#10797
mgol added a commit to mgol/angular.js that referenced this pull request Mar 31, 2015
If jQuery was used with Angular the touch logic was looking for touches
under the original event object. However, jQuery wraps all events, keeping
the original one under the originalEvent property and copies/normalizes some
of event properties. Not all properties are copied, e.g. touches which caused
them to not be recognized properly.

Thanks to @mcmar & @pomerantsev for original patch ideas.

Fixes angular#4001
Closes angular#8584
Closes angular#10797
petebacondarwin pushed a commit to petebacondarwin/angular.js that referenced this pull request Apr 2, 2015
If jQuery was used with Angular the touch logic was looking for touches
under the original event object. However, jQuery wraps all events, keeping
the original one under the originalEvent property and copies/normalizes some
of event properties. Not all properties are copied, e.g. touches which caused
them to not be recognized properly.

Thanks to @mcmar & @pomerantsev for original patch ideas.

Fixes angular#4001
Closes angular#8584
Closes angular#10797
@mgol mgol closed this in 06a9f0a Apr 2, 2015
mgol added a commit that referenced this pull request Apr 2, 2015
If jQuery was used with Angular the touch logic was looking for touches
under the original event object. However, jQuery wraps all events, keeping
the original one under the originalEvent property and copies/normalizes some
of event properties. Not all properties are copied, e.g. touches which caused
them to not be recognized properly.

Thanks to @mcmar & @pomerantsev for original patch ideas.

Fixes #4001
Closes #8584
Closes #10797
Closes #11488
netman92 pushed a commit to netman92/angular.js that referenced this pull request Aug 8, 2015
If jQuery was used with Angular the touch logic was looking for touches
under the original event object. However, jQuery wraps all events, keeping
the original one under the originalEvent property and copies/normalizes some
of event properties. Not all properties are copied, e.g. touches which caused
them to not be recognized properly.

Thanks to @mcmar & @pomerantsev for original patch ideas.

Fixes angular#4001
Closes angular#8584
Closes angular#10797
Closes angular#11488
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants