Clicked point to Lonboard map #671
Conversation
…into clicked-point2
|
I think the main question is whether we want to have this as a property or as an event. In general, I think the right way to think about the traitlets is as serializing state. The data state should be the same on each side, so it makes sense for all the data properties to be traitlets. But stuff like the clicked point potentially should be an event handler instead. |
|
yeah, that's fair (and what I was originally going for). I think I got excited to get the information from the javascript side into somewhere I could access it from the python side and ran with it :). I'll poke around some more when I get some more free time to see if I can wire it up as a legit "on_click" event. With me being totally new to this sort of work, if you happen to have any pointers where to read anything about doing something like this, I'm all ears, my random searching the internet hasn't been as productive as I'd like |
You can use custom messages. See https://ipywidgets.readthedocs.io/en/8.1.5/examples/Widget%20Events.html You can use Then register an on_message handler on the Python side: https://github.com/developmentseed/lonboard/pull/413/files#diff-ab269100f27648760a02f3304913cbb24dbb1b8b2a7a3dc8b514aac5c15ff34dR543 And we'd probably have a wrapper to ensure that only messages with an ID of something like |
|
Thanks Kyle, that helped a lot! I appear to have it all working on my end but need to get to work. I should be able to get it cleaned up and checked in tonight though! |
|
ok, all checked in, let me know what you think! |
src/index.tsx
Outdated
| while (x < 180) { | ||
| x += 360; | ||
| } | ||
| while (x > 180) { | ||
| x -= 360; | ||
| } | ||
| info.coordinate[0] = x; |
There was a problem hiding this comment.
Do you know how often this happens? This kind of coordinate shuffling won't work if we support alternate coordinate systems
There was a problem hiding this comment.
The only time this happens is when the user is zoomed out and can see the whole earth wrapped around, and then when they click on the far right side of the map the x coordinate can exceed 180 (and likewise for the left side, you can end up less than -180). if you've got plans to support other coordinate systems, we should certainly exclude this and leave that sort of logic for the to the person writing the callback to implement.
There was a problem hiding this comment.
Yeah I think for now let's remove this coordinate shuffling but document that in some cases the values can be outside the expected range.
There was a problem hiding this comment.
I've added a note to the docstring of on_click function indicating the X/Longitude may be less than or greater than expected, with an example of what will happen if the map is wrapped around and the user clicks on the left or the right part of the earth (hopefully it makes sense, I'm very open to any re-wording/clarification)
There was a problem hiding this comment.
Ideally we would only send back the onClick event if a callback has been registered on the Python side. That means we wouldn't have any performance impact in sending messages for the 99% of the time that a user doesn't have a Python callback registered.
That would mean we'd serialize a trait
So perhaps we should have on_click_handlers as a trait on BaseLayer, which is a tuple holding function callbacks. Then this gets synced to the frontend with custom serialization as a boolean that's just len(traits) > 0. Then if the frontend sees true, it sends the on-click messages back to Python.
And Python would have an observe method that watches the on_click_handlers and updates the global callback registry with those.
What do you think?
lonboard/_map.py
Outdated
| if msg.get("kind") != "on-click": | ||
| return | ||
| self._click_handlers(tuple(msg.get("coordinate"))) |
There was a problem hiding this comment.
I'd rather flip these:
| if msg.get("kind") != "on-click": | |
| return | |
| self._click_handlers(tuple(msg.get("coordinate"))) | |
| if msg.get("kind") != "on-click": | |
| return self._click_handlers(tuple(msg.get("coordinate"))) |
That makes it easier in the future to add other kind of message callbacks
There was a problem hiding this comment.
Good call, I agree that it would be better to only send the message if there is a callback registered.
I'm not following you on the
on_click_handlersas a trait onBaseLayer
why would we put it on the BaseLayer, not keep it on the map?
There was a problem hiding this comment.
Ideally we would only send back the onClick event if a callback has been registered on the Python side. That means we wouldn't have any performance impact in sending messages for the 99% of the time that a user doesn't have a Python callback registered.
That would mean we'd serialize a trait
So perhaps we should have on_click_handlers as a trait on BaseLayer, which is a tuple holding function callbacks. Then this gets synced to the frontend with custom serialization as a boolean that's just len(traits) > 0. Then if the frontend sees true, it sends the on-click messages back to Python.
And Python would have an observe method that watches the on_click_handlers and updates the global callback registry with those.
What do you think?
…ait for the front end to use so unnecessary messages arent sent if there are no python callbacks registered, and flip floppped the guard clause on the _handle_anywidget_dispatch
Sorry I think I meant to write |
excellent, that's what I've got in there now. when a new callback is registered/removed from python in the on_click(), instead of just registering the callback function, it will also populate a new boolean "_has_click_handlers" trait that the javascript side uses to decide if it needs to send the coordinate back to python |
This PR adds a new property to the Lonboard map that holds a tuple for the coordinate where the user last clicked on the map. It is not implemented as an .on_click(), as was discussed, but I think it accomplishes the same functionality (and may actually have more utility this way). hopefully this is an acceptable pattern, but if not, let me know and I'll see what I can do.