-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Add Locate Control plugin #1116
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Locate Control plugin #1116
Conversation
60c0016
to
177ba5b
Compare
Hi @fullonic, thanks for your work on this! Question: the 'files changed' tab shows a lot of unrelated changes. Can you try to rebase or merge master into your branch so we can see a clean diff? |
hey @Conengmo, thanks for your feedback. I just did a rebase as you suggest. I tried to reduce the history of the branch and I end up creating a dirty commit. Now looks better. I can't understand well why isn't passing all tests in Travis, but I think is not because this PR. |
Thanks for updating the branch, looks good now. I'll have a look later this weekend. |
Thanks @Conengmo. Line 79 in 913cf29
The issue isn't on the last release but on the master branch version. |
Good find! Yes if you can open a pr for that, great! |
Hi @fullonic, I reviewed your PR yesterday and overal it looks fine. I left mainly comments about style, to get it consistent with the rest of the code base. Two further questions: Can you update your branch from master? We got a fix in it that makes the tests work on Travis again. Could you add a cell to the plugins example notebook? That notebook is a gallery of all the plugins folium has, so would be nice to show this one as well! A very simple example is enough, with one or two lines to explain what it does. |
8eaea53
to
84f73e6
Compare
Hey @Conengmo, I just added right now an example to the notebook. Let me know if it's okay or should I add more information. Thank's for your help. |
@fullonic looks good! There is a tiny typo in the notebook though: 'displyed'. Question: why or how does this plugin interfere with |
@Conengmo, the typo is corrected. Thanks I have no idea why that happen. Even if I change the position of the locate button to "bottomright", for instance, it simple doesn't shows up. Only when changing the order... I didn't dig to much into the conflict because it was easy to "solve". |
Thanks @fullonic! It's a nice plugin and the code is nicely done. If somebody wants to figure out why it conflicts with Draw that's welcome, but for now your warning is good enough. |
Thanks @Conengmo for your support and help. Was nice to make a contribution to folium! |
Plugin based on Leaflet Locate Control
Related with issue #1098