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

fix(ngTouch): make tests pass on Chrome with touch enabled #11493

Closed

Conversation

gkalpak
Copy link
Member

@gkalpak gkalpak commented Apr 2, 2015

Using the initTouchEvent() method (introduces in 06a9f0a) did not correctly initialize the event, nor did the event get dispatched on the target element (e.g. on desktop Chrome with touch-events enabled).

Using the Event constructor and manually attaching a TouchList, works around the issue (although not a proper fix).

/ping @mzgol, @petebacondarwin

Using the `initTouchEvent()` method (introduces in 06a9f0a) did not
correctly initialize the event, nor did the event get dispatched on
the target element (e.g. on desktop Chrome with touch-events enabled).
Using the `Event` constructor and manually attaching a `TouchList`,
works around the issue (although not a proper fix).
@gkalpak gkalpak force-pushed the ngTouch-fix-tests-on-chrome-with-touch branch from 65a6412 to 7a87394 Compare April 2, 2015 17:52
gkalpak added a commit to gkalpak/angular.js that referenced this pull request Apr 2, 2015
@gkalpak
Copy link
Member Author

gkalpak commented Apr 2, 2015

Before merging this, @mzgol suggested making sure that:
If we revert changes from 06a9f0a in src/ngTouch/directive/ngClick.js and src/ngTouch/swipe.js (and only those two), then tests fail on iOS.

I submitted #11498 to verify this and indeed tests seem to still fail on iOS (e.g. here and there).

/fyi @mzgol, @petebacondarwin

@petebacondarwin
Copy link
Contributor

Awesome! Thanks @gkalpak and @mzgol

@petebacondarwin
Copy link
Contributor

I'll merge
(after fixing the typo in the commit msg :-))

gkalpak added a commit that referenced this pull request Apr 2, 2015
…h enabled Chrome

On certain browsers (e.g. on desktop Chrome with touch-events enabled),
using the `initTouchEvent()` method (introduced in 06a9f0a) did not
correctly initialize the event, nor did the event get dispatched on
the target element.

Using the `Event` constructor and manually attaching a `TouchList`,
works around the issue (although not a proper fix).

Fixes #11471
Closes #11493
gkalpak added a commit that referenced this pull request Apr 2, 2015
…h enabled Chrome

On certain browsers (e.g. on desktop Chrome with touch-events enabled),
using the `initTouchEvent()` method (introduced in 06a9f0a) did not
correctly initialize the event, nor did the event get dispatched on
the target element.

Using the `Event` constructor and manually attaching a `TouchList`,
works around the issue (although not a proper fix).

Fixes #11471
Closes #11493
@gkalpak gkalpak changed the title fix(ntTouch): make tests pass on Chrome with touch enabled fix(ngTouch): make tests pass on Chrome with touch enabled Apr 2, 2015
@gkalpak gkalpak deleted the ngTouch-fix-tests-on-chrome-with-touch branch April 2, 2015 21:44
@mgol
Copy link
Member

mgol commented Apr 3, 2015

Nice job!

@gkalpak
Copy link
Member Author

gkalpak commented Apr 3, 2015

Standing on the shoulders of giants 😛

@petebacondarwin
Copy link
Contributor

@gkalpak - you can already see the commits in the stream here because I changed the commit messages to reference this PR (with Closes #11493, etc).

@petebacondarwin
Copy link
Contributor

But yes, thanks to you and @mzgol for sorting this out.

@gkalpak
Copy link
Member Author

gkalpak commented Apr 4, 2015

@petebacondarwin, you are right. I was incorrectly looking for a message mentioning you ;)

Deleted the redundant message.

netman92 pushed a commit to netman92/angular.js that referenced this pull request Aug 8, 2015
…h enabled Chrome

On certain browsers (e.g. on desktop Chrome with touch-events enabled),
using the `initTouchEvent()` method (introduced in 06a9f0a) did not
correctly initialize the event, nor did the event get dispatched on
the target element.

Using the `Event` constructor and manually attaching a `TouchList`,
works around the issue (although not a proper fix).

Fixes angular#11471
Closes angular#11493
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants