Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

[ios] Improve tilt gesture recognizer detection. #15349

Merged
merged 8 commits into from
Aug 24, 2019

Conversation

fabian-guerra
Copy link
Contributor

@fabian-guerra fabian-guerra commented Aug 10, 2019

Fixes #11982

@fabian-guerra fabian-guerra added iOS Mapbox Maps SDK for iOS needs changelog Indicates PR needs a changelog entry prior to merging. labels Aug 10, 2019
@fabian-guerra fabian-guerra added this to the release-queso milestone Aug 10, 2019
@fabian-guerra fabian-guerra requested review from friedbunny, julianrex and a team August 10, 2019 01:23
@fabian-guerra fabian-guerra self-assigned this Aug 10, 2019
@friedbunny
Copy link
Contributor

friedbunny commented Aug 12, 2019

  • How does this improve tilt gesture recognition?
  • Why did you take this approach?
  • We need to write tests to verify the new(?) behavior before this can be accepted.

@fabian-guerra
Copy link
Contributor Author

fabian-guerra commented Aug 12, 2019

@friedbunny:

How does this improve tilt gesture recognition?
According to your comment in #11982:

The two-finger-vertical-drag gesture that controls tilt is currently (as of ios-v4.0.x) too easy to trigger — I often find myself accidentally tilting the map, when I intended to pinch zoom/rotate.

This pr tries to avoid the accidental trigger.

Why did you take this approach?
I analyzed how the gestures are triggered when you zoom/rotate/tilt. Tilt is mistakenly considered as zoom or pitch because we didn't consider the verticality nature of the tilt gesture. This pr changes how we calculate this property of the gesture. I compared two gesture properties:

  1. The slope between your fingers to make sure only slopes close to 0º (horizontal position but the threshold is < 45º) are considered.
  2. The gesture movement over time, calculate a slope with the previous position, I'm considering absolute values and degrees above 60º accounts for what it could be considered a vertical movement.

We need to write tests to verify the new(?) behavior before this can be accepted.
I added an horizontal movement test.

@friedbunny
Copy link
Contributor

Tried this out with @fabian-guerra just now and the main issue we saw was that a two-finger sideways (e.g., 90°) drag will now trigger the tilt gesture, when it should instead be recognized by the pan gesture (as it was previously).

@fabian-guerra fabian-guerra force-pushed the fabian-gesture-11982 branch 2 times, most recently from bb35655 to f71efd5 Compare August 22, 2019 00:02
@fabian-guerra fabian-guerra removed needs changelog Indicates PR needs a changelog entry prior to merging. needs tests labels Aug 22, 2019
platform/ios/src/MGLMapView.mm Show resolved Hide resolved
platform/ios/src/MGLMapView.mm Outdated Show resolved Hide resolved
platform/ios/src/MGLMapView.mm Outdated Show resolved Hide resolved
platform/ios/src/MGLMapView.mm Outdated Show resolved Hide resolved
platform/ios/src/MGLMapView.mm Outdated Show resolved Hide resolved
platform/ios/src/MGLMapView.mm Outdated Show resolved Hide resolved
platform/ios/CHANGELOG.md Outdated Show resolved Hide resolved
platform/ios/src/MGLMapView.mm Show resolved Hide resolved
Copy link
Contributor

@friedbunny friedbunny left a comment

Choose a reason for hiding this comment

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

Thanks, looking forward to playing with this more!

platform/ios/src/MGLMapView.mm Show resolved Hide resolved
@fabian-guerra fabian-guerra merged commit 5c066f8 into master Aug 24, 2019
@fabian-guerra fabian-guerra deleted the fabian-gesture-11982 branch August 24, 2019 00:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
iOS Mapbox Maps SDK for iOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tilt gesture is too easy to trigger
3 participants