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

Add touch support #2

Closed
jacobtoye opened this issue Jun 12, 2012 · 99 comments
Closed

Add touch support #2

jacobtoye opened this issue Jun 12, 2012 · 99 comments

Comments

@jacobtoye
Copy link
Member

Methodology of drawing doesn't work at all for touch browsers :|

  • Polylines and ploygons: it is too hard to click on the close marker.
  • Rectangles and Markers:drawn by mousedown -> mousemove -> mouseup, this won't work on touch devices.
@mphasize
Copy link

mphasize commented Jul 1, 2012

Hello Jacob,

I'm using your code for a project which I'm currently building for the iPad, so I added some "hacky" touch support. It's not perfect, but works for me. (I'm using it for drawing polygons only so far....) Maybe that could work as a starting point for you. Cheers and thanks for your work!

mphasize@0455bb2

Marcus

@jacobtoye
Copy link
Member Author

Awesome! Thanks for that @mphasize. Your code will be a good start. I'll hopefully get the time within the next month of so to add support for the rest of the shapes/markers.

@mphasize
Copy link

Had some time to do it myself . :-) See https://github.com/jacobtoye/Leaflet.draw/pull/5

@gamb
Copy link

gamb commented Apr 16, 2013

+1 is touch polygon drawing on the horizon?

@jacobtoye
Copy link
Member Author

