Allow featureGroup initialization with no overlay corners

[sashadev-sky]

A step to fix publiclab/mapknitter#919 (<=== Add issue number here)

• DistortableCollection.Edit #335

Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!

• PR is descriptively titled 📑 and links the original issue above 🔗
• tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with grunt
• code is in uniquely-named feature branch and has no merge conflicts 📁
• screenshots/GIFs are attached 📎 in case of UI updates
• @mention the original creator of the issue in a comment below for help or for a review

We're happy to help you get this ready -- don't be afraid to ask for help, and don't be discouraged if your tests fail at first!

If tests do fail, click on the red X to 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!

[sashadev-sky]

@rexagod @jywarren this PR is very small in code changes but makes a bigger difference to how the library can be used.

corners has never been a required option for L.distortableImageOverlay, but they were for DistortableCollection because I added a call to img.editing.enable from there. I moved this call inside the overlay class instead, since we already have an event listener there for image load.

I updated the README pretty thoroughly to basically make the point that for single image interface you don't really need them but for multi you want to include them or the images will render on top of eachother. The index.html and select.html demos now reflect this too!

I think it actually makes the plugin a lot easier to use, because people can add images to the map quickly, and use our API to retrieve the corners for a position they like and then add in the corners option. Otherwise they have to calculate it themselves and risk distorting their image.

[sashadev-sky]

in MK, hopefully this will allow us to stop adding images to the map instead of the feature group.

It also calls img.editing.enable by default once they're added to the map, so I bumped to 0.8.0. but this is already what we were doing for the feature group anyway.

[sashadev-sky]

ok I just found that @rexagod uses custom logic for img.editing.enable #312 and I have a feeling others might want to as well, so I am going to create an enabled option that can be set to false so that image editing isn't automatically enabled on load

[jywarren]

OK, interesting, so will this then need some more work before reviewing it? Thanks!

…

On Wed, Aug 14, 2019 at 12:57 AM Sasha Boginsky ***@***.***> wrote: ok I just found that @rexagod <https://github.com/rexagod> uses custom logic for img.editing.enable #312 <#312> and I have a feeling others might want to as well, so I am going to create an enabled option that can be set to false so that image editing isn't automatically enabled on load — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#381?email_source=notifications&email_token=AAAF6J5KFWHY6A4XEDRCEG3QEOGDPA5CNFSM4ILP7MKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4HWBZQ#issuecomment-521101542>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAF6J7J53DS6Y2EAJLD2K3QEOGDPANCNFSM4ILP7MKA> .

[jywarren]

Ah, does this fix publiclab/mapknitter#919 or what else would be required? I'm very eager to get that resolved, so any tips appreciated here! Thank you!

[sashadev-sky]

@jywarren Might fix it or might just be one step in fixing it. It's basically ready for review, all I am doing is adding an enabled option which will run based on a conditional. I will finish this ASAP. The main part to really review is to just read through the README.

[jywarren]

Ok, looks good to me! I rushed in a fix for the legacy exporter just now to address the urgent issue and I hope that'll fix it. But still eager to see this merged!

…

On Wed, Aug 14, 2019, 6:37 PM Sasha Boginsky ***@***.***> wrote: @jywarren <https://github.com/jywarren> Might fix it or might just be one step in fixing it. It's basically ready for review, all I am doing is adding an enabled option which will run based on a conditional. I will finish this ASAP. The main part to really review is to just read through the README. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#381?email_source=notifications&email_token=AAAF6J6LBVKTQDGAQ625RQLQESCKBA5CNFSM4ILP7MKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4KKEDQ#issuecomment-521445902>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAF6J3GS6IIHEU5DBBUBO3QESCKBANCNFSM4ILP7MKA> .

[jywarren]

As soon as it's complete, let's bump version in Mapknitter to get this pulled in.

…

On Wed, Aug 14, 2019, 7:06 PM Jeffrey Warren ***@***.***> wrote: Ok, looks good to me! I rushed in a fix for the legacy exporter just now to address the urgent issue and I hope that'll fix it. But still eager to see this merged! On Wed, Aug 14, 2019, 6:37 PM Sasha Boginsky ***@***.***> wrote: > @jywarren <https://github.com/jywarren> Might fix it or might just be > one step in fixing it. It's basically ready for review, all I am doing is > adding an enabled option which will run based on a conditional. I will > finish this ASAP. The main part to really review is to just read through > the README. > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#381?email_source=notifications&email_token=AAAF6J6LBVKTQDGAQ625RQLQESCKBA5CNFSM4ILP7MKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4KKEDQ#issuecomment-521445902>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/AAAF6J3GS6IIHEU5DBBUBO3QESCKBANCNFSM4ILP7MKA> > . >

[sashadev-sky]

I had to do a lot of restructuring and make a #335 for this option to work properly with both groups. almost done tho!

[sashadev-sky]

@jywarren ok this turned into a complicated one but it is done now. @rexagod @ViditChitkara I think enable, disable, editable API could come up for you guys so check it out! Let me know your thoughts.

Easiest probably to just look at README and try the code out if u'd like

[ViditChitkara]

That's awesome!! Will check this out.
Thanks

[jywarren]

Just a couple tiny queries about the docs. This is an incredible PR! The README is so great. cc @IshaGupta18 @rexagod just generally as it's a great PR, and also cc @ananyaarun @sagarpreet-chadha on the UI testing in here, it could be useful?

[jywarren]

And shall we bump to 0.7.1 with this? Any breaking changes? I think not...

[sashadev-sky]

And shall we bump to 0.7.1 with this? Any breaking changes? I think not...

I made it 0.8.0 because a) seemed like a big API update and I wanted people to notice the update corners stuff. b) Could be breaking for any implementations that make the image editing interface turn on by some custom logic rather than load. But doesn't matter anyway and the odds are slim.

Updated to 0.7.1