fix(gestures): slider and swipe touch #10314
Conversation
- slider was based on mdGesture service that set `touch-action: pan-x` in case the gesture was horizontal; according the google pointer-events documentation: ``` pan-x: The browser is only allowed to perform the horizontal scroll default action. ``` that means we should had enabled `pan-y` and vice-versa instead. - swipe directive didn't had any `touch-action` css styles, added `touch-action: none` on the directive element on link fixes #10294, #10187, #10145
googlebot
added
the
cla: yes
There was a problem hiding this comment.
Tested on touchscreen hardware
- Chrome v55 on Windows 10
- Chrome v55 on ChromeOS
- Chrome v55 on Android 7.0
- Microsoft Edge on Windows 10
- Firefox v51* on Windows 10
* via device emulator, since Firefox did not properly detect my touchscreen
iOS device tests passed via Browserstack testing
Internet Explorer 11 isn't working, but it wasn't working in master either, so no regression noted.
| @@ -248,7 +248,7 @@ function MdGesture($$MdGestureHandler, $$rAF, $timeout) { | |||
| 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'; | |||
| element[0].style[touchActionProperty] = options.horizontal ? 'pan-y' : 'pan-x'; | |||
There was a problem hiding this comment.
I think I initially wrote that lines and I think it was intentional to only allow pan-x for horizontal gestures and pan-y for vertical gestures.
Can you explain more why this needs to be the other way around?
| State | Value | Description |
|---|---|---|
| horizontal | pan-x | Only recognize X-Axis movement s |
| vertical | pan-y | Only recognize Y-Axis movements |
There was a problem hiding this comment.
Cause it's the other way around:
| Value | Description |
|---|---|
| pan-x | The browser is only allowed to perform the horizontal scroll default action. |
| pan-y | The browser is only allowed to perform the vertical scroll default action. |
Therefor we need
| State | Value |
|---|---|
| horizontal | pan-y |
| vertical | pan-x |
|
Tried this with Chrome 55 and Edge on a MS Surface and everything was peachy. Thanks! LGTM |
kara
added
pr: merge ready
|
This now break my application; I have an md-tabs with md-dynamic-height inside a layout-full and an md-content. Vertical scrolling in the md-content no longer works. |
|
Same scenario described by @crownofapollo . Vertical srolling in md-content no longer works with 1.1.3 |
|
@crownofapollo @xcafebabe what touch device? os? can you file a proper issue with a plunker/codepen so we can investigate further? |
|
Hi @EladBezalel, I send you two codepens with same code, turn on Device mode in Chrome and swipe up and down. With 1.1.2: swipe up-down works. With 1.1.3: swipe up-down does not work Tested with Nevertheless thanks for the great work you are doing! :) |
|
@EladBezalel Thanks for looking at this, but I'm also unable to swipe up-down too - Android 6 (HTC1) |
touch-action: pan-xin case the gesture was horizontal;according the google pointer-events documentation:
that means we should had enabled
pan-yand vice-versa instead.touch-actioncss styles, addedtouch-action: noneon the directive element on linkfixes #10294, #10187, #10145