Various possible features, to-dos, and bugfixes to be broken out

[jywarren]

...into separate issues

(copied in from README)

To do:

• [x] shift-drag (scale with no rotate) doesnt work if you shift first, only if you drag first
• [ ] default to order by size -- maybe need a custom $L.customOrdering boolean?
• plenty of other issues

Lower priority:

• [x] decide if we need to keep updating _bounds - yes

• create img.toGeoJSON() so we can send a concise description of an image to MapKnitter, plus properties:
  • locked
  • layer order
  • last touched/edited?
  • "{"type":"Feature","properties":{},"geometry":{"type":"Polygon","coordinates":[[[-0.08,51.509],[-0.06,51.503],[-0.047,51.51],[-0.08,51.509]]]}}"
• add onLock, onUnlock, onDistortEnd - and consider plumbing events properly
• add image ordering -- bringToFront() should be temporary only; we need img.raise() or img.lower() or img.raiseToTop() etc... also img.order() for current position in order
• add an img.revert() which reverts it to orig dimensions and rotation
• implement tab to select next image; $L.selectedIndex?

=================

Leftovers, persnickity stuff:

• plumb or remove debug system
• make shift-drag drag the nearest marker, not the image?

• [x] scale is not true scaling -- it moves points equally from the "center" which causes distortion when scaling down a lot

[rexagod]

@sashadev-sky Can you please glance over the issues above and let me know if you've started/finished up on anyone of these in any of your PRs? That'll be really helpful to know. I can then start working with the rest of these.

Some of these I think are done already:

• add image ordering -- bringToFront() should be temporary only; we need img.raise() or img.lower() or img.raiseToTop() etc... also img.order() for current position in order
• create img.toGeoJSON() so we can send a concise description of an image to MapKnitter, plus properties:
• Any other issue that comes to mind?

[jywarren]

Hi! I think finishing up submenus will be really useful as it'll allow us to better organize and consolidate current features, and make more room for more toolbar menu items! Thanks!

On Sun, Jul 7, 2019 at 9:58 AM Pranshu Srivastava [email protected] wrote:

@sashadev-sky https://github.com/sashadev-sky Can you please glance over the issues above and let me know if you've started/finished up on anyone of these in any of your PRs? That'll be really helpful to know.

Some of these I think are done with:

• add image ordering -- bringToFront() should be temporary only; we need img.raise() or img.lower() or img.raiseToTop() etc... also img.order() for current position in order
• create img.toGeoJSON() so we can send a concise description of an image to MapKnitter, plus properties:
• Any other issue that comes to mind?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/publiclab/Leaflet.DistortableImage/issues/87?email_source=notifications&email_token=AAAF6JZINWFI2J5ENL644VDP6HY6JA5CNFSM4ESFOHLKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZLMA5A#issuecomment-509001844, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAF6J3FO2553RMGU7F5MYLP6HY6JANCNFSM4ESFOHLA .

[sashadev-sky]

@rexagod Looking them over! @jywarren would appreciate your input on these points:

• [ ] shift-drag (scale with no rotate) doesnt work if you shift first, only if you drag first

  • shift-drag is BoxSelector! BoxSelector doesn't display this problem. Why did we want to have this for scale? Pretty sure this is old?

• [ ] decide if we need to keep updating _bounds

  • also old because we use corners instead of _bounds? But I think starting to track _bounds is something we want to start doing because a lot of Leaflet functions work on _bounds and it'll be easier to debug some current issues we're having. Perhaps open an issue to create a utility function that converts corners to a bounds and vice versa?

• [ ] add onLock, onUnlock, onDistortEnd - and consider plumbing events properly

  • in discussion here #328

• [ ] add an img.revert() which reverts it to orig dimensions and rotation

  • this is exactly what _restore does. but it would be called img.editing._restore. Do you want it exposed as public API (restore), testing, etc.? I can open an issue to do that! I can also rename it "revert"! That actually makes a bit more sense than "restore".

• [ ] make shift-drag drag the nearest marker, not the image?

  • same feedback as the first point in this list

• [ ] add image ordering -- bringToFront() should be temporary only; we need img.raise() or img.lower() or img.raiseToTop() etc... also img.order() for current position in order

  • we are currently still using bringToFront()? Did you address this in one of your open PRs?

