Remove popup_on #133
Remove popup_on #133
Conversation
There was a problem hiding this comment.
@themiurgo Quick questions before we merge this. Why did you choose an empty string instead of the 'var no_pop = null;'? Will the marker be clickable with the empty string? If so, that might be misleading.
There was a problem hiding this comment.
In fact if there is no popup text, I do not set a javascript variable at all in the HTML template. The marker will not be clickable so no variable is set.
There was a problem hiding this comment.
In fact if there is no popup text, I do not set a javascript variable at all in the HTML template.
In a local test the "click" hand appears when hovering a mark making the user think that he can click and see something. I guess we do need to set 'var no_pop = null;'.
There was a problem hiding this comment.
In fact, in my tests, that behaviour happens anyways, even if no_popup is set. Try the same on the master branch and you'll see the hand popping up anyways, even if popup_on is set to False.
There was a problem hiding this comment.
Ah, so setting it to null does not change that. Let me take a closer look at this later (I am still travelling...)
If that is what leaflet does then lets merge as is.
There was a problem hiding this comment.
Sure. Here's an example what comes out from folium (master branch) with popup_on=False on a marker:
https://gist.github.com/themiurgo/170dbb742897f9b86da6#file-no_popup-html
There was a problem hiding this comment.
See also here:
http://leafletjs.com/examples/quick-start.html
Third map, there are some elements without popup content. Hand still shows. 😄
There was a problem hiding this comment.
OK, then. It is a Leaflet issue! Lets merge...
|
Thanks @themiurgo! The PR looks good to merge. I just want to test it locally and once I get back in the office I will merge it. |
|
Great, let me know if any issues pop out! 😄 |
One did not 😛 |
2 participants
Solves #131