[viewport] Fix camera jump when default transition is canceled.

mapbox · Apr 6, 2022 · 963a260 · 963a260
1 parent 274236e
commit 963a260
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 4 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,7 +10,7 @@ Mapbox welcomes participation and contributions from everyone.
 * Add `LocationIndicatorLayer.bearingTransition` API to control transition of bearing property. ([1207](https://github.com/mapbox/mapbox-maps-android/pull/1207))
 * Add `MapboxConcurrentGeometryModificationException` with detailed information instead of `ConcurrentModificationException` that is thrown when GeoJson data is mutated. ([1248](https://github.com/mapbox/mapbox-maps-android/pull/1248))
 * Introduce `line-trim-offset` property for LineLayer. ([1252](https://github.com/mapbox/mapbox-maps-android/pull/1252))
-* Deprecate `FollowPuckViewportStateOptions.animationDurationMs`, the initial transition will be handled properly by the Viewport plugin internally. ([1256](https://github.com/mapbox/mapbox-maps-android/pull/1256), [1261](https://github.com/mapbox/mapbox-maps-android/pull/1261))
+* Deprecate `FollowPuckViewportStateOptions.animationDurationMs`, the initial transition will be handled properly by the Viewport plugin internally. ([1256](https://github.com/mapbox/mapbox-maps-android/pull/1256), [1261](https://github.com/mapbox/mapbox-maps-android/pull/1261), [1262](https://github.com/mapbox/mapbox-maps-android/pull/1262))
 * Mark `MapView#viewAnnotationManager` as non-experimental meaning View Annotation API will not have breaking changes in upcoming minor releases. ([1260](https://github.com/mapbox/mapbox-maps-android/pull/1260))
 
 ## Bug fixes 🐞

diff --git a/...c/main/kotlin/com/mapbox/maps/plugin/viewport/transition/DefaultViewportTransitionImpl.kt b/...c/main/kotlin/com/mapbox/maps/plugin/viewport/transition/DefaultViewportTransitionImpl.kt
@@ -3,7 +3,6 @@ package com.mapbox.maps.plugin.viewport.transition
 import android.animation.Animator
 import android.animation.AnimatorSet
 import android.animation.ValueAnimator
-import androidx.core.animation.doOnEnd
 import com.mapbox.maps.CameraOptions
 import com.mapbox.maps.CameraState
 import com.mapbox.maps.ScreenCoordinate
@@ -93,7 +92,28 @@ internal class DefaultViewportTransitionImpl(
           }
         initialAnimatorSet.childAnimations.forEach {
           val cameraAnimator = it as CameraAnimator<*>
-          cameraAnimator.doOnEnd { completedChildAnimators.add(cameraAnimator.type) }
+          cameraAnimator.addListener(
+            object : Animator.AnimatorListener {
+              private var isCanceled = false
+              override fun onAnimationStart(p0: Animator?) {
+                // no-ops
+              }
+
+              override fun onAnimationEnd(p0: Animator?) {
+                if (!isCanceled) {
+                  completedChildAnimators.add(cameraAnimator.type)
+                }
+              }
+
+              override fun onAnimationCancel(p0: Animator?) {
+                isCanceled = true
+              }
+
+              override fun onAnimationRepeat(p0: Animator?) {
+                // no-ops
+              }
+            }
+          )
         }
         startAnimation(initialAnimatorSet, instant = false)
         animatorSet = initialAnimatorSet
@@ -102,6 +122,7 @@ internal class DefaultViewportTransitionImpl(
     }
     return Cancelable {
       isCancelableCalled = true
+      keepObserving = false
       cancelAnimation()
       cancelable.cancel()
     }

diff --git a/...st/kotlin/com/mapbox/maps/plugin/viewport/transition/DefaultViewportTransitionImplTest.kt b/...st/kotlin/com/mapbox/maps/plugin/viewport/transition/DefaultViewportTransitionImplTest.kt
@@ -120,12 +120,13 @@ class DefaultViewportTransitionImplTest {
     animatorListenerSlot.captured.onAnimationEnd(animator)
     assertTrue(dataObserverSlot.captured.onNewData(cameraOptions))
     verify(exactly = 1) { animator.setObjectValues(0.0, 0.0) }
-    verify { mapCameraManagerDelegate.setCamera(cameraOptions { bearing(cameraOptions.bearing) }) }
+    verify(exactly = 1) { mapCameraManagerDelegate.setCamera(cameraOptions { bearing(cameraOptions.bearing) }) }
 
     // try to notify if animation is finished successfully
     animatorSetListenerSlot.captured.onAnimationEnd(mockk())
     // should stop observing data source and return false
     assertFalse(dataObserverSlot.captured.onNewData(cameraOptions))
+    verify(exactly = 2) { mapCameraManagerDelegate.setCamera(cameraOptions { bearing(cameraOptions.bearing) }) }
     // verify cleanup
     verify { cameraPlugin.unregisterAnimators(animator) }
     verify { completionListener.onComplete(true) }
@@ -160,8 +161,11 @@ class DefaultViewportTransitionImplTest {
     verify { cancelable.cancel() }
     animatorSetListenerSlot.captured.onAnimationCancel(animator)
     animatorSetListenerSlot.captured.onAnimationEnd(animator)
+    animatorListenerSlot.captured.onAnimationCancel(animator)
+    animatorListenerSlot.captured.onAnimationEnd(animator)
     // should stop observing data source and return false
     assertFalse(dataObserverSlot.captured.onNewData(cameraOptions))
+    verify(exactly = 0) { mapCameraManagerDelegate.setCamera(cameraOptions { bearing(cameraOptions.bearing) }) }
     verify { cameraPlugin.unregisterAnimators(animator) }
     verify { cameraPlugin.anchor = cachedAnchor }
   }