• [ ] create img.toGeoJSON() so we can send a concise description of an image to MapKnitter, plus properties:

  • not sure about this one but I think this is veery different from what we achieved in generateExportJson.

[jywarren]

shift-drag (scale with no rotate) doesnt work if you shift first, only if you drag first

I think this is OK. This comes up sometimes when people want to ONLY scale something, but preserve its rotation. we should be able to detect clicking on a handle and not BoxSelect if so, and get this working in either order, but not a priority!

_bounds -- yeah, agree with a lot of Leaflet functions work on _bounds -- for sure!

+1 revert as the new restore!

toGeoJson is not that important either. In theory we might've chosen GeoJSON as a standard that we could build generateExportJSON around, but we didn't, and it's not a big deal. Maybe as part of a future breaking change? someday? 😄

[jywarren]

I think highest priorites now are:

• add spinner to multi-exporter
• submenus

Then:

• finish gesture rotation #253
• https://github.com/publiclab/Leaflet.DistortableImage/issues/306
• anything labeled high priority?

[sashadev-sky]

@jywarren I don't remember shift + drag for scaling ever being a feature! I could add this, but it doesn't seem like a convenient way to scale from the keyboard, having to target the handle exactly.

Do you think maybe we should see through #165 (rotate gesture support) first and then decide what makes sense after? I am not sure if I will end up adding scaling to it (hopefully will) and not sure if it will just be touch or also mousepad (have to double check, so far I think it is leaning towards touch only).

[sashadev-sky]

@jywarren geoJSON may help solve some bugs. I am slowly trying to root out the primary library bugs right now but I think they will involve some big changes - from switching to webGL or Canvas, SVG, geoJSON, or using only transformation matrices. Still a lot to think about there!

gesture rotation was working okay last time I worked on it. Still needs cleaning up and have to verify the implementation details. But it cannot use css transformations, and using _rotateBy and _scaleBy is buggy (probably even more buggy because touch gestures are less precise). So this is why i'm trying to work out all the kinks before. I will make sure, though, to fix it up and prepare for merge soon whether or not I resolve the issues.

[jywarren]

Sure, the shift/drag to scale only was just turning off the rotation line in the rotate/scale, if it detects the shift key. It's not super important.

Regarding gesture rotation, sounds great, and i think the key here is that we're looking to plug it into the same kind of "move the corner points" approach to rotation and scale as the rotate/scale tool. That is, we should a) establish the angle and distance of the gesture from the center point, and b) apply a rotate/scale to all four corner points. That way it'll share much of the logic with the already-existing system. Looking forward to it!

On Wed, Jul 10, 2019 at 2:34 PM Sasha Boginsky [email protected] wrote:

@jywarren https://github.com/jywarren geoJSON may help solve some bugs. I am slowly trying to root out the primary library bugs right now but I think they will involve some big changes - from switching to webGL or Canvas, SVG, geoJSON, or using only transformation matrices. Still a lot to think about there!

gesture rotation was working okay last time I worked on it. Still needs cleaning up and have to verify the implementation details. But it cannot use css transformations, and using _rotateBy and _scaleBy is buggy (probably even more buggy because touch gestures are less precise). So this is why i'm trying to work out all the kinks before. I will make sure, though, to fix it up and prepare for merge soon whether or not I resolve the issues.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/publiclab/Leaflet.DistortableImage/issues/87?email_source=notifications&email_token=AAAF6JYPQCVBUQ2UIB2SASLP6YTPPA5CNFSM4ESFOHLKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZULFCI#issuecomment-510177929, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAF6J2LUTGULKDHJAFX5CTP6YTPPANCNFSM4ESFOHLA .

[sashadev-sky]

@jywarren I just got the initial placement and animateZoom of the images to work with transform matrices in #339 so v happy about that! Hoping I can get rotation and scale to work with it, and then the distortion will be gone 🎉

[7malikk]

@jywarren Hello there, I'm an outreachy applicant, I'm interested in contributing to the Mapknitter project. I noticed some tasks in this issue have not been checked off. Can I convert them to FTOs or work on them myself?

[7malikk]

@TildaDares Hello again, I noticed some tasks in this issue have not been checked off. Can I convert them to FTOs or work on them myself?