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

fix double tap zoom on iOS 13 #9757

Merged
merged 1 commit into from
Jun 8, 2020
Merged

fix double tap zoom on iOS 13 #9757

merged 1 commit into from
Jun 8, 2020

Conversation

ansis
Copy link
Contributor

@ansis ansis commented Jun 3, 2020

fix #9756

iOS 13 does not fire the second touchstart/end events in a double tap if the touchstart listener is non-passive. This fix makes it passive.

Calling preventDefault() allows the second event to be fired but suppresses other events like click.

touchmove needs to remain non-passive so that it can be used to prevent touches from scrolling or scaling the page on some versions of iOS Safari.


This fix does NOT work if a parent of the canvas (including the window/document/body) has a non-passive touchstart listener that doesn't call preventDefault(). I don't know of a fix for this that doesn't suppress all click dom events as well.


Launch Checklist

  • briefly describe the changes in this PR
  • write tests for all new functionality
  • manually test the debug page
  • apply changelog label ('bug', 'feature', 'docs', etc) or use the label 'skip changelog'
  • add an entry inside this element for inclusion in the mapbox-gl-js changelog: <changelog>Fix zooming with a double tap on iOS SafarI 13.</changelog>

I have done initial testing on iOS safari v12 and v13 but these needs to be tested more thoroughly on other touch devices.

fix #9756

iOS 13 does not fire the second touchstart/end events in a double tap if
the touchstart listener is non-passive. This fix makes it passive.

Calling preventDefault() allows the second event to be fired but
suppresses other events like `click`.

`touchmove` needs to remain non-passive so that it can be used to
prevent touches from scrolling or scaling the page on some versions of
iOS Safari.
@ansis ansis requested a review from ryanhamley June 3, 2020 22:29
@ansis
Copy link
Contributor Author

ansis commented Jun 3, 2020

Note: changing the listener to passive means that this won't prevent a click event from being fired anymore:

map.on('touchstart', e => e.originalEvent.preventDefault());

This might be considered a breaking change but:

  • it was never explicitly supported
  • it hasn't always been supported

Copy link
Contributor

@ryanhamley ryanhamley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this in iOS 13 Chrome and Safari and both worked as expected while 1.10.1 did not. I'd approve this but I think the fact that map.on('touchstart', e => e.originalEvent.preventDefault()); no longer works may deserve some discussion. It's not officially supported but preventing the original event seems to be a fairly common use case and we do point it out as a possible solution. Vlad did so on a ticket today #9755 (comment) albeit for touchmove. It also seems weird that this workaround would be available for every event except for touchstart.

@ansis
Copy link
Contributor Author

ansis commented Jun 8, 2020

I'd approve this but I think the fact that map.on('touchstart', e => e.originalEvent.preventDefault()); no longer works may deserve some discussion. It's not officially supported but preventing the original event seems to be a fairly common use case and we do point it out as a possible solution. Vlad did so on a ticket today #9755 (comment) albeit for touchmove. It also seems weird that this workaround would be available for every event except for touchstart.

The linked example uses stopPropagation() which will continue working the same way. The only thing preventDefault() on touchstart does in this case is prevent the relevant mouse events from firing. While someone may be relying on this behavior I think it's fairly unlikely.

I see no other way of working around this browser bug

@ansis ansis merged commit 25cc7a9 into master Jun 8, 2020
@ansis ansis deleted the fix-ios-13-double-tap-9756 branch June 8, 2020 20:34
ansis added a commit that referenced this pull request Jun 8, 2020
fix #9756

iOS 13 does not fire the second touchstart/end events in a double tap if
the touchstart listener is non-passive. This fix makes it passive.

Calling preventDefault() allows the second event to be fired but
suppresses other events like `click`.

`touchmove` needs to remain non-passive so that it can be used to
prevent touches from scrolling or scaling the page on some versions of
iOS Safari.
ansis added a commit that referenced this pull request Jun 8, 2020
fix #9756

iOS 13 does not fire the second touchstart/end events in a double tap if
the touchstart listener is non-passive. This fix makes it passive.

Calling preventDefault() allows the second event to be fired but
suppresses other events like `click`.

`touchmove` needs to remain non-passive so that it can be used to
prevent touches from scrolling or scaling the page on some versions of
iOS Safari.
sgolbabaei added a commit that referenced this pull request Jun 9, 2020
sgolbabaei added a commit that referenced this pull request Jun 10, 2020
* Cherry pick (#9757) to release-erie

* Cherry pick (#9753) to release-erie

* Cherry pick (#9749) to release-erie

* Cherry pick (#9748) to release-erie
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.

double tap to zoom in does not work on iOS 13
2 participants