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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 1 commit into from
Apr 6, 2022
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 🐞
Expand Down
25 changes: 23 additions & 2 deletions ...c/main/kotlin/com/mapbox/maps/plugin/viewport/transition/DefaultViewportTransitionImpl.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -102,6 +122,7 @@ internal class DefaultViewportTransitionImpl(
}
return Cancelable {
isCancelableCalled = true
keepObserving = false
cancelAnimation()
cancelable.cancel()
}
Expand Down
6 changes: 5 additions & 1 deletion ...st/kotlin/com/mapbox/maps/plugin/viewport/transition/DefaultViewportTransitionImplTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) }
Expand Down Expand Up @@ -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 }
}
Expand Down