Leaflet / Leaflet.toolbar

Flexible, extensible toolbars for Leaflet maps.
MIT License
197 stars 70 forks source link

Leaflet 1.0.0-rc1 compatible release #41

Closed yohanboniface closed 7 years ago

yohanboniface commented 8 years ago

Leaflet 1.0.0-rc1 is out since quite a while, I think it would be great to have an npm release with the compliant version.

prophe05 commented 7 years ago

+1

jywarren commented 7 years ago

+1 - do we know what's going wrong? I see a "deprecated" error over here in Leaflet v1.2.x:

Class.js:121 Deprecated include of L.Mixin.Events: this property will be removed in future releases, please inherit from L.Evented instead. Error
    at checkDeprecatedMixinEvents (https://publiclab.github.io/Leaflet.DistortableImage/node_modules/leaflet/dist/leaflet-src.js:401:47)
    at Function.Class.extend (https://publiclab.github.io/Leaflet.DistortableImage/node_modules/leaflet/dist/leaflet-src.js:329:3)
    at https://publiclab.github.io/Leaflet.DistortableImage/node_modules/leaflet-toolbar/dist/leaflet.toolbar.js:1:49
    at https://publiclab.github.io/Leaflet.DistortableImage/node_modules/leaflet-toolbar/dist/leaflet.toolbar.js:1:4695
checkDeprecatedMixinEvents @ Class.js:121
Class.extend @ Class.js:51
(anonymous) @ leaflet.toolbar.js:1
(anonymous) @ leaflet.toolbar.js:1

But not sure that's the issue. This is at:

https://publiclab.github.io/Leaflet.DistortableImage/examples/

jywarren commented 7 years ago

That's with leaflet-toolbar 0.1.0 -- i'll update to 0.1.2 (latest at present) and check again.

jywarren commented 7 years ago

npm is hanging -- i'll return to this later but happy to collab with folks on this.

jywarren commented 7 years ago

confirmed same w 1.2.

justinmanley commented 7 years ago

Not sure what the problem is, but there were some pretty drastic API changes between Leaflet 0.7 and 1.0. It's been a while since I've worked on this - I'll dive in and take a look this weekend.

On Thu, Aug 24, 2017, 10:16 AM Jeffrey Warren notifications@github.com wrote:

confirmed same w 1.2.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Leaflet/Leaflet.toolbar/issues/41#issuecomment-324699507, or mute the thread https://github.com/notifications/unsubscribe-auth/ABl1h0jUBiyLYXMNzJUpm3rNApT1cT5Mks5sba_-gaJpZM4I5pwh .

jywarren commented 7 years ago

Awesome, thanks, Justin! Good to hear from you! 🎈

Here are the changes we made to Leaflet.DistortableImage from v0.7 to v1.x in case they're helpful:

https://github.com/publiclab/Leaflet.DistortableImage/compare/8c3c40e2e3c9cd590b90cd1ce6065d02b37b11e8...master

rakeshtinku commented 7 years ago

@justinmanley i would like to help you with this. let me know how i could help you in making this toolbar compatable with 1.x.

justinmanley commented 7 years ago

@rakeshtinku great! thank you for volunteering! I'm really excited that you're interested in helping out.

I had two deadlines come up, so I won't have a chance to really dive into the code until 9/12.

In the meantime, a good place for you to start is to install the examples in this repo and catalog in detail the issues that arise when you try to go through the examples with the leaflet version set to 1.x.

You should be able to get the examples working with Leaflet 0.7 by following the instructions I give here: https://github.com/Leaflet/Leaflet.toolbar/issues/8.

I've also created a bug to track another similar issue that has plagued this library since its initial release: https://github.com/Leaflet/Leaflet.toolbar/issues/47. Let me know if fixing this would be helpful to you.

Thanks again for offering to help out!

justinmanley commented 7 years ago

Fixed with https://github.com/justinmanley/leaflet-draw-toolbar.

jywarren commented 7 years ago

Thanks - and it looks like based on these lines, the API is the same, or very very close: https://github.com/justinmanley/leaflet-draw-toolbar/blob/master/examples/popup.html#L69-L71

jywarren commented 7 years ago

Does leaflet-draw-toolbar replace Leaflet.Toolbar and deprecate this repository? Thank you!

justinmanley commented 7 years ago

Nope! leaflet-draw-toolbar augments Leaflet.Toolbar. Previously, you needed to patch https://github.com/Leaflet/Leaflet.draw/pull/354 in order to use Leaflet.draw with Leaflet.toolbar. Now, Leaflet.toolbar + Leaflet.draw should work together out-of-the-box if you use leaflet-draw-toolbar.

Note that Leaflet.toolbar is independent of Leaflet.draw (no dependencies on Leaflet.draw). Before, I tried to solve the problem of Leaflet.draw compatibility with inheritance (Leaflet/Leaflet.draw#354); leaflet-draw-toolbar attempts to solve the same problem using composition (creating a bunch of ToolbarActions which wrap the handlers in Leaflet.draw).

jywarren commented 7 years ago

Ah, i see -- you've also made a new release to Leaflet.Toolbar itself. Awesome, so as I understand it, we don't have to redirect any dependencies, we just have to update our downstream package.json to point at Leaflet.Toolbar ~v0.3.0.

Thanks! I hadn't noticed the new releases and they're super exciting.