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

fix(gestures): detect touch action and provide polyfill. #7857

Closed
Closed
Changes from all commits
Commits
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
42 changes: 37 additions & 5 deletions src/core/services/gesture/gesture.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ function MdGesture($$MdGestureHandler, $$rAF, $timeout) {
var userAgent = navigator.userAgent || navigator.vendor || window.opera;
var isIos = userAgent.match(/ipad|iphone|ipod/i);
var isAndroid = userAgent.match(/android/i);
var touchActionProperty = getTouchAction();
var hasJQuery = (typeof window.jQuery !== 'undefined') && (angular.element === window.jQuery);

var self = {
Expand Down Expand Up @@ -215,7 +216,7 @@ function MdGesture($$MdGestureHandler, $$rAF, $timeout) {
// If we don't preventDefault touchmove events here, Android will assume we don't
// want to listen to anymore touch events. It will start scrolling and stop sending
// touchmove events.
ev.preventDefault();
if (!touchActionProperty && ev.type === 'touchmove') ev.preventDefault();

// If the user moves greater than <maxDistance> pixels, stop the hold timer
// set in onStart
Expand All @@ -234,7 +235,7 @@ function MdGesture($$MdGestureHandler, $$rAF, $timeout) {
* The drag handler dispatches a drag event if the user holds and moves his finger greater than
* <minDistance> px in the x or y direction, depending on options.horizontal.
* The drag will be cancelled if the user moves his finger greater than <minDistance>*<cancelMultiplier> in
* the perpindicular direction. Eg if the drag is horizontal and the user moves his finger <minDistance>*<cancelMultiplier>
* the perpendicular direction. Eg if the drag is horizontal and the user moves his finger <minDistance>*<cancelMultiplier>
* pixels vertically, this handler won't consider the move part of a drag.
*/
.handler('drag', {
Expand All @@ -243,6 +244,18 @@ function MdGesture($$MdGestureHandler, $$rAF, $timeout) {
horizontal: true,
cancelMultiplier: 1.5
},
onSetup: function(element, options) {
if (touchActionProperty) {
// We check for horizontal to be false, because otherwise we would overwrite the default opts.
this.oldTouchAction = element[0].style[touchActionProperty];
element[0].style[touchActionProperty] = options.horizontal === false ? 'pan-y' : 'pan-x';
}
},
onCleanup: function(element) {
if (this.oldTouchAction) {
element[0].style[touchActionProperty] = this.oldTouchAction;
Copy link
Member

Choose a reason for hiding this comment

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

You can replace this one with .css as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

Same as above. This is not valid.

Copy link
Contributor

Choose a reason for hiding this comment

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

@devversion - what are you saying ^^ ?

Copy link
Member Author

Choose a reason for hiding this comment

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

@ThomasBurleson Yes, Kristiyan an I discussed in an comment above about the same suggestion.

We came to the resolution, that using jqLite's css is not valid, because jqLite is transforming the touchActionProperty an that would break the polyfill detection.

https://github.com/angular/angular.js/blob/master/src/jqLite.js#L145
image

Copy link
Contributor

Choose a reason for hiding this comment

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

There's also some overhead when you use jqLite functions. Most of the time it doesn't matter, but it's good to err on the side of performance in cases like there unless the css method provides some benefit.

}
},
onStart: function (ev) {
// For drag, require a parent to be registered with $mdGesture.register()
if (!this.state.registeredParent) this.cancel();
Expand All @@ -253,7 +266,7 @@ function MdGesture($$MdGestureHandler, $$rAF, $timeout) {
// If we don't preventDefault touchmove events here, Android will assume we don't
// want to listen to anymore touch events. It will start scrolling and stop sending
// touchmove events.
ev.preventDefault();
if (!touchActionProperty && ev.type === 'touchmove') ev.preventDefault();

if (!this.state.dragPointer) {
if (this.state.options.horizontal) {
Expand All @@ -277,7 +290,7 @@ function MdGesture($$MdGestureHandler, $$rAF, $timeout) {
this.dispatchDragMove(ev);
}
},
// Only dispatch dragmove events every frame; any more is unnecessray
// Only dispatch dragmove events every frame; any more is unnecessary
dispatchDragMove: $$rAF.throttle(function (ev) {
// Make sure the drag didn't stop while waiting for the next frame
if (this.state.isRunning) {
Expand Down Expand Up @@ -318,6 +331,19 @@ function MdGesture($$MdGestureHandler, $$rAF, $timeout) {
}
});

function getTouchAction() {
var testEl = document.createElement('div');
var vendorPrefixes = ['', 'webkit', 'Moz', 'MS', 'ms', 'o'];
Copy link
Member

Choose a reason for hiding this comment

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

According to Caniuse, there's only the ms vendor prefix.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thought the same, but HammerJS used that as well. And I'm sure it has a significance

Copy link
Member

Choose a reason for hiding this comment

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

Well it's probably not slowing down too much, but no other prefixes ,going back to the oldest browser version, weren't mentioned.

Copy link
Member Author

Choose a reason for hiding this comment

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

I know what you mean, this prefix looks not valid. But this logic has been ported from HammerJS and it seems to have a role. Otherwise we should file an issue and ask there.


for (var i = 0; i < vendorPrefixes.length; i++) {
var prefix = vendorPrefixes[i];
var property = prefix ? prefix + 'TouchAction' : 'touchAction';
if (angular.isDefined(testEl.style[property])) {
return property;
}
}
}

}

/**
Expand Down Expand Up @@ -346,6 +372,8 @@ function MdGestureHandler() {
dispatchEvent: hasJQuery ? jQueryDispatchEvent : nativeDispatchEvent,

// These are overridden by the registered handler
onSetup: angular.noop,
onCleanup: angular.noop,
onStart: angular.noop,
onMove: angular.noop,
onEnd: angular.noop,
Expand Down Expand Up @@ -395,7 +423,7 @@ function MdGestureHandler() {
return null;
},

// Called from $mdGesture.register when an element reigsters itself with a handler.
// Called from $mdGesture.register when an element registers itself with a handler.
// Store the options the user gave on the DOMElement itself. These options will
// be retrieved with getNearestParent when the handler starts.
registerElement: function (element, options) {
Expand All @@ -404,11 +432,15 @@ function MdGestureHandler() {
element[0].$mdGesture[this.name] = options || {};
element.on('$destroy', onDestroy);

self.onSetup(element, options || {});

return onDestroy;

function onDestroy() {
delete element[0].$mdGesture[self.name];
element.off('$destroy', onDestroy);

self.onCleanup(element, options || {});
}
}
};
Expand Down