Skip to content

Untoggle overlays and additional base layers #772

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

Merged
merged 10 commits into from
Jan 23, 2018

Conversation

Conengmo
Copy link
Member

@Conengmo Conengmo commented Nov 22, 2017

I worked on solving issue #606 and #571. I added functionality in the LayerControl class to untoggle overlays that have hidden=True. While I was there I simplified the checks in render(). If an item is an instance of Layer it will always have the overlay and control attributes.

I added the hide parameter to some classes to test it out, but not to all children of Layer yet.

What do you guys think, is this a good way to do this?

@Conengmo Conengmo changed the title WIP Hide overlays WIP Hide overlays (issue #606) Nov 22, 2017
folium/map.py Outdated
self._parent._children.items() if isinstance(val, Layer)
and val.overlay and val.control])
self.overlays_hidden = [val.get_name() for val in
self._parent._children.values() if isinstance(val, Layer)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

E128 continuation line under-indented for visual indent

folium/map.py Outdated
and val.overlay and val.control])
self.overlays_hidden = [val.get_name() for val in
self._parent._children.values() if isinstance(val, Layer)
and val.overlay and val.control and val.hide]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

E128 continuation line under-indented for visual indent

@Conengmo Conengmo changed the title WIP Hide overlays (issue #606) WIP Hide overlays Nov 22, 2017
@ocefpaf
Copy link
Member

ocefpaf commented Nov 25, 2017

What do you guys think, is this a good way to do this?

Yep. That fits folium perfectly. Thanks!

Can you:

  • add the keyword to everything else that inherits from Layer;
  • change the nome from hide to... Maybe toggle? I accept other suggestions but hide seems like something that won't be displayed ever;
  • add an entry to the changelog/

Frank added 4 commits November 28, 2017 17:04
`show` toggles whether the layer is shown on opening. It is True by default.

Also did some small refactoring here and there to make __init__'s and docstrings more consistent.
Doesn't really make sense to inherit from TileLayer. So I made it a bit more consistent by inheriting directly from Layer.
@Conengmo
Copy link
Member Author

I added the show parameter to all classes that inherit from Layer. At least, I hope I got them all :)
show seemed a good choice, I saw it's also being used in #778.

I noticed Heatmap and HeatMapWithTime inherited from TileLayer, that didn't really make sense to me. They also didn't use anything from TileLayer. So for consistency I changed both classed to inherit from Layer.

I also did some work on the use of name, overlay and control in __init__ statements and docstrings in files I touched. It should be fairly consistent now.

I didn't test all code I touched. I did test creating some maps with GeoJson objects, with Featuregroups, MarkerClusters and HeatMaps, and everything seemed to work alright.

Let me know if there's something you want different or added.

@Conengmo Conengmo changed the title WIP Hide overlays Hide overlays Nov 28, 2017
@Conengmo
Copy link
Member Author

Conengmo commented Dec 7, 2017

I noticed that according to Leaflet, any additional base layers should not be added to the map. In Folium all base layers are added to the map. As a result the last base layer is selected in layer control, while the first base layer is displayed.

Fixing this by not adding additional base layers to the map is hard, since we then need to check this in every class that inherits from Layer. Removing the additional base layers in LayerControl also works, so I added it here.

@Conengmo Conengmo changed the title Hide overlays Untoggle overlays and additional base layers Dec 7, 2017
@ocefpaf ocefpaf merged commit 9e2ee74 into python-visualization:master Jan 23, 2018
@ocefpaf
Copy link
Member

ocefpaf commented Jan 23, 2018

Perfect! Thanks @Conengmo!!

@carlospendola
Copy link

thanks master! @Conengmo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants