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

change "drag a point" example to use Marker #6102

Closed
mollymerp opened this issue Feb 6, 2018 · 3 comments
Closed

change "drag a point" example to use Marker #6102

mollymerp opened this issue Feb 6, 2018 · 3 comments

Comments

@mollymerp
Copy link
Contributor

Performance isn't great on https://www.mapbox.com/mapbox-gl-js/example/drag-a-point/ and this is a pain point for users (#2334 (comment)). Using Marker would drastically simplify the code in the example and have better performance because we wouldn't be using setData to change the location.

@jfirebaugh
Copy link
Contributor

Since this would require doing much of the work for #3087 (Add "draggable" property to "Marker" class), it's probably best just to do that. Then the example is a one-liner.

@jmandel1027
Copy link
Contributor

jmandel1027 commented Feb 14, 2018

I'd love to take this on, I got really familiar with Marker while working on #6031 with @andrewharvey.

I agree @jfirebaugh, this would make it really easy for people to use. If it checks out, I can totally include docs for the changes in the PR with a simple example and explanation.

Exposing a draggable option that defaults to false from the Marker class sounds like a good way to go. This can reference an onDrag() method that updates the marker to new coordinates when the mouseenter && click events end.

@jmandel1027
Copy link
Contributor

jmandel1027 commented Feb 19, 2018

Hi I was looking into this a bit over the weekend and wanted to get feedback on two possible routes to take for adding dragging behaviors to marker points, I've looked into both routes but wanted to check in and get feedback on my approach.

option 1: A dedicated drag_marker event handler

after looking into how drag_pan handler was working to get more of a feel for how events are treated, I thought it might be potentially advantageous to house this functionality in a dedicated drag_marker event handler file that hooks into the marker element, I tried this initially but got a bit stuck. I tried this first as opposed to the maybe simpler solution of just tacking this onto marker for a few reasons:

  • keep main marker file lightweight as this is a particular, though popular use case.
  • stay consistent with patterns used in other event handlers ie pan, rotate, zoom.

I did make some progress though still have to sort out a few more of the nuts and bolts, namely correctly hooking up the drag_marker handler to marker, as well as writing further tests, this approach however might be unnecessarily complex.

option 2: Embedding this into Marker

This was the other route I started to explore after hitting a wall with option one. Didn't see any performance downsides with going this route but I liked the idea of splicing away the functionality into a separate file. However I'm realizing its simpler just to have this built into marker rather than a separate handler process. I could be totally overthinking this as i'm still getting familiar with the architecture of the project and open source contributing in general but thought i'd mention my approach as I work on things. 🌱n_n

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants