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

Leaflet control #1902

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 75 additions & 1 deletion folium/features.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,18 @@
import json
import operator
import warnings
from typing import Any, Callable, Dict, Iterable, List, Optional, Sequence, Tuple, Union
from typing import (
Any,
Callable,
Dict,
Iterable,
List,
Literal,
Optional,
Sequence,
Tuple,
Union,
)

import numpy as np
import requests
Expand All @@ -20,6 +31,7 @@
from folium.folium import Map
from folium.map import FeatureGroup, Icon, Layer, Marker, Popup, Tooltip
from folium.utilities import (
JsCode,
TypeJsonValue,
TypeLine,
TypePathOptions,
Expand Down Expand Up @@ -1973,3 +1985,65 @@ def __init__(
out.setdefault(cm(color), []).append([[lat1, lng1], [lat2, lng2]])
for key, val in out.items():
self.add_child(PolyLine(val, color=key, weight=weight, opacity=opacity))


class Control(MacroElement):
Copy link
Member

Choose a reason for hiding this comment

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

how would the CustomControl thing we also want use this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point and the answer is I don't know yet.

I noted that the CustomControl is like a Control that is linked to a layer, something I did not realize at first. In pure Leaflet this is something of an anomaly, as both Controls and Layers are contained in a Map and there is no structural association between Control and Layer. Typically it is the other way round and a Control determines the visibility of the Layer.

My guess is it would be fine as you are implementing it now. It would probably be the same as other Controls that are associated with a set of Layers.

Copy link
Member

Choose a reason for hiding this comment

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

I noted that the CustomControl is like a Control that is linked to a layer, something I did not realize at first. In pure Leaflet this is something of an anomaly, as both Controls and Layers are contained in a Map and there is no structural association between Control and Layer. Typically it is the other way round and a Control determines the visibility of the Layer.

That's interesting... So I guess the custom control is a bit of a weird one, and it's okay not to consider it here.

"""
Add a Leaflet Control object to the map

Parameters
----------
control: str
The javascript class name of the control to be rendered.
position: str
One of "bottomright", "bottomleft", "topright", "topleft"

Examples
--------

>>> import folium
>>> from folium.features import Control, Marker
>>> from folium.plugins import Geocoder

>>> m = folium.Map(
... location=[46.603354, 1.8883335], attr=None, zoom_control=False, zoom_start=5
... )
>>> Control("Zoom", position="topleft").add_to(m)
"""

_template = Template(
"""
{% macro script(this, kwargs) %}
var {{ this.get_name() }}_options = {{ this.options|tojson }};
{% for key, value in this.functions.items() %}
{{ this.get_name() }}_options["{{key}}"] = {{ value }};
{% endfor %}

var {{ this.get_name() }} = new L.Control.{{this._name}}(
{{ this.get_name() }}_options
).addTo({{ this._parent.get_name() }});
{% endmacro %}
"""
)

def __init__(
self,
control: Optional[str] = None,
position: Literal[
"bottomright", "bottomleft", "topright", "topleft"
] = "bottomleft",
**kwargs,
):
super().__init__()
if control:
self._name = control
kwargs["position"] = position

# extract JsCode objects
self.functions = {}
for key, value in list(kwargs.items()):
if isinstance(value, JsCode):
self.functions[camelize(key)] = value.js_code
kwargs.pop(key)

self.options = parse_options(**kwargs)
4 changes: 2 additions & 2 deletions folium/plugins/beautify_icon.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ def __init__(
inner_icon_style="",
spin=False,
number=None,
**kwargs
**kwargs,
Copy link
Member

Choose a reason for hiding this comment

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

why are all these trailing commas in this PR? Nothing can follow a **kwargs entry so a trailing comma is not necessary. Black or ruff doesn't add these if I'm not mistaken, so why add them?

Copy link
Collaborator Author

@hansthen hansthen Jun 16, 2024

Choose a reason for hiding this comment

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

They were added by ruff format, so don't blame me :-). I agree they do not make sense in this case. As far as I know there is no option to remove these trailing commas. I can manually undo these kind of changes, but its a hassle. I'd prefer to just blindly follow ruff.

Copy link
Member

Choose a reason for hiding this comment

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

We don't run ruff format on this repo yet though. We use Black to autoformat and have Ruff as linter. So it's better not to run ruff format, since it will create a lot of changes.

Copy link
Member

Choose a reason for hiding this comment

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

We could switch over to ruff format though and ditch Black. Maybe that's good to do anyway, and if we merge that before this PR that also solves the issue.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

My bad, I thought we used ruff as a formatter as well. I can create an extra PR to ditch black if you want, but I have no strong opinion either way. If not I will undo the ruff formatting changes.

):
super().__init__()
self._name = "BeautifyIcon"
Expand All @@ -111,5 +111,5 @@ def __init__(
spin=spin,
isAlphaNumericIcon=number is not None,
text=number,
**kwargs
**kwargs,
)
5 changes: 3 additions & 2 deletions folium/plugins/draw.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
from branca.element import Element, Figure, MacroElement
from branca.element import Element, Figure
from jinja2 import Template

from folium.elements import JSCSSMixin
from folium.features import Control


class Draw(JSCSSMixin, MacroElement):
class Draw(JSCSSMixin, Control):
"""
Vector drawing and editing plugin for Leaflet.

Expand Down
8 changes: 4 additions & 4 deletions folium/plugins/fullscreen.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
from branca.element import MacroElement
from jinja2 import Template

from folium.elements import JSCSSMixin
from folium.features import Control
from folium.utilities import parse_options


class Fullscreen(JSCSSMixin, MacroElement):
class Fullscreen(JSCSSMixin, Control):
"""
Adds a fullscreen button to your map.

Expand Down Expand Up @@ -56,7 +56,7 @@ def __init__(
title="Full Screen",
title_cancel="Exit Full Screen",
force_separate_button=False,
**kwargs
**kwargs,
):
super().__init__()
self._name = "Fullscreen"
Expand All @@ -65,5 +65,5 @@ def __init__(
title=title,
title_cancel=title_cancel,
force_separate_button=force_separate_button,
**kwargs
**kwargs,
)
8 changes: 4 additions & 4 deletions folium/plugins/geocoder.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
from typing import Optional

from branca.element import MacroElement
from jinja2 import Template

from folium.elements import JSCSSMixin
from folium.features import Control
from folium.utilities import parse_options


class Geocoder(JSCSSMixin, MacroElement):
class Geocoder(JSCSSMixin, Control):
"""A simple geocoder for Leaflet that by default uses OSM/Nominatim.

Please respect the Nominatim usage policy:
Expand Down Expand Up @@ -78,7 +78,7 @@ def __init__(
zoom: Optional[int] = 11,
provider: str = "nominatim",
provider_options: dict = {},
**kwargs
**kwargs,
):
super().__init__()
self._name = "Geocoder"
Expand All @@ -89,5 +89,5 @@ def __init__(
zoom=zoom,
provider=provider,
provider_options=provider_options,
**kwargs
**kwargs,
)
4 changes: 2 additions & 2 deletions folium/plugins/groupedlayercontrol.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
from branca.element import MacroElement
from jinja2 import Template

from folium.elements import JSCSSMixin
from folium.features import Control
from folium.utilities import parse_options


class GroupedLayerControl(JSCSSMixin, MacroElement):
class GroupedLayerControl(JSCSSMixin, Control):
"""
Create a Layer Control with groups of overlays.

Expand Down
4 changes: 2 additions & 2 deletions folium/plugins/heat_map.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ def __init__(
overlay=True,
control=True,
show=True,
**kwargs
**kwargs,
):
super().__init__(name=name, overlay=overlay, control=control, show=show)
self._name = "HeatMap"
Expand All @@ -96,7 +96,7 @@ def __init__(
radius=radius,
blur=blur,
gradient=gradient,
**kwargs
**kwargs,
)

def _get_self_bounds(self):
Expand Down
4 changes: 2 additions & 2 deletions folium/plugins/locate_control.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@
Based on leaflet plugin: https://github.com/domoritz/leaflet-locatecontrol
"""

from branca.element import MacroElement
from jinja2 import Template

from folium.elements import JSCSSMixin
from folium.features import Control
from folium.utilities import parse_options


class LocateControl(JSCSSMixin, MacroElement):
class LocateControl(JSCSSMixin, Control):
"""Control plugin to geolocate the user.

This plugins adds a button to the map, and when it's clicked shows the current
Expand Down
2 changes: 1 addition & 1 deletion folium/plugins/marker_cluster.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ def __init__(
show=True,
icon_create_function=None,
options=None,
**kwargs
**kwargs,
):
if options is not None:
kwargs.update(options) # options argument is legacy
Expand Down
8 changes: 4 additions & 4 deletions folium/plugins/measure_control.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
from branca.element import MacroElement
from jinja2 import Template

from folium.elements import JSCSSMixin
from folium.features import Control
from folium.utilities import parse_options


class MeasureControl(JSCSSMixin, MacroElement):
class MeasureControl(JSCSSMixin, Control):
"""Add a measurement widget on the map.

Parameters
Expand Down Expand Up @@ -68,7 +68,7 @@ def __init__(
secondary_length_unit="miles",
primary_area_unit="sqmeters",
secondary_area_unit="acres",
**kwargs
**kwargs,
):
super().__init__()
self._name = "MeasureControl"
Expand All @@ -79,5 +79,5 @@ def __init__(
secondary_length_unit=secondary_length_unit,
primary_area_unit=primary_area_unit,
secondary_area_unit=secondary_area_unit,
**kwargs
**kwargs,
)
8 changes: 4 additions & 4 deletions folium/plugins/minimap.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
from branca.element import MacroElement
from jinja2 import Template

from folium.elements import JSCSSMixin
from folium.features import Control
from folium.raster_layers import TileLayer
from folium.utilities import parse_options


class MiniMap(JSCSSMixin, MacroElement):
class MiniMap(JSCSSMixin, Control):
"""Add a minimap (locator) to an existing map.

Uses the Leaflet plugin by Norkart under BSD 2-Clause "Simplified" License.
Expand Down Expand Up @@ -103,7 +103,7 @@ def __init__(
toggle_display=False,
auto_toggle_display=False,
minimized=False,
**kwargs
**kwargs,
):
super().__init__()
self._name = "MiniMap"
Expand All @@ -128,5 +128,5 @@ def __init__(
toggle_display=toggle_display,
auto_toggle_display=auto_toggle_display,
minimized=minimized,
**kwargs
**kwargs,
)
58 changes: 17 additions & 41 deletions folium/plugins/mouse_position.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
from branca.element import MacroElement
from jinja2 import Template

from folium.elements import JSCSSMixin
from folium.utilities import parse_options
from folium.features import Control
from folium.utilities import JsCode


class MousePosition(JSCSSMixin, MacroElement):
class MousePosition(JSCSSMixin, Control):
"""Add a field that shows the coordinates of the mouse position.

Uses the Leaflet plugin by Ardhi Lukianto under MIT license.
Expand Down Expand Up @@ -45,34 +43,6 @@ class MousePosition(JSCSSMixin, MacroElement):

"""

_template = Template(
"""
{% macro script(this, kwargs) %}
var {{ this.get_name() }} = new L.Control.MousePosition(
{{ this.options|tojson }}
);
{{ this.get_name() }}.options["latFormatter"] =
{{ this.lat_formatter }};
{{ this.get_name() }}.options["lngFormatter"] =
{{ this.lng_formatter }};
{{ this._parent.get_name() }}.addControl({{ this.get_name() }});
{% endmacro %}
"""
)

default_js = [
(
"Control_MousePosition_js",
"https://cdn.jsdelivr.net/gh/ardhi/Leaflet.MousePosition/src/L.Control.MousePosition.min.js",
)
]
default_css = [
(
"Control_MousePosition_css",
"https://cdn.jsdelivr.net/gh/ardhi/Leaflet.MousePosition/src/L.Control.MousePosition.min.css",
)
]

def __init__(
self,
position="bottomright",
Expand All @@ -83,19 +53,25 @@ def __init__(
prefix="",
lat_formatter=None,
lng_formatter=None,
**kwargs
**kwargs,
):
super().__init__()
self._name = "MousePosition"

self.options = parse_options(
super().__init__(
control="MousePosition",
position=position,
separator=separator,
empty_string=empty_string,
lng_first=lng_first,
num_digits=num_digits,
prefix=prefix,
**kwargs
lat_formatter=JsCode(lat_formatter) if lat_formatter else None,
lng_formatter=JsCode(lng_formatter) if lng_formatter else None,
**kwargs,
)
self.add_js_link(
"Control_MousePosition_js",
"https://cdn.jsdelivr.net/gh/ardhi/Leaflet.MousePosition/src/L.Control.MousePosition.min.js",
)
self.add_css_link(
"Control_MousePosition_css",
"https://cdn.jsdelivr.net/gh/ardhi/Leaflet.MousePosition/src/L.Control.MousePosition.min.css",
Comment on lines +70 to +76
Copy link
Member

Choose a reason for hiding this comment

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

please use the JSCSSMixin class attributes for these

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Why? The method calls seem cleaner to me compared to the static attributes?

Copy link
Member

@Conengmo Conengmo Jun 16, 2024

Choose a reason for hiding this comment

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

the static attributes allow users that want to create offline maps to bundle these dependencies and then update the values in the static attributes. I know they might also use these new methods, but it's breaking behavior to define these in the init for this use case.

Also, I think it's better to have a consistent way of working. So not have two different ways of defining dependencies.

)
self.lat_formatter = lat_formatter or "undefined"
self.lng_formatter = lng_formatter or "undefined"
Loading