-
Notifications
You must be signed in to change notification settings - Fork 409
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
Also store pixel position in state.mousePosition #1211
Conversation
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.
Please add tests
@@ -268,7 +268,8 @@ let LeafletMap = React.createClass({ | |||
this.props.onMouseMove({ | |||
x: pos.lng, | |||
y: pos.lat, | |||
crs: "EPSG:4326" | |||
crs: "EPSG:4326", | |||
pixel: event.containerPoint |
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.
I would copy coordinates also in this case, instead of storing Leaflet objects
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.
Should I also change other places in the leaflet/Map.jsx which use event.containerPoint (and even event.latlng), i.e. in the singleclick
or contextmenu
handlers?
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.
it would be useful, but only if it doesn't take you too much time
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.
That's the smaller part, the bigger part is figuring out how to write a useful test about this ;)
83d6ad3
to
21fe12f
Compare
I haven't found a way to add a sensible test for this. I suppose a proper test would be simulating a mouse move event and checking that the i.e. mouseMove event handler gets called with object one expects. However I can't find a way to simulate a mousemove event, below does not work:
|
Useful i.e. to display a hover tooltip in an "on-the-fly" identify mode.