Enable touch event propagation properly

[godismyjudge95]

This is to fix #617 It allows the event to propagate through if there are no mapbox targets nearby. This sort of breaks the one tap move drawing functionality, but still allows for the double tap to move the drawing. It also enables popups and clusters to work again.

I believe this is the best temporary solution until another is found. The one provided by @Plantain here: https://github.com/mapbox/mapbox-gl-draw/pull/830 basically broke drawing completely on some browsers. I have tested extensively and am employing the patch to all my apps.

[rdgfuentes]

Thanks for proposing this fix! Take a look at the continuous integration check that failed, you have some trailing spaces that are not allowed by the linter:

/home/travis/build/mapbox/mapbox-gl-draw/src/events.js
   90:1  error  Trailing spaces not allowed  no-trailing-spaces
   95:1  error  Trailing spaces not allowed  no-trailing-spaces
   99:1  error  Trailing spaces not allowed  no-trailing-spaces
  120:1  error  Trailing spaces not allowed  no-trailing-spaces
  125:1  error  Trailing spaces not allowed  no-trailing-spaces
  127:1  error  Trailing spaces not allowed  no-trailing-spaces

[aptlin]

@godismyjudge95, could you please fix the trailing spaces for the PR to merge?

[godismyjudge95]

@sdll @rdgfuentes It appears my fix is interfering with tests 498-502... This is due to my original statement: "This sort of breaks the one tap move drawing functionality," however, this is a necessary trade off until we can figure out another solution.

To see a demo of the issue I am talking about go to this website: https://www.terraplat.com/ The drawing works perfectly until you try to resize it. In order to resize, you need to click once on the point to select it, and then click and drag to move the point. I realize this is less intuitive than clicking once and dragging, however, the client needed to have clusters and drawing and popups functioning... This patch was the only way we could get all 3 working together.

[kjkurtz]

Any further thoughts on how to keep this moving forward? Definitely causes issues with onClick events combined with Draw.

[godismyjudge95]

@rdgfuentes @sdll @kjkurtz @kkaefer

I added a condition to my event pass through change and all the tests should pass now. Let me know if there is anything else I need to do to get this merged.

[godismyjudge95]

@asheemmamoowala Sorry to bring you into this, but is there any way we can get this merged into the next version? This issue has been resolved for almost a year.

[avpeery]

Hi team @asheemmamoowala @rreusser @karimnaaji @arindam1993, checking in if is there a reason this PR has not been merged? Is there something preventing this change, or a different approach that would be preferred? Thanks!

[godismyjudge95]

Hi team @asheemmamoowala @rreusser @karimnaaji @arindam1993, checking in if is there a reason this PR has not been merged? Is there something preventing this change, or a different approach that would be preferred? Thanks!

@avpeery Any idea if this has been reviewed by anyone authorized to merge it?

[scheepers]

Hi Is there a solution to this? I am still getting this error and it's a blocker for the project I'm working on.

[godismyjudge95]

Hi Is there a solution to this? I am still getting this error and it's a blocker for the project I'm working on.

If you want the solution asap you could manually download my repo and include it in your project. My patch still has not been merged. :/

[deen13]

We're also waiting for this. :disappointed:

[chwallen]

@godismyjudge95 Thanks for the PR, we are using a work workaround based on this in our project. We cannot use yours directly as you check if (!target && currentModeName === 'simple_select') and assume simple_select is the default mode whereas we use a custom mode.

To make it more customizable, our version adds an additional item into the options object called passthroughTouchEventModeNames (like this which is a list (it can default to ['simple_select'] for compatibility reasons), like this:

const passthroughTouchEventModeNames = ctx.options.passthroughTouchEventModeNames || ["simple_select"];

Instead of if (!target && currentModeName === 'simple_select'), we check it using if (!target && passthroughTouchEventModeNames.some((name) => name === currentModeName)).

In case this will get merged, I would love if the mode(s) were customizable and not bound to simple_select. :smile:

[SnailBones]

@chwallen Would you be interested in submitting a PR from your fork?

[rhysstubbs]

This PR has been open for 2+ years and I've just encountered this issue myself... surely somebody can answer why this hasn't been resolved yet?

[kokapelli]

This PR has been open for 2+ years and I've just encountered this issue myself... surely somebody can answer why this hasn't been resolved yet?

Would also like to know. Not a great feeling moving away from our own drawing methods just to find out during final testing that you're forced to double the amount of map click/touch related code just to have a working MVP caused by such a minute issue.

[stepankuzmin]

Hi! I'm sorry for not getting back to you sooner. We've switched to passive event listeners instead of preventDefault here, and this should be fixed in the next release https://github.com/mapbox/mapbox-gl-draw/pull/1146