search

mstahv / g-leaflet

Simple javascript overlay based GWT API for Leaflet
Other
14 stars 25 forks source link

Fix Popup.addCloseListener #20

Closed lightoze closed 9 years ago

lightoze commented 9 years ago

There's no close event in Leaflet, but there is popupclose. Also there's a problem that popupclose is not fired against standalone popup layer, for which I've proposed a fix in https://github.com/Leaflet/Leaflet/pull/3812 and manually merged it into g-leaflet for now.

As a bonus, #19 is fixed here because I'm lazy to create separate branch

mstahv commented 9 years ago

As Popup is now a real layer in Leaflet 1.0, maybe the right fix would be to remove the close listener totally and just use added/remove events and naming also in the Java API? Don't remember right away if it is there in GWT API, but should definitely be added.

mstahv commented 9 years ago

BTW. slf4j-simple dependency removal is just fine in this pull request as well.

lightoze commented 9 years ago

I've updated the patch. This remove event is fired so indirectly from Leaflet Map that I did not see it while reading its source :)

mstahv commented 9 years ago

Yep, I noticed that, but meant that maybe Popup should now extend org.peimari.gleaflet.client.Layer and that class should have addRemoveListener/addAddedListener ?

lightoze commented 9 years ago

Well, I'd like to change all v-leaflet layer connectors so that they do not re-create widgets on every state change. That would quite probably require changes in g-leaflet as well, so I'd better to change Popup along that later work.

mstahv commented 9 years ago

Fair enough, I'll pull this in. I can look at the refactoring as well once I find a moment to work with leaflet integrations.

mstahv commented 9 years ago

Just let me know if you need release for your project, otherwise I'll wait for some other fixes as well.