(edited) Publish changes compatible with downstream React wrapper

[ChrisLowe-Takor]

Hi,

Terrific mapping library guys. I was wondering if the maintainers and contributors would mind if I publish a react wrapper for this. Here is the repo: https://github.com/ChrisLowe-Takor/react-leaflet-distortable-imageoverlay

It's not strictly a wrapper as I had to chop and change quiet a bit to get all the functionality online (had some major problems with the Toolbar and Translating for one thing). It uses a combination of my own implementations, Leaflet.Path.Transform and modified copy of the Leaflet.DistortableImage distribution.

Would appreciate your permission and will be happy to chip in with some contributions from time to time.

[jywarren]

Hi, Chris - wow, very exciting! 🎉

Of course go ahead -- I'm interested though in if you think any of your changes would be usefully submitted upstream to merge back into this library. Where is the modified copy you're working from -- is it this one?

https://github.com/ChrisLowe-Takor/react-leaflet-distortable-imageoverlay/blob/master/src/lib/leaflet-distortableimage.js

Is that the compiled version from this repo's dist/_____ folder? I'd love to see a diff against the version you started with... were the changes breaking any original functionality?

Thanks for getting in touch!

[ChrisLowe-Takor]

Great! Thanks a lot. I'll publish it sometime this week and close the issue.

Is that the compiled version from this repo's dist/_____ folder? I'd love to see a diff against the version you started with... were the changes breaking any original functionality?

Yep thats the one. The diff won't be very exciting I'm afraid. I had to chop out the toolbar to get it compiling so it's step backwards for the upstream. Not sure why but I just couldn't get it to play nicely with react.

One difference that you may or may not find useful is I split the rotate and scale into seperate actions. It fit my use case a little better than doing them together and it accidentally fixed that distortion problem noted in #85.

[jywarren]

Hi, Chris - I'd like to consider integrating your downstream changes with an options flag so that we retain the same upstream code, and things like the distortion bug you were really helpful on are easier to resolve across both uses of the codebase. Do you think the no-toolbar variant would be possible to set via an options flag? We could even create a grunt task to compile a non-toolbar version alongside the original in the /dist/ code. Just looking for ways to organize things so that we can help one another out on debugging and such moving forward!

The split of rotate and scale, maybe we could actually offer all three and have the separation settable via an options param. Cool!

[jywarren]

(renamed accordingly, didn't seem worth opening a new issue)

[ChrisLowe-Takor]

Using grunt to conditionally compile is a terrific idea. I wasn't looking forward to to manually updating my copy and it'll mean any new features I had in mind can be made here instead of downstream.

[jywarren]

Awesome. Do you want to try opening a PR with your version of the dist/____ file so we can see the diffs, then copy them in appropriately to the src/ directory? Thanks Chris!

[jywarren]

I'm happy to pitch in here too, thanks!

[jywarren]

I'd like to take the next step here, so let's try to identify the sections of code that were changed, more discretely:

1. the Toolbar was removed on these lines
2. rotation/scale were split into separate actions (Also see this line

Here is the full diff; it's not too complex: https://gist.github.com/jywarren/59f0cb0baec03bee99540ac2af2ecf70/revisions?diff=split#diff-d592453497ece2dffbf1a19cb06db126

We can actually preserve "scale + rotate" as well as separate "scale" and "rotate" tools, so both are usable in different UIs.

Suppressing the toolbar

We can, separately, disable the whole var EditOverlayAction = LeafletToolbar.ToolbarAction.extend({ section based on a constructor parameter like createToolbar, either within the toolbar source file here:

Leaflet.DistortableImage/src/edit/DistortableImage.Edit.js

Line 243 in dc9e64c

this.toolbar = new L.DistortableImage.EditToolbar(raised_point).addTo(map, overlay);

Here's an example of using a constructor parameter:

Leaflet.DistortableImage/src/edit/DistortableImage.Edit.js

Line 21 in dc9e64c

this._mode = this._overlay.options.mode || 'distort';

And/or on this line where we instantiate the toolbar where we can put a conditional:

Leaflet.DistortableImage/src/edit/DistortableImage.Edit.js

Line 243 in dc9e64c

this.toolbar = new L.DistortableImage.EditToolbar(raised_point).addTo(map, overlay);

Then @ChrisLowe-Takor will be able to simply run require('leaflet-distortable-image') to include the latest version... how does that sound?

[jywarren]

Moved the second part to a stand-alone issue in #111 !

Next we can break out the "create new separate scale/rotate tools" as well.

[jywarren]

Hi, I also documented how we can do the rotate/scale work, which looks a little more involve but not too bad: #112

[jywarren]

We're halfway there - @rexagod just made it possible to suppress the toolbar in #111; discussion at #132. Next #112 should make separate scale and rotate tools possible.

We're also working on a more abstract means of attaching new UIs - #140 -- thanks all!

[jywarren]

Hi @ChrisLowe-Takor ! Thanks to fantastic contributions by several new contributors and especially @rexagod, v0.3.0 now allows separate Rotate and Scale tools as well as the ability to turn off the default popup toolbar. If you'd like to link your library back up by including it in your package.json instead of copying in the code, i think you'll be able to connect back up to our latest version and get the benefits of the ongoing bugfixes as well as new features like the client-side full resolution download (#100)! (https://publiclab.org/notes/warren/02-15-2019/leaflet-distortableimage-full-resolution-download)

I've opened a PR which I believe will start in that direction, although some integration code may be needed to set the defaults etc:

ChrisLowe-Takor/react-leaflet-distortable-imageoverlay#4