Honestly not at this stage. :(

@gamb
Copy link

gamb commented Apr 17, 2013

Alas... I actually have sufficient impetus to work on this today, assume the solution isn't simply a case of binding ~touchClick events then? Before I take a look, are there some design issues I should consider?

@jacobtoye
Copy link
Member Author

Yeah I wish it were that easy :( unfortunately we would need to redo most if the handlers. Check out my responses to the pull request referenced above. Let me know what you think.

@kingrollo
Copy link

Could I possibly request two things if you're not planning to implement this at present? Firstly, could you look at disabling the buttons on mobile devices? Secondly, could you make it clear in your front page on Github that it isn't available on mobile? Just for the sake of clarity, you know...

Both would be much appreciated!

@thejae
Copy link

thejae commented Jun 23, 2013

Hi jacobtoye & philipn, first of all, great work!! I'd like to know, if I want iPad support today, what is the easiest hack I can apply?

Thanks!

@jacobtoye
Copy link
Member Author

@thejae your best bet is to check out what this pull was doing and try apply it to a fork. It will be quite a bit of work :(

@thejae
Copy link

thejae commented Jun 25, 2013

@jacobtoye Thanks for your reply :) do you accept donations in exchange for feature requests? If yes, what kind of donations are we speaking about for Touch support?

@philipn
Copy link

philipn commented Jul 22, 2013

@jacobtoye This plugin is fantastic! I'd also be interested in knowing how much time and money it'd take to complete this work, as we may be able to sponsor a bit.

@jacobtoye
Copy link
Member Author

@philipn @thejae Thanks for the offer guys, however I'm too busy to work on another project.

Unfortunately it wouldn't be as simple as adding some touch handlers to the plugin. The handlers would need to create and edit the shapes differently.

@philipn
Copy link

philipn commented Jul 24, 2013

Thanks! @mphasize - do you have any plans to dive back into this work?

@mphasize
Copy link

@philipn Hej Philip, I don't have any immediate plans to get back to this, although I might need the touch support myself for a little side project next year.
Getting basic touch support working was easy a year ago, but I didn't stay up to date with Leaflet, so I don't know how that might have changed. Anyway, for public deployment you would want something more than just basic support. I think @jacobtoye already tried to explain, that the interaction patterns between mouse and touch drawing are quite different.
The basic support could probably be done in a day or two, but in order to create a natural, performant and delightful solution for touch drawing, I would probably need at least a weeks worth of programming hours plus time for the interaction design and graphics plus time for some user testing in between.

Just a few things to think about: Bigger touch targets for the handles and as a result of this coping with the problem of overlapping handles. Easy adjustment and repositioning of all handles even while drawing the shapes (since we have more than one finger, we should be able to use them). How to do exact positioning when the touch point is hidden behind your finger. How to deal with different screen sizes from phone to tablet and so on.

Cheers!
M

@pztrick
Copy link
Contributor

pztrick commented Dec 6, 2013

+1 for touch support :)

I looked at pull 5 and am dismayed that it apparently (?) no longer works for master branch. It seems like Polygon will be easier to implement (well, elegantly anyway) so if I find the time I'll effort that...

@pztrick
Copy link
Contributor

pztrick commented Dec 25, 2013

Here is my work-in-progress. Basic touch support for Polyline/Polygon seems to be working in my application.

pztrick/Leaflet.draw@d567caf

h/t @mphasize

@sellonen
Copy link

sellonen commented Jan 2, 2014

Hey @pztrick , against all odds, your solution works really fine on my friend's Windows phone as well. Thanks a million!

@pztrick
Copy link
Contributor

pztrick commented Jan 28, 2014

@sellonen It looks like a recent Chrome update vanquished my fix. I'll probably get to fixing it today or tomorrow if it's easy enough. (Still working in IE11.)

Just FYI in case you are using it!

@sellonen
Copy link

Thanks for keeping me updated. My need will in fact only materialize in about a month. Markers are by far the most important in our product, and I think I will make a slightly different implementation for touch users: When the drawing starts, a marker is placed in the middle of the map with a popup saying "Drag the map until I'm at the correct location, and then click ok". The marker stays in the middle while map is being dragged, of course. I think that will be easier for touch users, who otherwise cannot see a marker until they have clicked on the map and then drawing finishes already... Also it's quite easy to accidentally produce a click event on the map on touch devices, so I don't think binding anything to it will be a good idea. Though this experience is with a bit older phones and openlayers, i have not tested modern tools.

@pztrick
Copy link
Contributor

pztrick commented Jan 28, 2014

Panning is a neat idea. Good luck!

@ghost
Copy link

ghost commented Feb 20, 2014

I developed my webapplication specifically with leaflet and leaflet.draw because leaflet has a good support for mobile devices. I wasn't aware that leaflet.draw didn't support touch events :-(

My workaround is to use the Dolphin browser, the only browser on Android in which my webapplication works the same as on my laptop/desktop. My webapplication doesn't work in FireFox and Chrome or Chroma Beta (I didn't test Opera because Dolphin works ok)

Hopefully leaflet.draw will get native touch support so that it will work in all mobile browser.

BTW I use geoJSON with polygon's, polylines and points/markers in combination with a non-real-world background, and accuracy is not very important in my case. It's a kind of a rough map sketch tool

@DrPandemic
Copy link

Thank you for all your work @michaelguild13, your fork saved me!
I found something interresting while playing with it on iPad. While drawing a new polygon, I was unable to easily add "points" to this polygon if I was clicking on a zone with multiple layers under it. I think I had 3 or 4 layers.
To fix it I simply reduce the number of layers that I was using.

@michaelguild13
Copy link
Contributor

Thanks Jean! I've been meaning to update this lib with the one on ng's
mapmaker but haven't found the time. I'll do my best to fix that this week.
Sorry to everyone working with the old version.

On Monday, August 24, 2015, Jean-Samuel Aubry-Guzzi <
[email protected]> wrote:

Thank you for all your work @michaelguild13
https://github.com/michaelguild13, your fork saved me!
I found something interresting while playing with it on iPad. While
drawing a new polygon, I was unable to easily add "points" to this polygon
if I was clicking on a zone with multiple layers under it. I think I had 3
or 4 layers.
To fix it I simply reduce the number of layers that I was using.


Reply to this email directly or view it on GitHub
#2 (comment).

Michael Guild
571.338.9782
[email protected]

@MeiRct
Copy link

MeiRct commented Sep 3, 2015

@michaelguild13 Can you please list the changes that should come with the updated version?
@jacobtoye Are you still on this project, or we should follow another one?

@michaelguild13
Copy link
Contributor

@MeiRct , right now I am trying to get the code that I built for the mapmaker project so I can update this repo with the latest. But I currently no longer work for natgeo so there's that. For now, I am just going to go though this chat and trouble shoot what I can.

@sellonen
Copy link

@michaelguild13 I've been using your code for a while now, thank you a lot. However, windows phones sometimes send 'touchcancel' and 'touchleave' events, causing on error in the Leaflet function L.DomEvent.AddPointerListener ( https://github.com/Leaflet/Leaflet/blob/stable/src/dom/DomEvent.Pointer.js#L20 )

I could overcome this by editing that function directly in Leaflet, but I'd rather not do it, not least because of the changes coming up in leaflet-1.0. Could we somehow treat touchcancel and touchleave as touchend events directly in Leaflet.draw? That way I wouldn't have to create a fork of my own for Leaflet.

@michaelguild13
Copy link
Contributor

If your report a bug, I'll do a fix for it.

On Monday, September 14, 2015, sellonen [email protected] wrote:

@michaelguild13 https://github.com/michaelguild13 I've been using your
code for a while now, thank you a lot. However, windows phones sometimes
send 'touchcancel' and 'touchleave' events, causing on error in the Leaflet
function L.DomEvent.AddPointerListener (
https://github.com/Leaflet/Leaflet/blob/stable/src/dom/DomEvent.Pointer.js#L20
)

I could overcome this by editing that function directly in Leaflet, but
I'd rather not do it, not least because of the changes coming up in
leaflet-1.0. Could we somehow treat touchcancel and touchleave as touchend
events directly in Leaflet.draw? That way I wouldn't have to create a fork
of my own for Leaflet.


Reply to this email directly or view it on GitHub
#2 (comment).

Michael Guild
571.338.9782
[email protected]

@sellonen
Copy link

Do you mean "raise an issue"? I'm probably a bit noob with github, but there is no "issues" section in your fork of Leaflet.draw.

@michaelguild13
Copy link
Contributor

haha, you're right...weird...
ok, I'll look into this tomorrow evening.

On Mon, Sep 14, 2015 at 7:43 AM, sellonen [email protected] wrote:

Do you mean "raise an issue"? I'm probably a bit noob with github, but
there is no "issues" section in your fork of Leaflet.draw.


Reply to this email directly or view it on GitHub
#2 (comment).

Michael Guild
571.338.9782
[email protected]

@alexkb
Copy link

alexkb commented Oct 21, 2015

@michaelguild13 any chance of getting that issue queue going for your fork? P.S. Great work on getting touch support.

@michaelguild13
Copy link
Contributor

@sellonen , What's your fix for the windows phone?
@alexkb , Sadly, I don't have a windows phone to test with and I also don't have a good emulator setup to catch the events.

To play it safe, might change the code up so touchleave or touchcancel are treated the same as touchend.

L.DomEvent.on(this._container, 'touchcancel', this._onTouchEnd, this);
L.DomEvent.on(this._container, 'touchleave', this._onTouchEnd, this);

@sellonen
Copy link

Hi, because I have never really looked at the code of Leaflet.Draw and didn't feel comfortable editing it, I resorted to making small modifications to Leaflet itself. Here's what worked: sellonen/Leaflet@0f7760b

Let me know if an update to your Leaflet.Draw renders my hack unnecessary.

@alexkb
Copy link

alexkb commented Oct 21, 2015

@michaelguild13 I don't have a windows phone either. My point about the issue queue was so we could ask other questions relating to your fork, instead of asking them all in this one thread! :)

@sellonen
Copy link

I can test Windows phone 8 if you publish a demo. And I agree, this discussion should take place somewhere else...

@michaelguild13
Copy link
Contributor

Ok, so I am an idiot and didn't realize that I had issues setting turned off on this repo. Anyhoo,
Everyone, please report issues here https://github.com/michaelguild13/Leaflet.draw/issues.

Also, if you want to work with me on this, please feel free to do pull requests.

@omeid
Copy link

omeid commented Nov 26, 2015

@michaelguild13 What is the end goal of the fork? are you intending to create a pull request at some stage?

@alexkb
Copy link

alexkb commented Nov 26, 2015

@michaelguild13 you're no idiot in my book, you've done some great work making an already good library work on a trending platform (mobile) and you should commended!

@omeid
Copy link

omeid commented Nov 26, 2015

Just saw #285, never mind. :) Good work by the way!

@michaelguild13
Copy link
Contributor

@omeid , I tried doing a pull request a few years ago and they didn't want to accept it, stating that they wanted me to create another class for only touch instead of integrating it as I have.

If they want it, I am more than happy for them to take it.

Also, I haven't been that active on this due to working more in react as of late.

ddproxy pushed a commit that referenced this issue Dec 18, 2016
# This is the 1st commit message:

checking distance to last point
fixes; touch event still appears to be firing twice on IE
change gitignore so we can deploy git repo
add touch blocker
block touch when touch has already begun
touch events and log
browser.touch typo
dont show tooltip text for touch
move browser touch check to disable showlength
try timeout approach
debugging intersections
try different distance method
alternate method of distance calculation
another approach to distance
function for approaching close distance
lower threshold for finishing shape and log
prevent L.browser unbinding issue

# This is the commit message #2:

remove conditional application of event handlers and onMouseMove call from onTouch
@ddproxy ddproxy closed this as completed Dec 18, 2016
@seedgabo
Copy link

seedgabo commented May 26, 2017

is this folk working with the latest version of leaflet right now?

@michaelguild13
Copy link
Contributor

Most likely not. I've given up supporting this fork since I've been busy on other projects.
I think their latest version took some of the code from this fork and merged it in...I think... No promises :P

@kajeagentspi
Copy link

This issue still exists on version 1.0.4. Thankfully @michaelguild13 fork still works but with warnings though.

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

No branches or pull requests