-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
multiple markers with one icon #2053
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -271,7 +271,6 @@ class Icon(MacroElement): | |
var {{ this.get_name() }} = L.AwesomeMarkers.icon( | ||
{{ this.options|tojavascript }} | ||
); | ||
{{ this._parent.get_name() }}.setIcon({{ this.get_name() }}); | ||
{% endmacro %} | ||
""" | ||
) | ||
|
@@ -322,6 +321,24 @@ def __init__( | |
**kwargs, | ||
) | ||
|
||
class SetIcon(MacroElement): | ||
"""Set the icon of a marker after both are created.""" | ||
_template = Template(""" | ||
{% macro script(this, kwargs) %} | ||
{{ this.marker.get_name() }}.setIcon({{ this.icon.get_name() }}); | ||
{% endmacro %} | ||
""") | ||
|
||
def __init__(self, marker: 'Marker', icon: 'Icon'): | ||
super().__init__() | ||
self._name = "SetIcon" | ||
self.marker = marker | ||
self.icon = icon | ||
|
||
def register_marker(self, marker: "Marker") -> None: | ||
"""Register a marker to use this icon, so one icon can have multiple markers.""" | ||
self.add_child(Icon.SetIcon(marker=marker, icon=self)) | ||
|
||
|
||
class Marker(MacroElement): | ||
""" | ||
|
@@ -383,7 +400,11 @@ def __init__( | |
self.options = remove_empty( | ||
draggable=draggable or None, autoPan=draggable or None, **kwargs | ||
) | ||
self.icon: Optional[Icon] = None | ||
if icon is not None: | ||
# here we make sure the icon is rendered after the last marker that needs it | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It feels more natural to make SetIcon a child of Marker and add the Icon to the map instead of to the Marker. |
||
if icon._parent is not None: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These two lines feel a bit awkward. Also they are not necessary if we make Icon a child of Map like in my earlier comments. |
||
del icon._parent._children[icon.get_name()] | ||
self.add_child(icon) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In my proposal, the Icon could be added to the map (or to the parent of the Marker). To ensure backwards compatibility we can add the icon from the marker. We'd have to move it either to I am not sure if |
||
self.icon = icon | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you follow my setIcon proposal, we could call it here. |
||
if popup is not None: | ||
|
@@ -406,6 +427,8 @@ def render(self) -> None: | |
raise ValueError( | ||
f"{self._name} location must be assigned when added directly to map." | ||
) | ||
if self.icon: | ||
self.icon.register_marker(self) | ||
super().render() | ||
|
||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To me it would feel more natural to have a
setIcon
method onMarker
(which generates the same code).