Skip to content
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

Ensure toolbar buttons toggle widgets, and turn off when widgets are closed. #1763

Merged
merged 10 commits into from
Oct 7, 2023
22 changes: 17 additions & 5 deletions geemap/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -765,25 +765,37 @@ def add_layer(

addLayer = add_layer

def _open_help_page(self, host_map: MapInterface, selected: bool) -> None:
def _open_help_page(
self, host_map: MapInterface, selected: bool, item: toolbar.Toolbar.Item
) -> None:
del host_map # Unused.
del item # Unused.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: del host_map, item # Unused.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

if selected:
common.open_url("https://geemap.org")

def _toolbar_main_tools(self) -> List[toolbar.Toolbar.Item]:
@toolbar._cleanup_toolbar_item
def inspector_tool_callback(map, _, item):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: Could you add type annotations here + call del on the unused properties?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

map.add("inspector")
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if add should return the ipywidget widget that was just added... It would make it quite a bit more useful. We'd be changing the public API, but I'm okay with that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think this would be a great addition to the API, but I'm less inclined to make the change in this PR.

return map._inspector

@toolbar._cleanup_toolbar_item
def basemap_tool_callback(map, _, item):
map.add("basemap_selector")
return map._basemap_selector

return [
toolbar.Toolbar.Item(
icon="map",
tooltip="Basemap selector",
callback=lambda m, selected: m.add("basemap_selector")
if selected
else None,
callback=basemap_tool_callback,
reset=False,
),
toolbar.Toolbar.Item(
icon="info",
tooltip="Inspector",
callback=lambda m, selected: m.add("inspector") if selected else None,
callback=inspector_tool_callback,
reset=False
Copy link
Collaborator

Choose a reason for hiding this comment

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

Minor nit: reset=False,. The only reason I mention this is because the automatic formatter will add the comma next time one of us edits this file :)

),
toolbar.Toolbar.Item(
icon="question", tooltip="Get help", callback=self._open_help_page
Expand Down
21 changes: 15 additions & 6 deletions geemap/map_widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,14 @@ def __init__(
children=[self.toolbar_header, self.inspector_checks, self.tree_output]
)

def cleanup(self):
"""Removes the widget from the map and performs cleanup."""
if self._host_map:
self._host_map.default_style = {"cursor": "default"}
self._host_map.on_interaction(self._on_map_interaction, remove=True)
if self.on_close is not None:
self.on_close()

def _create_checkbox(self, title, checked):
layout = ipywidgets.Layout(width="auto", padding="0px 6px 0px 0px")
return ipywidgets.Checkbox(
Expand Down Expand Up @@ -578,11 +586,7 @@ def _on_toolbar_btn_click(self, change):

def _on_close_btn_click(self, change):
if change["new"]:
if self._host_map:
self._host_map.default_style = {"cursor": "default"}
self._host_map.on_interaction(self._on_map_interaction, remove=True)
if self.on_close is not None:
self.on_close()
self.cleanup()

def _get_visible_map_layers(self):
layers = {}
Expand Down Expand Up @@ -896,9 +900,14 @@ def _on_dropdown_click(self, change):
if self.on_basemap_changed and change["new"]:
self.on_basemap_changed(self._dropdown.value)

def _on_close_click(self, _):
def cleanup(self):
if self.on_close:
self.on_close()
if hasattr(self, 'toggle_off'):
self.toggle_off()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Calling toggle_off seems like it would be error-prone (e.g. why doesn't inspector have it?). Is there a way we could do it automatically?


def _on_close_click(self, _):
self.cleanup()


@Theme.apply
Expand Down
Loading