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 #285

Closed
wants to merge 76 commits into from
Closed

Touch support #285

wants to merge 76 commits into from

Conversation

michaelguild13
Copy link
Contributor

Added touch support for all tools.

@atombender
Copy link
Contributor

@michaelguild13 Just so you know, your patch is full of tab characters. Leaflet.draw uses spaces.

@michaelguild13
Copy link
Contributor Author

As far as I can tell, the src code was already full of tabs...Otherwise my
ide has been inserting them but I've been checking and it doesn't look like
that's the case.

I tend to reframe from tab characters in general. Long story short, I'll
remove there where I see them but I am not going to go though all of
leaflet.draw removing them :)

On Tue, Apr 8, 2014 at 3:38 PM, Alexander Staubo
[email protected]:

@michaelguild13 https://github.com/michaelguild13 Just so you know,
your patch is full of tab characters. Leaflet.draw uses spaces.

Reply to this email directly or view it on GitHubhttps://github.com//pull/285#issuecomment-39892180
.

Michael Guild
571.338.9782
[email protected]

@atombender
Copy link
Contributor

My apologies, it's the opposite — Leaflet uses tabs, you're using spaces. While tabs are evil, the only thing more evil than tabs is to mix spaces and tabs.

@michaelguild13
Copy link
Contributor Author

No worries dood. I am just happy people are reviewing my code :).

On Tue, Apr 8, 2014 at 4:07 PM, Alexander Staubo
[email protected]:

My apologies, it's the opposite -- Leaflet uses tabs, you're using spaces.
While tabs are evil, the only thing more evil than tabs is to mix spaces
and tabs.

Reply to this email directly or view it on GitHubhttps://github.com//pull/285#issuecomment-39895442
.

Michael Guild
571.338.9782
[email protected]

@atombender
Copy link
Contributor

Sure. One comment: Rather than duplicating the hooking logic from L.Marker in L.Marker.Touch, I think you should submit a pull request to the Leaflet project that adds the required touch events. Sounds like a reasonable thing they would accept.

@michaelguild13
Copy link
Contributor Author

ha, not a bad idea...LOL why didn't I think of that. haha, I'll do that
later today! thanks for for that :D

On Tue, Apr 8, 2014 at 4:13 PM, Alexander Staubo
[email protected]:

Sure. One comment: Rather than duplicating the hooking logic from L.Markerin
L.Marker.Touch, I think you should submit a pull request to the Leaflet
project that adds the required touch events. Sounds like a reasonable thing
they would accept.

Reply to this email directly or view it on GitHubhttps://github.com//pull/285#issuecomment-39896001
.

Michael Guild
571.338.9782
[email protected]

@michaelguild13
Copy link
Contributor Author

So the last thing that I am working on is getting the edit for rectangles to work and adding in extra buttons to help the user experience for touch. After that, I will be implementing some new tools.

  • Free hand draw
  • color picker
  • line thickness

@benmanu
Copy link

benmanu commented Jun 9, 2015

@michaelguild13 have you had any insight in the mobile IE issues @baysoft raised above? I've setup a vanilla implementation using your michaelguild13/Leaflet.draw fork but the map isn't working on Nokia Lumia 930 (8.1), or MS Surface. I haven't had much experience debugging IE on mobile so don't have any more details sorry!

@michaelguild13
Copy link
Contributor Author

I can try checking it out this weekend. Chances are, it's something simple
like an extra coma or something sensitive...That's IE forya :P

On Tue, Jun 9, 2015 at 1:25 AM, Ben Manu [email protected] wrote:

@michaelguild13 https://github.com/michaelguild13 have you had any
insight in the mobile IE issues @baysoft https://github.com/baysoft
raised above? I've setup a vanilla implementation using your
michaelguild13/Leaflet.draw fork but the map isn't working on Nokia Lumia
930 (8.1), or MS Surface. I haven't had much experience debugging IE on
mobile so don't have any more details sorry!


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

Michael Guild
571.338.9782
[email protected]

@benmanu
Copy link

benmanu commented Jun 18, 2015

@michaelguild13 I've narrowed this issue down a bit further but my lack of js touchevent knowledge is holding me back. The issue looks to be in the src/ext/TouchEvents.js file, with the touchcancel and touchleave event not being supported in ie mobile, instead of failing gracefully it decides to break the whole map.

@michaelguild13
Copy link
Contributor Author

Yeah, that might be it. Also, this repo doesn't contain the latest from
National Geographic. So, I'll have to update it when I can. Sorry for any
delays.

On Wed, Jun 17, 2015 at 11:42 PM, Ben Manu [email protected] wrote:

@michaelguild13 https://github.com/michaelguild13 I've narrowed this
issue down a bit further but my lack of js touchevent knowledge is holding
me back. The issue looks to be in the src/ext/TouchEvents.js file, with
the touchcancel and touchleave event not being supported in ie mobile,
instead of failing gracefully it decides to break the whole map.


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

Michael Guild
571.338.9782
[email protected]

Matthew McFarland and others added 2 commits August 6, 2015 10:34
* Update `removeHooks` to remove the correct events which are added
  during `addHooks`.  Specifically `mouseup` and `zoomlevelschange` were
  still registered on the map, resulting in errors when the associated
  event handlers were called.

* Update event handler name.  The `zoomlevelchange` handler was updated
  to reflect the actual event being handled.

* Update distribution files.
Remove correct events from Draw.Polyline
@vko-online
Copy link

Does now master branch support touch like in michaelguild13/Leaflet.draw fork?

@baysoft
Copy link

baysoft commented Aug 7, 2015

@michaelguild13 My problems with Internet Explorer 11 is disappear when I'm using Microsoft Edge.(Windows 10), but it still exist while using Microsoft Edge on Windows Phone 10 (Insider Preview build 10.166). Just want you to know, thank you ...

@IvanSanchez
Copy link
Member

@baysoft That sounds very similar to Leaflet/Leaflet#3674. I suggest you try using code from the master branch of Leaflet plus the leaflet-master branch of Leaflet.draw, and see if the problem can be reproduced there.

@omeid omeid mentioned this pull request Nov 26, 2015
@ddproxy ddproxy added the feature label Mar 6, 2016
@ddproxy ddproxy added this to the 0.2.5 milestone Mar 6, 2016
@ddproxy
Copy link
Member

ddproxy commented Mar 7, 2016

@michaelguild13 @fligler this is included in my most recent PR (which includes fixes and a few other PR's) for a 0.2.5 release. Maybe I should move it to 0.3 :/ It's a big change to the library.

@michaelguild13
Copy link
Contributor Author

@ddproxy go for it. I haven't really been watching this repo.

fix hook marker not move in Cicle Edit
remove click handler when disabling L.Draw.Marker
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.