highlight_color for vector classes #1866
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Conengmo
reviewed
Feb 19, 2024
There was a problem hiding this comment.
Thanks for working in this @yschopfer19! I’m on mobile so I haven’t looked into this that detailed yet, but I wanted to share my first thought. Which is that I’d rather avoid adding the repeated code. Maybe you can think of a way to have that extra template code only in one place?
I do like the idea of having highlight functionality on vector classes, but if the code becomes much longer or more complex I’m not sure if it’s worth it.
I hope you’re not discouraged by this. And if you want I can help with this as well later.
thanks again for your effort!
hansthen
added a commit
to hansthen/folium
that referenced
this pull request
Mar 14, 2024
In combination with JsCode this makes it easier for users to add `on` method calls for event handling without extending Folium itself. The functionality was inspired by PR python-visualization#1866 by @yschopfer19. The PR was not accepted yet, because of concerns with code duplication. In the approach taken in the current PR, python-visualization#1866 would not be necessary anymore, as the requested changes could be added completely in client code space.
hansthen
added a commit
to hansthen/folium
that referenced
this pull request
May 18, 2024
In combination with JsCode this makes it easier for users to add `on` method calls for event handling without extending Folium itself. The functionality was inspired by PR python-visualization#1866 by @yschopfer19. The PR was not accepted yet, because of concerns with code duplication. In the approach taken in the current PR, python-visualization#1866 would not be necessary anymore, as the requested changes could be added completely in client code space.
Conengmo
added a commit
that referenced
this pull request
Jun 2, 2024
* Add EventHandler to layer object In combination with JsCode this makes it easier for users to add `on` method calls for event handling without extending Folium itself. The functionality was inspired by PR #1866 by @yschopfer19. The PR was not accepted yet, because of concerns with code duplication. In the approach taken in the current PR, #1866 would not be necessary anymore, as the requested changes could be added completely in client code space. * Make realtime inherit from Layer * Changes after review comments by conengmo * Updates after review comments * Add extra docstring line * Add Evented class In Leaflet Evented is the parent class of both `L.Map` and `L.Layer`. It adds the `on` method which can be used to add event handlers to a leaflet object. * Update folium/map.py Co-authored-by: Frank Anema <[email protected]> * Update folium/map.py Co-authored-by: Frank Anema <[email protected]> * As requested in review comment --------- Co-authored-by: Frank Anema <[email protected]>
|
Since #1959 was merged maybe that can be used for this functionality. |
|
@yschopfer19 By now we implemented #1959, which allows users to add javascript event handlers. It allows users to do what you proposed in your PR. If #1959 does not serve your need, please let us know. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The implementation introduces the possibility to highlight polylines, polygons, rectangles, circles, and circle markers if a tooltip is shown.
Changes Made:
Related Issue: #1763