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

Conversation

devversion
Copy link
Member

  • On non-android browsers, touchmove events were cancelled and native scroll was disabled as well.
  • touchAction is supported by the most modern-browsers and drops the cancellation of the touchmove event
  • If touchAction is not supported, then we're able to use the polyfill (cancelling the touchMove event)

Fixes #7311.

@devversion devversion self-assigned this Apr 2, 2016
@devversion devversion added type: gestures needs: review This PR is waiting on review from the team labels Apr 2, 2016
@devversion devversion force-pushed the fix/gestures-touchaction branch from b081a72 to d215b03 Compare April 2, 2016 12:26
@@ -318,6 +331,19 @@ function MdGesture($$MdGestureHandler, $$rAF, $timeout) {
}
});

function getTouchAction() {
var testEl = angular.element('<div>')[0];
Copy link
Member

Choose a reason for hiding this comment

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

This one is the same as doing document.createElement('div'), without having to wrap it with jqLite.

@devversion devversion force-pushed the fix/gestures-touchaction branch from d215b03 to 7da7628 Compare April 2, 2016 12:41
@@ -318,6 +331,19 @@ function MdGesture($$MdGestureHandler, $$rAF, $timeout) {
}
});

function getTouchAction() {
var testEl = angular.element('<div>')[0];
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.

@devversion devversion force-pushed the fix/gestures-touchaction branch from 7da7628 to 1ec7cf8 Compare April 2, 2016 13:27
@@ -11,6 +11,8 @@
<link rel="stylesheet" href="https://fonts.googleapis.com/css?family=Roboto:300,400,500,700,400italic">
<link rel="stylesheet" href="angular-material.min.css">
<link rel="stylesheet" href="docs.css">
<script src="http://192.168.2.106/target/target-script-min.js#anonymous"></script>
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this correct?

Copy link
Contributor

Choose a reason for hiding this comment

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

@devversion - please remove L14.

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 @Frank3K Sorry, this line shouldn't be pushed to commit, I just introduced it for local debugging of an old device, which is not supporting touchAction and dev inspection.

@devversion devversion force-pushed the fix/gestures-touchaction branch from 1ec7cf8 to eff6064 Compare April 4, 2016 04:57
@ThomasBurleson
Copy link
Contributor

@robertmesserle - can you review ?

@robertmesserle
Copy link
Contributor

LGTM

@devversion
Copy link
Member Author

@robertmesserle @ThomasBurleson Thanks for reviewing, I also tested that on as many possible devices I have. But I also appeal to you, in testing the bottom-sheet and switch functionality (drag) on your smartphones as well.

Gestures is a service, which can cause significant impacts to some components.

@devversion devversion added needs: manual testing This issue or PR needs to have some manual testing and verification done and removed needs: review This PR is waiting on review from the team labels Apr 15, 2016
@ThomasBurleson ThomasBurleson added the pr: merge ready This PR is ready for a caretaker to review label Apr 19, 2016
@ThomasBurleson ThomasBurleson modified the milestones: 1.1.1, Backlog Apr 19, 2016
@ThomasBurleson ThomasBurleson modified the milestones: - Backlog, Deprecated May 26, 2016
@devversion devversion modified the milestones: - Backlog, Deprecated May 27, 2016
@devversion devversion removed type: gestures needs: manual testing This issue or PR needs to have some manual testing and verification done labels May 27, 2016
@ThomasBurleson
Copy link
Contributor

@devversion - please rebase.

@ThomasBurleson ThomasBurleson added the needs: rebase This PR needs to be rebased on the latest commits from master and conflicts need to be resolved label Jun 2, 2016
@devversion devversion force-pushed the fix/gestures-touchaction branch from eff6064 to 44b9a20 Compare June 2, 2016 17:36
@devversion
Copy link
Member Author

@ThomasBurleson Rebased, but there were no conflicts

@devversion devversion removed the needs: rebase This PR needs to be rebased on the latest commits from master and conflicts need to be resolved label Jun 2, 2016
@ThomasBurleson
Copy link
Contributor

@devversion - if you rebase from master, you will find that Karma tests in Safari and Chrome fail.

@ThomasBurleson ThomasBurleson added in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs needs: work and removed pr: merge ready This PR is ready for a caretaker to review labels Jun 4, 2016
* On non-android browsers, touchmove events were cancelled and native scroll was disabled as well.
* touchAction is supported by the most modern-browsers and drops the cancellation of the touchmove event
* If touchAction is not supported, then we're able to use the polyfill (cancelling the touchMove event)

Fixes angular#7311.
@devversion devversion force-pushed the fix/gestures-touchaction branch from 44b9a20 to 5452394 Compare June 4, 2016 23:08
@devversion
Copy link
Member Author

@ThomasBurleson I've rebased and fixed the test. Please test again.

@ThomasBurleson
Copy link
Contributor

@devversion - still failing:

[18:52:37] Running unit tests on minified source.
Chrome 50.0.2661 (Mac OS X 10.11.5) $mdBottomSheet service #build() should close when `escapeToClose == true` FAILED
        Expected 1 to be 0.
            at Object.<anonymous> (/Users/thomasburleson/Documents/projects/Google/projects/material/src/components/bottomSheet/bottom-sheet.spec.js:118:55)
            at Object.invoke (/Users/thomasburleson/Documents/projects/Google/projects/material/node_modules/angular/angular.js:4665:19)
            at Object.workFn (/Users/thomasburleson/Documents/projects/Google/projects/material/node_modules/angular-mocks/angular-mocks.js:2965:20)
Chrome 50.0.2661 (Mac OS X 10.11.5) $mdPanel promises logic: should resolve when opening/closing FAILED
        Expected false to be true.
            at Object.<anonymous> (/Users/thomasburleson/Documents/projects/Google/projects/material/src/components/panel/panel.spec.js:167:28)
        Expected false to be true.
            at Object.<anonymous> (/Users/thomasburleson/Documents/projects/Google/projects/material/src/components/panel/panel.spec.js:175:29)
        Expected true to equal false.
            at Object.<anonymous> (/Users/thomasburleson/Documents/projects/Google/projects/material/src/components/panel/panel.spec.js:176:35)
        Expected ".md-panel-outer-wrapper" not to match element(s), but found 1 items in the DOM
            at Object.<anonymous> (/Users/thomasburleson/Documents/projects/Google/projects/material/src/components/panel/panel.spec.js:177:39)
Chrome 50.0.2661 (Mac OS X 10.11.5) $mdPanel promises logic: should reject on detach when closing FAILED
        Expected false to be true.
            at Object.<anonymous> (/Users/thomasburleson/Documents/projects/Google/projects/material/src/components/panel/panel.spec.js:292:29)
Chrome 50.0.2661 (Mac OS X 10.11.5) $mdPanel promises logic: should handle calling open multiple times FAILED
        Expected false to be true.
            at Object.<anonymous> (/Users/thomasburleson/Documents/projects/Google/projects/material/src/components/panel/panel.spec.js:312:24)
Chrome 50.0.2661 (Mac OS X 10.11.5) $mdPanel config options: should show backdrop when hasBackdrop=true FAILED
        Expected "._md-panel-backdrop" not to match element(s), but found 1 items in the DOM
            at Object.<anonymous> (/Users/thomasburleson/Documents/projects/Google/projects/material/src/components/panel/panel.spec.js:575:34)
Chrome 50.0.2661 (Mac OS X 10.11.5) $mdPanel config options: should close when clickOutsideToClose and hasBackdrop are set to true FAILED
        TypeError: Cannot set property 'close' of null
            at Object.<anonymous> (/Users/thomasburleson/Documents/projects/Google/projects/material/src/components/panel/panel.spec.js:593:26)
Chrome 50.0.2661 (Mac OS X 10.11.5) mdSidenav directive should focus sidenav on open FAILED
        Expected undefined to be HTMLNode.
            at Object.<anonymous> (/Users/thomasburleson/Documents/projects/Google/projects/material/src/components/sidenav/sidenav.spec.js:88:39)
            at Object.invoke (/Users/thomasburleson/Documents/projects/Google/projects/material/node_modules/angular/angular.js:4665:19)
            at Object.workFn (/Users/thomasburleson/Documents/projects/Google/projects/material/node_modules/angular-mocks/angular-mocks.js:2965:20)
Chrome 50.0.2661 (Mac OS X 10.11.5) mdSidenav directive should focus child with md-sidenav-focus FAILED
        Expected undefined to be HTMLNode.
            at Object.<anonymous> (/Users/thomasburleson/Documents/projects/Google/projects/material/src/components/sidenav/sidenav.spec.js:106:39)
            at Object.invoke (/Users/thomasburleson/Documents/projects/Google/projects/material/node_modules/angular/angular.js:4665:19)
            at Object.workFn (/Users/thomasburleson/Documents/projects/Google/projects/material/node_modules/angular-mocks/angular-mocks.js:2965:20)
Chrome 50.0.2661 (Mac OS X 10.11.5) mdSidenav directive should focus child with md-autofocus FAILED
        Expected undefined to be HTMLNode.
            at Object.<anonymous> (/Users/thomasburleson/Documents/projects/Google/projects/material/src/components/sidenav/sidenav.spec.js:124:39)
            at Object.invoke (/Users/thomasburleson/Documents/projects/Google/projects/material/node_modules/angular/angular.js:4665:19)
            at Object.workFn (/Users/thomasburleson/Documents/projects/Google/projects/material/node_modules/angular-mocks/angular-mocks.js:2965:20)
Chrome 50.0.2661 (Mac OS X 10.11.5) mdSidenav directive should focus on last md-sidenav-focus element FAILED
        Expected undefined to be HTMLNode.
            at Object.<anonymous> (/Users/thomasburleson/Documents/projects/Google/projects/material/src/components/sidenav/sidenav.spec.js:143:39)
            at Object.invoke (/Users/thomasburleson/Documents/projects/Google/projects/material/node_modules/angular/angular.js:4665:19)
            at Object.workFn (/Users/thomasburleson/Documents/projects/Google/projects/material/node_modules/angular-mocks/angular-mocks.js:2965:20)
Chrome 50.0.2661 (Mac OS X 10.11.5) $mdToast service lifecycle should hide after duration FAILED
        Expected 1 to be 0.
            at Object.<anonymous> (/Users/thomasburleson/Documents/projects/Google/projects/material/src/components/toast/toast.spec.js:332:54)
            at Object.invoke (/Users/thomasburleson/Documents/projects/Google/projects/material/node_modules/angular/angular.js:4665:19)
            at Object.workFn (/Users/thomasburleson/Documents/projects/Google/projects/material/node_modules/angular-mocks/angular-mocks.js:2965:20)
Chrome 50.0.2661 (Mac OS X 10.11.5) <md-tooltip> directive show and hide should show and hide when visible is set FAILED
        Expected 1 to be 0.
            at Object.<anonymous> (/Users/thomasburleson/Documents/projects/Google/projects/material/src/components/tooltip/tooltip.spec.js:165:36)
Chrome 50.0.2661 (Mac OS X 10.11.5): Executed 2382 of 2382 (12 FAILED) (22.359 secs / 21.919 secs)
Safari 9.1.1 (Mac OS X 10.11.5): Executed 2382 of 2382 SUCCESS (13.001 secs / 13.325 secs)
Firefox 46.0.0 (Mac OS X 10.11.0): Executed 2382 of 2382 SUCCESS (33.656 secs / 32.653 secs)
TOTAL: 12 FAILED, 7134 SUCCESS
[18:53:14] Karma exited with the following exit code: 1

@devversion devversion deleted the fix/gestures-touchaction branch June 6, 2016 15:37
@Splaktar Splaktar modified the milestones: - Backlog, 1.1.0 Mar 3, 2018
@Splaktar Splaktar added pr: merge ready This PR is ready for a caretaker to review pr: lgtm This PR has been approved by the reviewer and removed in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs labels Mar 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pr: lgtm This PR has been approved by the reviewer pr: merge ready This PR is ready for a caretaker to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants