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

Bump gestures library to v0.5.0, refine thresholds and velocity animations #15136

Merged
merged 5 commits into from
Aug 15, 2019

Conversation

LukasPaczos
Copy link
Contributor

Closes #14776. Closes #14775.

  • Bumps gestures library to v0.5.0, which includes our custom scale gestures detector, dropping the dependency on the compat one and bringing minor improvements, see Accessing hidden field violation - ScaleGestureDetector mapbox-gestures-android#39 and Introduce a custom scale gesture detector implementation mapbox-gestures-android#73.
  • Disables the rotation gesture detector if a scale gesture has been recognized first to prevent unwanted rotation. This behavior is adjustable through the UiSettings.
  • Improves the responsiveness of scale and rotation gestures, while at the same time better separates them and judges which gesture should be executed first based on the relative gesture speed (linear or angular speed respectively).
  • Improves scale and rotation velocity animations by calculating the value addition linearly.
  • Decreases a chance of firing both scale and rotation velocity animations at the same time by using a [last gesture delta change] / [velocityXY] ratio.
  • Adjusts value addition and required gesture's speed ratio for the screen's pixel density.

I've paid special attention to interactions that occur when the device is used with one hand only, with fingers placed sideways (a tribute to users with long nails 💅) or when the user has to reach to the device that is further away. Those scenarios should be dramatically improved in comparison to the behavior on master, even when a phone-on-the-table developer scenario should remain the same or might even require more willingness to register gestures like a rotation with high velocity.

I'd love any additional 👀 and feedback from @mapbox/android or anyone able to quickly build the Maps SDK. Most of the magic numbers in the PR have been meticulously chosen to offer the best experience on the devices I could test on locally.

Currently, the PR is built against the gesture's library snapshot. When the feedback is addressed and the PR is approved, I'm going to release the stable v0.5.0 in order to avoid patch releases if anything comes up that would need to be adjusted on the gesture library's end.

I didn't include any additional tests as most of the proposed changes are subjective and we don't have a mechanism to test them.

@LukasPaczos LukasPaczos added the Android Mapbox Maps SDK for Android label Jul 17, 2019
@LukasPaczos LukasPaczos requested a review from tobrun July 17, 2019 11:37
Copy link
Member

@tobrun tobrun left a comment

Choose a reason for hiding this comment

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

Validated changes on my end and looks good. To make CI pass you need to update sub modules and rerun license generation. Would be great if someone else from @mapbox/maps-android could test these changes (or anyone outside of Mapbox).

@langsmith
Copy link
Contributor

Installed the test app on this branch and can confirm that changes look good. Map doesn't errantly rotate when trying to tilt the map.

@tobrun
Copy link
Member

tobrun commented Jul 22, 2019

@chloekraw would you also be able to do some testing on your end? @langsmith should be able to set you up with some test devices/apps.

@chloekraw
Copy link
Contributor

@tobrun yep, would be happy to! @langsmith I'll find you tomorrow afternoon.

@langsmith
Copy link
Contributor

I've given @chloekraw a Pixel 2. Installed this branch's version of the Maps SDK test app on it. Also put the master version of the Mapbox Android demo app on it, which is using 8.1.0 of the Maps SDK.

@chloekraw
Copy link
Contributor

I tested this PR on a Pixel 3 and also asked my friend who is an Android user to test. I am normally an iPhone user.

  • While I use two fingers on one hand to make two-finger gestures on the map, my friend uses both thumbs for those gestures (zoom, rotate).
  • My friend did a blind test and did not know which one contained the PR.

Overall, we found the map with this PR to be much better! Great work @LukasPaczos.

  • Pitch detection was much better. It was pretty hard to trigger pitch before and the animation was very jerky. The pitch animation with this PR is quite smooth. Also, in the old version, once a pitch gesture was recognized, it was more likely to incorrectly register subsequent gestures as pitch as well.
  • Zoom was great -- much smoother than before. I think we fixed unwanted rotation during zoom changes. I didn't notice any problems, but my friend sometimes does small zooms by holding his thumbs 45 degrees from the edge of the device and making a small jerky motion in that direction. The map did not always register this gesture.
  • Rotation was better. The animation used to be jerky and now it's pretty smooth. My friend noticed that rotating the map sometimes triggered unexpected and unwanted zoom changes when the rotation change. So I think this is the converse of the problem we fixed (unwanted rotation during zoom gesture).
  • My friend also expected that if he flicked the map, the camera would coast more in proportion to how much pressure he applied to the flick. If you try this out on Google Maps, you'll see that the camera travels much more than on our maps.

@LukasPaczos if any small/easy adjustments come to mind that can address any of this feedback, it could be worth adding into this PR, but I'd be perfectly happy merging as is and cutting new tickets for each piece of feedback.

cc @mapbox/maps-android

cc @mapbox/maps-ios in case this is also helpful as we're thinking about gestures improvements

@chloekraw chloekraw self-requested a review August 14, 2019 06:53
@LukasPaczos LukasPaczos added this to the release-queso milestone Aug 14, 2019
@LukasPaczos LukasPaczos force-pushed the lp-gestures-0.5.0 branch 3 times, most recently from 26db139 to 41e966d Compare August 15, 2019 13:52
@LukasPaczos
Copy link
Contributor Author

@chloekraw thank you very much for the amazing feedback!

My friend noticed that rotating the map sometimes triggered unexpected and unwanted zoom changes when the rotation change.

9fd4d4e adds an additional threshold that makes this experience a bit better.

but my friend sometimes does small zooms by holding his thumbs 45 degrees from the edge of the device and making a small jerky motion in that direction. The map did not always register this gesture.

I'm not exactly sure how to replicate that.

My friend also expected that if he flicked the map, the camera would coast more in proportion to how much pressure he applied to the flick.

This will be targeted as part of https://github.com/mapbox/mapbox-gl-native/issues/15110.

Thanks again everyone for the feedback!

@LukasPaczos LukasPaczos merged commit eab3fbf into master Aug 15, 2019
@LukasPaczos LukasPaczos deleted the lp-gestures-0.5.0 branch August 15, 2019 15:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Android Mapbox Maps SDK for Android
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rotation is too easy to trigger Velocity animations are twitchy
4 participants