stefanocudini / leaflet-panel-layers

Leaflet Control Layers extended with support groups and icons
https://opengeo.tech/maps/leaflet-panel-layers/
MIT License
286 stars 92 forks source link

Leaflet 1.0 compatibility #20

Closed vfp1 closed 7 years ago

vfp1 commented 8 years ago

Hi, thanks for this awesome plugin!

I am trying to recreate it on my own scripts and it seems that some of your paths on the example folders are missing or directing to the wrong directory. For instang in group-layers.html there is a link referring to:

<link rel="stylesheet" href="/maps/leaflet/dist/leaflet.css" />

which seems that "maps" folder is not on the master folder. Perhaps just the leaflet.css and leaflet.js from the last versions of Leaflet?

Best,

imaginalis commented 8 years ago

Perhaps just the leaflet.css and leaflet.js from the last versions of Leaflet?

Yes, I thought so but when I replaced it by the latest version of Leaflet I've got this: screenshot There's nothing wrong on the browser console.

I used Leaflet 1.0.1 but author on github page of the plugin wrote:

Tested in Leaflet 0.7.3

So all in all - it looks like it's not compatible with the latest Leaflet.

nungyao commented 7 years ago

Same problem on Leaflet 1.0.2. Can someone fix it? Thanks!

c0x6a commented 7 years ago

I've got the same problem here, using Leaflet 1.0.2 leaflet-panel-layers seems to fail loading

skipper71 commented 7 years ago

Hello Stefano, I'm quite new with leaflet, and I found your library very useful for my scope, which works great wit Leaflet up to 0.7.7 but no way in 1.0.3, the official release by now.

Prior to go on with the development, I need to know, if you are planning some fix for the above issue, or meybe you already have a solution for that, or it will remain like this.

Thanks in advance for your kind answer. Miguel, Genoa (IT)

stefanocudini commented 7 years ago

hi @skipper71 I have already identified the cause of the problem is in this difference type of property to storing the list of layers defined for L.Control.Layers of Leaflet

in version 1.0.x: https://github.com/Leaflet/Leaflet/blob/master/src/control/Control.Layers.js#L83 is an Array []

in version 0.7.x: https://github.com/Leaflet/Leaflet/blob/0.7/src/control/Control.Layers.js#L15 is an Object {}

I'm thinking about how to solve easily and maintain compatibility, this plugin now use an Array(0.7.x)

tomchadwin commented 7 years ago

@stefanocudini Did you make any progress with this? I'd like to try to help, as I could use this plugin, but need Leaflet1 compatibility.

tomchadwin commented 7 years ago

Does this approach help:

https://github.com/ismyrnow/leaflet-groupedlayercontrol/commit/5c3db55507fac4fe499969d410813512e9ebce81#diff-7a4578c3ea4c0f421057ead703fa0e5a

strepto42 commented 7 years ago

I'd love to use your awesome plugin too but have the same problem, and I'm stuck on leaflet 1+ due to other plugins.

strepto42 commented 7 years ago

FYI I have forked this and added compatibility to 1.0.0+ - bit busy to properly test (I only want certain functionality) but it seems to work ok for the most part.

@tomchadwin thanks for putting me on the right track. @stefanocudini thank you for a great module. You're welcome to pull in my work (and test it properly!) whenever.

tomchadwin commented 7 years ago

@strepto42 Nice! I'll try to test it when I can and feed back.

stefanocudini commented 7 years ago

hi @strepto42 ! great new! I'll try to merge your work tomorrow

strepto42 commented 7 years ago

Awesome! Apologies if it's dodgy in any way, it was pretty rushed and javascript isn't my favourite ;)

tomchadwin commented 7 years ago

EDIT: ignore - user error.

I've tested, and it works for simple, root level layers, but I couldn't get it to work for groups. I get an empty, elongated panel (blue highlight is just because of hovering over it with the element inspector in Chrome dev tools):

2017-06-12 10_29_44-index html

No errors, though, as you can see.

tomchadwin commented 7 years ago

Ack! User error - looks like it works perfectly. Thank you so much!

strepto42 commented 7 years ago

Lol oh dang, I was going to say, at least it looks pretty!

I'm slightly amazed it worked properly but most of the examples seemed ok when I tried them locally. My own use case (grouped collapsible layers that play nicely with huge numbers of clustered markers) works nicely.

Thanks guys.

tomchadwin commented 7 years ago

@strepto42 You might want to do a little tidying before you make a pull request:

https://github.com/stefanocudini/leaflet-panel-layers/compare/master...strepto42:master

If you can limit it only to the changes required to make this work (source, docs, examples, not compiled dist code or any local changes you had to tweak for your environment), the Stefano will be able to merge. It's a bit harder with all the extra things (like that changed path in all the demos).

strepto42 commented 7 years ago

Yeah I hear you. Just been to busy to make a proper PR but if Stefano would like me to I'll organise it in the next couple of days. Basically it's just the one file from memory.

blankoworld commented 7 years ago

We rely on you ^_^

I read @strepto42 branch. Use files and replay changes to generate a diff: https://github.com/stefanocudini/leaflet-panel-layers/compare/master...blankoworld:master

Hope it will help to integrate these changes more quickly.

AndreaAstronaut commented 7 years ago

This plugin is exactly what I need! :) Thanks so much for developing it @stefanocudini Any luck with the merge?

strepto42 commented 7 years ago

PR created, sorry it took so long.

stefanocudini commented 7 years ago

@strepto42 thanks for your contribute!

@blankoworld @cjdp25 @skipper71 the plugin is updated and publish on NPM https://www.npmjs.com/package/leaflet-panel-layers version 1.1.0 for supporting Leaflet 1.1.0

@tomchadwin If you need more support to include the plugin in qgis2web I am here or via email!

thanks to all! I apologize for the delay, but at this moment I'm workless and have to deal with more important things in my life :(