-
Notifications
You must be signed in to change notification settings - Fork 287
Working Rotate Gesture Pull #304
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
@jywarren this is now working. Needs a lot of cleaning.. shouldn't all be inside of the load event of course and for testing purposes map moving is disabled. Need to add a Is this for touch screens only? currently it works on both (click or touch and drag). Note there needs to be some sort of logic to separate moving the image from rotating it. |
examples/listeners.html
Outdated
| lastTouchAlpha = touchInfo.alpha; | ||
| } | ||
| } else if (touchSpeed) { | ||
| spinSpeed = touchSpeed * touchInfo.radius / centerRadius; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So here, I guess I'd prefer we don't give rotation "momentum", you know? What about just using the angle to manipulate the images using the same method as in RotateScaleHandle?:
Leaflet.DistortableImage/src/edit/RotateScaleHandle.js
Lines 17 to 20 in 70b22a2
| angle = this.calculateAngleDelta(formerLatLng, newLatLng), | |
| scale = this._calculateScalingFactor(formerLatLng, newLatLng); | |
| if (angle !== 0) { edit._rotateBy(angle); } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jywarren agreed! I need to go back and debug since I started tracking rotation. I don't think the speed code should actually be doing anything at this point just needs to be removed. The bug I was concerned about is that as you rotate you hit a sort of "bump" and the image stops and starts rotating in the other direction.
I also implemented rotation via Hammer.js and it was functionally the seem. Going to push that here now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok Hammer.js implementation is under index.html. I'll go over these again this weekend and rebase!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also just realized the listeners.html code here wasn't up to date before - Sorry I might have lost track of my branches!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also just remembered - our _rotateBy takes in a deltaAngle that is tiny because drag is fired so often. This doesn't play nice with touch, so this is why I needed to track rotation. That part is still a little buggy too but works well enough
Fixes #165 (<=== Add issue number here)
Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!
gruntIf tests do fail, click on the red
Xto learn why by reading the logs.Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software
Thanks!
===========
pending:
use new
rotationproperty tracking (added in Class Factories #275) to debug a problem where image "stops rotating and bounces back"update PR so that it has both the implementation using
Hammer.jsand no library if can't decide which one is betterclean up code / implementation. add to my demo page so jywarren can test easily.
Need to add a
panToso that if you try to rotate an image that is off screen a little bit, the screen will move so its not anymore or an error will be thrown.===========