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

Touch Support for Drawing Tools #5

Closed
wants to merge 4 commits into from
Closed

Conversation

mphasize
Copy link

Hi Jacob,

I had some time to do the rest of the touch support for Leaflet.Draw. I did some cleanup and improvements and while there's still more things to be considered, all drawing tools should work properly on multitouch browsers. (Well, actually I have only iOS devices for testing, but touch api should be similar on Android.)

So here's the details:

  • registration of touch events is conditional based on L.Browser.touch
  • same goes for label texts "tap" or "click" and iconSize of polyline/polygon handlers
  • there's a touch target multiplier, so that a small invisible area around the marker is still a valid touch target
  • touch move events prevent the map from panning while you're trying to draw something

More ideas:

  • As you point out in issue Polyline/polygon vertex markers useful? #3, all those touch markers are very confusing. I would suggest to keep all the markers, though not clickable and very small, and have the marker that finishes drawing stand out by making it the big one. (Maybe even use an icon on it?)
  • Hints: With mouse interaction it makes sense to have the hints following the mouse cursor, but in touch interaction there's no hovering, so this turns out to be a problem – especially the first hint (explaining how to start). I think it makes sense to show the hints in a static location, somewhere along the top of the map (so that they're not hidden by the users hand). At the very least, all the hints need to be positioned above the finger, instead of below.

So far. I hope you like the work.
Cheers!
Marcus

@jacobtoye
Copy link
Member

Awesome work @mphasize! This looks really good. Seeing this made my day :) I'll have a play with it tomorrow and accept the pull.

@mourner
Copy link
Member

mourner commented Jul 12, 2012

(comment deleted) oh nevermind, I got it wrong :)

@mourner
Copy link
Member

mourner commented Jul 12, 2012

@mphasize @jacobtoye guys, I see there's a lot of stuff in code that should be handled by Leaflet instead of having to do it manually - mousemove/touchmove tracking, coordinates conversion etc. Maybe it would be better if we could improve Leaflet's map events to cover all the cases?

@jacobtoye
Copy link
Member

Hi @mourner. I'll look at seeing what we can move over to Leaflet in the next few weeks. Now that you mention it, it makes a lot of sense to bind to the map rather than directly to the container.

@mphasize
Copy link
Author

Hej, sounds like a good idea to reuse existing funtionality in Leaflet. Although I did some poking around in the code, I have to admit I didn't even know the Marker.Drag stuff existed. Alas, my project on geovis is done for the moment and I have to move on to other stuff, so I just wanted to give something back and say thanks for being able to build on top of your work!
(Well, I might actually still fix a small bug when drawing polygons, where you are able to click the first point and close the polygon immediately – instead of creating a second and third point first...)

Cheers!
M

@TwistaTim
Copy link

Hey Guy's, Just wondering what the progress of this situation is.....
I see mphasize's pull request hasn't been merged with the main stream.

I am deploying a web app that has the draw plugin but I am wondering if i should hide the draw controls on any touch device due to it not working?

Fantastic plugin btw :)

@jacobtoye
Copy link
Member

I'd suggest hiding the controls of touch devices.

I haven't had a chance to decide the best way to do touch support, or even if it is necessary (there hasn't been many requests). I'm keeping this pull open so I can check it out when I get around to implementing touch support.

@davewalk
Copy link

Hi, for what's it worth I just released an application that I could use touch support for so that it could work on tablets.

@Elexy
Copy link

Elexy commented Jan 22, 2013

My 2c: I'd like to use touch drawing. For my current project, mobile wen app and I need to release soon, I'll go with native maps then I guess.

@ghost
Copy link

ghost commented Feb 18, 2013

We'd love to see touch drawing. We run a mobile app (HTML5+PhoneGap+Leaflet) and a website that is mobile-friendly.

@jacobtoye
Copy link
Member

The issue with touch is that the drawing and editing handlers will need to be designed with touch devices in mind. This would probably mean touch specific handlers.

Some thoughts:

  • Any handles (square icons on the shapes vertices) need to be larger to support fingers. What are the negatives effects of using larger icons? Look? Overlap?
  • What is the best way to draw rectangle and circles on touch devices (don't have mouse drag).

@max-nova
Copy link

Hi Jacob - I'm one of the guys behind SilviaTerra. I think you raise valid concerns about the touch compatibility, but I also think that one of the great differentiating factors about Leaflet (and why we switched from OpenLayers) was its mobile-friendliness. Happy to think through some of these issues with you and even contribute some code. More and more of our users are going iPad-only, so having a fully-functional mobile solution is pretty important to us.

What would be concrete next steps we could take?

@jacobtoye
Copy link
Member

Hi guys. Unfortunately I need to concentrate one my employers work for quite a while and won't be able to dedicate enough time to Leaflet.draw to add touch support.

If you guys want to fork the repo and start adding support I'd be happy to help with direction and any issues you face.

@jacobtoye
Copy link
Member

Closing this pull as it is old and will no longer work with master. See #2 for touch support issue.

@jacobtoye jacobtoye closed this Feb 19, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants