map.addControl(draw) breaks touch events for popups

[ebendennis]

Adding the draw controls to a map affects the functionality of touch events for popups. Click events still fire as expected, but touch events no longer fire.

[mcwhittemore]

Eben,

Thanks for bringing this to our attention. Cn you please provide a code example so we can see exactly how it breaks.

Regards, Matthew

[ebendennis]

Matthew,

Been working on this app where I noticed the issue: https://github.com/iconengineering/maps/tree/master/comments-test

[mcwhittemore]

Eben,

Thanks for this. One thing I noticed was that you are loading Draw inside of map.on('load' ...); This might not affect this problem, but its pretty important.

Also, if possible, can you wire up something in jsbin. I need to review a bunch of issues and PRs in a single day, and being able to quickly test them out will help me respond to you. Sorry, I should have said this in my first comment.

Regards, Matthew

[ebendennis]

No problem, here you go - https://jsbin.com/runaxuv/edit?js,output

[mcwhittemore]

Eben,

Thanks for the jsbin. I just play around with this a bit and I see what you mean. My guess is that Draw is stopping all touch events even if it takes no action on the event. We probably need to make sure stopExtendedActions doesn't happen if there are more dom nodes that could handle the event.

https://github.com/mapbox/mapbox-gl-draw/blob/e1df892d543977e02265c7a9f634fc97d076f83b/src/modes/simple_select.js#L117-L126

[Radikaltech]

I'm not really good at coding, still learning my way on it, but I did observe something, I have a div for changing map layers on the lower right of the map on the link below, the pop ups work when I touch(tap) right above the edge in the word "light" you gotta insist sometimes, I wonder why the event running right in that area. https://radikaltech.com/DroneFlightPlan1.6.html (I'm using an Iphone 6+)

[jwilliams-ecometrica]

Hi @mcwhittemore,

I'm just wondering has there being any progress on resolving this issue?

[mcwhittemore]

@jwilliams-ecometrica, nope. If you have time to help fix it, that would be great!

[jwilliams-ecometrica]

Hi @mcwhittemore, Fair enough. I'm not sure quite how to go about fixing this (Other than simply removing the offending call to stopExtendedInteractions().

[mcwhittemore]

stopExtendedInteractions() is being called in too many cases here. Ideally this would only run if the map was the direction of the tap, right now its running when its child elements are clicked as well.

[mcwhittemore]

Really, this might be best solved by adding some sort of filter that only allows events that come directly from the map to be passed to these handlers here.

https://github.com/mapbox/mapbox-gl-draw/blob/d57440cec3a7d55055391f1941e276f1cfa24ecc/src/events.js#L222-L236

[jwilliams-ecometrica]

Hi @mcwhittemore , wouldn't this still mean that click events on the map would be disabled?

[mcwhittemore]

@jwilliams-ecometrica - yep, that is the goal of stopExtendedInteractions(). Draw is changing how the mouse/taps work and wants to make sure more events don't get propagated. The problem this bug points out is that sometimes the user isn't trying to interact with the Map, but because mapbox-gl-js provides an event delegator so all things that listen click or tap or ... are called.

I should note, that this is just my guess. To make sure this works, we need to write a test that proves we've fixed the problem. This way going forward we know we won't break this again.

[jwilliams-ecometrica]

Hi @mcwhittemore, but what if you want to interact with the map outside of mapbox-gl-draw?

[mcwhittemore]

MapboxDraw takes over mouse events as part of the functionality it offers. While it might do this in the extreme, I've found that generally this is OK and what users expect.

[jwilliams-ecometrica]

Hi @mcwhittemore ,

I see, I had thought this was this issue in question. Sorry for misunderstanding!

[chriswhong]

Chiming in, we are experiencing this too, and the workaround has been to simply add and remove the draw control every time the user wants to draw (our use case is simple and we are only using polygon mode, and don't provide editing or deleting of the drawn polygon)

[jonnenauha]

Just got hit with this as well. Noticed that popups no longer close when clicking on map if touch is the way of interaction. Another seems to be that markers cant be tapped to show its attached popup (also listens to map.on("click")). I use chrome dev tools (toggle device toolbar) to emulate a mobile phone. This enables touch events with your mouse.

If you add draw mode into the map, touch taps on the map wont auto close popups. This is caused by draw logic suppressing preventDefault() touchstart/move/end in event handlers. If you comment these lines out it will work as expected. This happens before your custom mode gets the event, even if you implement the touch handlers.

Here is the code, same can be found from the three touch handlers: https://github.com/mapbox/mapbox-gl-draw/blob/master/src/events.js#L80-L83

As I understand it, the map listens for "click" on the dom (canvas or the container). On mobile, touch events also trigger the dom click as well. But seems that if the underlying touchstart/move/end are suppressed/prevented, the browser wont trigger a emulated click event for it.

You can see the intention is good from the code comment, but seems the assumption of proagation is incorrect on mobile. On non mobile, the click will still fire for the map handler, but for mobile it is emulated via touch events, which are being suppressed, so it wont be emulated.

events.touchstart = function(event) {
    // Prevent emulated mouse events because we will fully handle the touch here.
    // This does not stop the touch events from propogating to mapbox though.
    event.originalEvent.preventDefault();
   ...
}

There seems to be no sane way to override this. Even if you set options { touchEnabled: false } it will still suppress before checking this bool.

Now the question is why is the event suppressed. This only happens for touch. What breaks if this suppress is not done? Does the map handle its own touch events and it breaks the draw UI stuff? Could this suppress simply be removed?

Could the touch be only suppressed if there is a feature at the event pos? onst target = featuresAt.touch(event, null, ctx)[0]; could only suppress if (target) is hit.

Anyhow. I'm doing my own draw mode that does not use any of the UI or interactions. I just want to draw shapes from code (similar to the static example mode, but with some stuff on top). If touchEnabled : false is the, i believe also the onTap wont get called in my mode impl.

[jonnenauha]

One suggestion would be to move the preventDefault() below the options.touchEnabled checks in all three funcs. At least users that are doing their own draw impl. could set this to true. If they want click/tap/touch events, could get them from map.on(type) instead. It really should not suppress the events if the touch handling is disabled via options.

https://github.com/mapbox/mapbox-gl-draw/blob/master/src/events.js#L80-L123

Edit: Reading the comment, it seems the intention is to allow map to handle the events. Like the whole draw lib wont override "click" functionality. The question what goes wrong when letting the touches be unsuppressed. Why was this added to begin with? It would be enough if it would not be suppressed when it is a "tap" to allow map to get the "click" event.

Commit that added it is here https://github.com/mapbox/mapbox-gl-draw/commit/62607fd881d802c0a3a33f327663d3a8a620afbb by @ChrisBellew, any insights why the preventDefault was added there?

[medv]

Hit this in a big way today. Some clarification would be great. My use case is perfectly solved if the touch is not suppressed, which seemingly is the expected interaction.

[Plantain]

This is affecting me to. Would be great to have some direction on how this is intended to behave.

[Plantain]

Well, I issued a pull request with the suggested fix for this. Please try it and see if it solves your issues. I am testing on a few different devices.

[godismyjudge95]

Any update on this one? I also ran into this today. Took forever to figure out it was the gl draw plugin. Feel free to send me any solutions you have.

[benderlidze]

Hi. Any updates on this? Anything will be appreciated! Thanks

[godismyjudge95]

@benderlidze I posted a pull request, that fixed the issue for me and my clients.

[benderlidze]

Hi @godismyjudge95 Thank you! Can you send me a link to your builded mapbox-gl-draw version? For some reasons I can't build it on my pc, got an error with npm.

[godismyjudge95]

@benderlidze Here is a gist of it: https://gist.github.com/godismyjudge95/a4ea43263db53b90b05511c911cd0034

I had some trouble building it also. I after a bit of tinkering, I figured out that I had to change the dev dependencies and the dependencies to look like this:

  "devDependencies": {
    "@babel/core": "^7.1.2",
    "@babel/preset-env": "^7.1.0",
    "@babel/register": "^7.0.0",
    "@mapbox/mapbox-gl-draw-static-mode": "^1.0.1",
    "@turf/centroid": "^6.0.0",
    "babel-eslint": "^8.0.3",
    "babel-plugin-istanbul": "^5.1.0",
    "babelify": "^10.0.0",
    "browserify": "^16.1.1",
    "browserify-middleware": "^8.1.0",
    "envify": "^4.0.0",
    "eslint": "^4.2.0",
    "eslint-config-mourner": "^2.0.1",
    "express": "^4.13.4",
    "mapbox-gl": "0.51.0",
    "mapbox-gl-js-mock": "1.0.0",
    "mock-browser": "^0.92.10",
    "nyc": "^12.0.1",
    "opener": "^1.4.1",
    "sinon": "5.0.0",
    "synthetic-dom-events": "0.3.0",
    "tape": "^4.0.0",
    "uglify-js": "^3.0.22",
    "unassertify": "^2.0.3"
  },
  "peerDependencies": {
    "mapbox-gl": ">=0.27.0 <=0.51.0"
  },
  "dependencies": {
    "@mapbox/geojson-area": "^0.2.1",
    "@mapbox/geojson-extent": "^0.3.2",
    "@mapbox/geojson-normalize": "0.0.1",
    "@mapbox/geojsonhint": "^2.0.0",
    "@mapbox/point-geometry": "0.1.0",
    "gulp-sourcemaps": "^1.7.1",
    "hat": "0.0.3",
    "lodash.isequal": "^4.2.0",
    "xtend": "^4.0.1"
  }

Once I changed that, ran npm install, and made the patch in the events section it built/ran perfectly. Let me know if you need anything else.

[benderlidze]

@godismyjudge95 Thank you very much!

[radekdob]

Its 2020 October and the latest version contains that bug. Are you planning to fix it ? I dont want to stick to @godismyjudge95
solution beacuse it may be in some cases outdated.

[godismyjudge95]

Its 2020 October and the latest version contains that bug. Are you planning to fix it ? I dont want to stick to @godismyjudge95 solution beacuse it may be in some cases outdated.

I have submitted an updated pull request, so it is just waiting to be reviewed: https://github.com/mapbox/mapbox-gl-draw/pull/869