Fix Chrome 55+ mobile & new pad sensitivity option #330
Conversation
inuyaksa
commented
May 8, 2017
inuyaksa
commented
- fixed Chrome 55+ mobile pich/touch flickering bug Pinch zoom broken in Chrome v55.02883.91 on Mobile #303
- fixed Chrome passive event warning Unable to preventDefault inside passive event listener due to target being treated as passive. #328
- add new otion "pan sensitivity"
|
I think you should have done this in 3 different PRs... Also, looking at the PR diff, in one place you're adding 3 lines of commented code, and a few lines below you're adding some trailing space. |
|
Sorry, what do you mean about PRs? Feel free to use my corrections for your project. I'm using since 2 month and it works well on all configurations desktop/mobile. |
|
PR -> Pull requests.. There's too many changes here, and they're not related. |
dist/jquery.panzoom.js
Outdated
| @@ -1125,6 +1128,10 @@ | |||
| if (this.panning) { | |||
| return; | |||
| } | |||
|
|
|||
| var type = event.type; | |||
| if (window.PointerEvents&&(type === 'touchstart')) return false; // fix double events pointer/touch on Chrome >=55 #303 | |||
There was a problem hiding this comment.
window.PointerEvents??? Is there such a thing? I think this is a typo. And you meant window.PointerEvent. See https://developers.google.com/web/updates/2016/10/pointer-events
dist/jquery.panzoom.js
Outdated
| @@ -310,6 +310,9 @@ | |||
| // Pan only when the scale is greater than minScale | |||
| panOnlyWhenZoomed: false, | |||
|
|
|||
| // Pan sensitivity, min pixels to start to panning | |||
| panSensitivity: 4, | |||
|
|
|||
There was a problem hiding this comment.
New functionality move to a different PR.
dist/jquery.panzoom.js
Outdated
| var type = event.type; | ||
|
|
||
| // Indicate that we are currently panning | ||
| //this.panning = true; |
There was a problem hiding this comment.
Dead code.. Don't add stuff like this in a PR.
dist/jquery.panzoom.js
Outdated
|
|
||
| // Use proper events | ||
| if (type === 'pointerdown') { | ||
| moveEvent = 'pointermove'; | ||
| endEvent = 'pointerup'; | ||
| } else if (type === 'touchstart') { | ||
| moveEvent = 'touchmove'; | ||
| endEvent = 'touchend'; | ||
| endEvent = 'touchend'; |
There was a problem hiding this comment.
Trailing whitespace.. This change should not be here.
dist/jquery.panzoom.js
Outdated
| @@ -1160,9 +1169,6 @@ | |||
| // Remove any transitions happening | |||
| this.transition(true); | |||
|
|
|||
| // Indicate that we are currently panning | |||
| this.panning = true; | |||
|
|
|||
There was a problem hiding this comment.
This seems to be because of the new Pan sensitivity functionality.. Move to a different PR, please.
dist/jquery.panzoom.js
Outdated
| if ((Math.abs(coords.pageX-startPageX)<self.options.panSensitivity)&&(Math.abs(coords.pageY-startPageY)<self.options.panSensitivity)) return true; | ||
| } | ||
|
|
||
| self.panning = true; |
There was a problem hiding this comment.
Pan sensitivity -> different PR.
dist/jquery.panzoom.js
Outdated
| @@ -1196,7 +1202,8 @@ | |||
|
|
|||
| var move = function(e) { | |||
| var coords; | |||
| e.preventDefault(); | |||
| //e.preventDefault(); // chrome passive event warning https://www.chromestatus.com/features/5093566007214080 #328 | |||
| e.stopPropagation(); | |||
There was a problem hiding this comment.
You can add a comment that the use of stopPropagation is because preventDefault is not allowed.. But don't comment out the old code.
This reverts commit e66b336.
|
That reverted all changes. The pull request is now useless, so I'll close it. |
|
I'm working on it, you so fast and close my PR :) |
|
@inuyaksa Sounds good. Thanks for the help! |
