Prevent animated values from leaving expected bounds

blend-library/src/main/java/com/wealthfront/blend/Blend.kt

@@ -83,7 +83,7 @@ open class Blend {
    */
   open fun stopPulsing(vararg views: View) {
     views
-        .flatMap { view -> AdditiveViewProperties.ALPHA.getAnimationData(view).runningAnimations }
+        .flatMap { view -> AdditiveViewProperties.ALPHA.getAnimationData(view).queuedAnimations }
         .mapNotNull { singlePropertyAnimation -> singlePropertyAnimation.animator }
         .filter { animator -> animator.repeatCount != 0 }
         .toSet()

blend-library/src/main/java/com/wealthfront/blend/animator/BlendableAnimator.kt

@@ -21,7 +21,7 @@ import java.util.ArrayList
  * It wraps a [ValueAnimator] ([innerAnimator]) and delegates all [Animator] methods to it, with some redirection to
  * ensure proper adherence to e.g. listener contracts.
  *
- * For proper functioning in sets, you must call [commitFutureValuesIfNotCommitted] when the set of animations is
+ * For proper functioning in sets, you must call [queueAnimationsIfNotAlreadyQueued] when the set of animations is
  * started, even if this animation is delayed. If not, the end state of the set of animations becomes dependent on
  * when the individual animators start, which can be unpredictable when they're user-initiated.
  */
@@ -43,7 +43,7 @@ open class BlendableAnimator : Animator() {
    * The actions to run before this animator starts.
    */
   @VisibleForTesting val beforeStartActions: MutableList<() -> Unit> = mutableListOf()
-  private var valuesCommitted = false
+  private var animationsQueued = false
 
   val allAnimations: List<SinglePropertyAnimation<*>> get() = animations.toList()
 
@@ -96,17 +96,27 @@ open class BlendableAnimator : Animator() {
   }
 
   /**
-   * Commit the future values of all [animations] to the [com.wealthfront.blend.properties.AnimationData] objects
-   * described by their properties.
+   * Queue [animations], adding their future values to the [com.wealthfront.blend.properties.AnimationData] associated
+   * with each property/subject pair.
    *
-   * These future values are necessary for any animations started after this animator to blend properly.
+   * This should be called when the animation set that contains this animator is started, so animations started after
+   * this one can properly blend with this one.
    */
-  open fun commitFutureValuesIfNotCommitted() {
-    if (!valuesCommitted) {
+  open fun queueAnimationsIfNotAlreadyQueued() {
+    if (!animationsQueued) {
       beforeStartActions.forEach { it() }
-      animations.forEach { it.setUpOnAnimationStart(this) }
+      animations.forEach { it.setUpOnAnimationQueued(this) }
     }
-    valuesCommitted = true
+    animationsQueued = true
+  }
+
+  /**
+   * Mark all [animations] as part of a fully-queued set of animations.
+   *
+   * This step is necessary for individual animations in a set to not remove each other when being queued.
+   */
+  open fun markAnimationsAsFullyQueued() {
+    animations.forEach { it.isPartOfAFullyQueuedSet = true }
   }
 
   override fun start() {
@@ -120,16 +130,16 @@ open class BlendableAnimator : Animator() {
     innerAnimator.addListener(object : AnimatorListenerAdapter() {
       override fun onAnimationEnd(animation: Animator?) {
         animations.forEach { it.runEndActions() }
-        valuesCommitted = false
+        animationsQueued = false
       }
     })
 
-    commitFutureValuesIfNotCommitted()
     if (repeatCount > 0) {
       cancelAnimatorOnViewsDetached()
     }
+    queueAnimationsIfNotAlreadyQueued()
     innerAnimator.start()
-    super.start()
+    markAnimationsAsFullyQueued()
   }
 
   private fun cancelAnimatorOnViewsDetached() {

blend-library/src/main/java/com/wealthfront/blend/animator/SinglePropertyAnimation.kt

@@ -1,5 +1,6 @@
 package com.wealthfront.blend.animator
 
+import androidx.annotation.VisibleForTesting
 import com.wealthfront.blend.properties.AdditiveProperty
 
 /**
@@ -29,17 +30,26 @@ class SinglePropertyAnimation<Subject>(
    * The animator running this animation
    */
   var animator: BlendableAnimator? = null
-    private set
+    @VisibleForTesting internal set
+  /**
+   * Whether this animation has started running. Note that it can be queued and not started.
+   */
+  val isStarted: Boolean get() = animator?.isStarted ?: false
+  /**
+   * Whether the set that this animation belongs to is done queuing all of its animations. Useful for figuring out
+   * when to cancel unstarted (but queued) animations.
+   */
+  var isPartOfAFullyQueuedSet: Boolean = false
 
   /**
-   * Set up the starting values for this animation and commit the [targetValue] as [property]'s future value.
+   * Set up the starting values for this animation and queue the [targetValue] as [property]'s future value.
    */
-  fun setUpOnAnimationStart(animator: BlendableAnimator) {
+  fun setUpOnAnimationQueued(animator: BlendableAnimator) {
     this.animator = animator
-    property.setUpOnAnimationStart(subject)
+    property.setUpOnAnimationQueued(subject)
     startValue = property.getFutureValue(subject)
     previousValue = startValue
-    property.addRunningAnimation(subject, this)
+    property.addQueuedAnimation(subject, this)
     property.addInterruptableEndActions(subject, *interruptibleEndActions.toTypedArray())
   }
 
@@ -52,12 +62,17 @@ class SinglePropertyAnimation<Subject>(
     previousValue = newValue
   }
 
+  fun markCancelled() {
+    startValue = targetValue
+    previousValue = targetValue
+  }
+
   /**
    * Run the end actions added by [BlendableAnimator.doOnFinishedUnlessLastAnimationInterrupted], if we haven't been
    * interrupted by another animation on [subject].[property].
    */
   fun runEndActions() {
     property.runEndActions(subject, this)
-    property.removeRunningAnimator(subject, this)
+    property.removeQueuedAnimation(subject, this)
   }
 }

blend-library/src/main/java/com/wealthfront/blend/dsl/BuilderTraits.kt

@@ -35,9 +35,9 @@ interface AnimationStarter {
     return animatorSet.asAnimator().apply {
       addListener(object : AnimatorListenerAdapter() {
         override fun onAnimationStart(animation: Animator?) {
-          animatorSet.childAnimators
-            .mapNotNull { it as? BlendableAnimator }
-            .forEach { it.commitFutureValuesIfNotCommitted() }
+          val animators = animatorSet.childAnimators.mapNotNull { it as? BlendableAnimator }
+          animators.forEach { it.queueAnimationsIfNotAlreadyQueued() }
+          animators.forEach { it.markAnimationsAsFullyQueued() }
         }
       })
     }

blend-library/src/main/java/com/wealthfront/blend/properties/AdditiveProperty.kt

@@ -10,8 +10,8 @@ import kotlin.math.roundToInt
  * To properly blend animations, it includes methods like [getFutureValue], [addInterruptableEndActions], and
  * [getAnimationData].
  *
- * We store data on running animations ([AnimationData]) on each subject, ideally. This systematically prevents leaks of
- * references to [Subject]s. The mechanism for storing is described by the implementation of [getAnimationData]
+ * We store data on queued animations ([AnimationData]) on each subject, ideally. This systematically prevents leaks
+ * of references to [Subject]s. The mechanism for storing is described by the implementation of [getAnimationData]
  */
 interface AdditiveProperty<in Subject> {
 
@@ -52,19 +52,19 @@ interface AdditiveProperty<in Subject> {
   /**
    * Perform any setup tasks when an animation on this property starts.
    */
-  fun setUpOnAnimationStart(subject: Subject) { }
+  fun setUpOnAnimationQueued(subject: Subject) { }
 
   /**
-   * Add a running animation to the [subject]'s [AnimationData] for this property.
+   * Add a queued animation to the [subject]'s [AnimationData] for this property.
    */
-  fun addRunningAnimation(subject: Subject, animation: SinglePropertyAnimation<*>) =
-      getAnimationData(subject).addRunningAnimation(animation)
+  fun addQueuedAnimation(subject: Subject, animation: SinglePropertyAnimation<*>) =
+      getAnimationData(subject).addQueuedAnimation(animation)
 
   /**
-   * Remove a running animation to the [subject]'s [AnimationData] for this property.
+   * Remove a queued animation from the [subject]'s [AnimationData] for this property.
    */
-  fun removeRunningAnimator(subject: Subject, animation: SinglePropertyAnimation<*>) =
-    getAnimationData(subject).removeRunningAnimation(animation)
+  fun removeQueuedAnimation(subject: Subject, animation: SinglePropertyAnimation<*>) =
+      getAnimationData(subject).removeQueuedAnimation(animation)
 
   /**
    * Add [endActions] to the [subject]'s [AnimationData] for this property so they can be cancelled if another
@@ -79,7 +79,7 @@ interface AdditiveProperty<in Subject> {
    */
   fun runEndActions(subject: Subject, animation: SinglePropertyAnimation<*>) {
     val animationData = getAnimationData(subject)
-    if (animation == animationData.runningAnimations.lastOrNull()) {
+    if (animation == animationData.queuedAnimations.lastOrNull()) {
       animationData.interruptableEndActions.forEach { it() }
       animationData.interruptableEndActions.clear()
     }

blend-library/src/main/java/com/wealthfront/blend/properties/AnimationData.kt

@@ -4,52 +4,62 @@ import com.wealthfront.blend.animator.SinglePropertyAnimation
 import kotlin.math.roundToInt
 
 /**
- * A container for storing data about running animations to facilitate blending.
+ * A container for storing data about queued animations to facilitate blending.
  *
  * Associated with a specific subject and a property.
  */
 class AnimationData {
 
   /**
-   * All animations currently running on the associated subject for the associated property.
+   * All animations currently queued to run on the associated subject for the associated property. These animations
+   * may or may not be currently running, however.
    */
-  val runningAnimations: MutableList<SinglePropertyAnimation<*>> = mutableListOf()
+  val queuedAnimations: MutableList<SinglePropertyAnimation<*>> = mutableListOf()
   /**
    * End actions that will be cancelled if another animation is started for the associated subject.property.
    */
   val interruptableEndActions: MutableList<() -> Unit> = mutableListOf()
   var isLatestAnimationDone = false
 
   /**
-   * Get the value of this property as if all running animations have finished.
+   * Get the value of this property as if all queued animations have finished.
    */
   val futureValue: Float?
-    get() = runningAnimations.lastOrNull()?.targetValue
+    get() = queuedAnimations.lastOrNull { it.isStarted }?.targetValue
 
   /**
-   * Register a new animation that has started.
+   * Register a new animation that has started. This cancels any [interruptableEndActions] that we have, since a new
+   * animation has started.
    *
-   * This also cancels any [interruptableEndActions] that we have, since a new animation has started.
+   * This also cancels all non-started [queuedAnimations], in order to avoid animating to any unexpected values.
+   * See [this github issue](https://github.com/wealthfront/blend/issues/4) for more information.
    */
-  fun addRunningAnimation(animation: SinglePropertyAnimation<*>) {
+  fun addQueuedAnimation(animation: SinglePropertyAnimation<*>) {
     if (isLatestAnimationDone) {
-      runningAnimations -= runningAnimations.last()
+      queuedAnimations -= queuedAnimations.last()
     }
     isLatestAnimationDone = false
-    runningAnimations += animation
+    queuedAnimations.filter {
+      !it.isStarted && it.isPartOfAFullyQueuedSet
+    }.forEach {
+      it.markCancelled()
+      queuedAnimations.remove(it)
+    }
+    queuedAnimations += animation
     interruptableEndActions.clear()
   }
 
   /**
-   * Remove a running animation because it has either finished or been cancelled.
+   * Remove a queued animation because it has either finished or been cancelled.
    */
-  fun removeRunningAnimation(animation: SinglePropertyAnimation<*>) {
-    if (runningAnimations.lastOrNull() == animation && runningAnimations.size > 1) {
+  fun removeQueuedAnimation(animation: SinglePropertyAnimation<*>) {
+    animation.markCancelled()
+    if (queuedAnimations.lastOrNull() == animation && queuedAnimations.size > 1) {
       isLatestAnimationDone = true
     } else {
-      runningAnimations -= animation
-      if (isLatestAnimationDone && runningAnimations.size == 1) {
-        runningAnimations.clear()
+      queuedAnimations -= animation
+      if (isLatestAnimationDone && queuedAnimations.size == 1) {
+        queuedAnimations.clear()
         isLatestAnimationDone = false
       }
     }

blend-library/src/main/java/com/wealthfront/blend/properties/DimensionViewProperties.kt

@@ -35,7 +35,7 @@ object DimensionViewProperties {
 
     override fun getAnimationData(subject: View): AnimationData = subject.getAnimationData(id)
 
-    override fun setUpOnAnimationStart(subject: View) {
+    override fun setUpOnAnimationQueued(subject: View) {
       if (subject.visibility == GONE) {
         setValue(subject, 0f)
         subject.visibility = VISIBLE
@@ -67,7 +67,7 @@ object DimensionViewProperties {
 
     override fun getAnimationData(subject: View): AnimationData = subject.getAnimationData(id)
 
-    override fun setUpOnAnimationStart(subject: View) {
+    override fun setUpOnAnimationQueued(subject: View) {
       if (subject.visibility == GONE) {
         setValue(subject, 0f)
         subject.visibility = VISIBLE

blend-library/src/main/java/com/wealthfront/blend/properties/ViewPropertySupport.kt

@@ -44,7 +44,7 @@ fun <Subject : Any> makeExternalAdditiveViewProperty(
           interpolateAction(startValue, endValue, timeFraction, subject)
         }
 
-    override fun setUpOnAnimationStart(subject: Subject) {
+    override fun setUpOnAnimationQueued(subject: Subject) {
       setUpAction?.invoke(subject)
     }
   }
@@ -101,7 +101,7 @@ internal fun <Subject : View> makeAdditiveViewProperty(
         interpolateAction(startValue, endValue, timeFraction, subject)
       }
 
-    override fun setUpOnAnimationStart(subject: Subject) {
+    override fun setUpOnAnimationQueued(subject: Subject) {
       setUpAction?.invoke(subject)
     }
   }
@@ -159,7 +159,7 @@ internal fun wrapViewProperty(
 
     override fun getAnimationData(subject: View): AnimationData = subject.getAnimationData(id)
 
-    override fun setUpOnAnimationStart(subject: View) {
+    override fun setUpOnAnimationQueued(subject: View) {
       setUpAction?.invoke(subject)
     }
   }

blend-library/src/test/java/com/wealthfront/TestExtensions.kt

@@ -0,0 +1,39 @@
+@file:Suppress("UNCHECKED_CAST")
+
+package com.wealthfront
+
+import org.mockito.ArgumentCaptor
+import org.mockito.Mockito
+import org.mockito.Mockito.`when`
+import org.mockito.stubbing.OngoingStubbing
+import org.mockito.stubbing.Stubber
+
+internal fun <T> whenever(x: T): OngoingStubbing<T> = `when`(x)
+
+internal fun <T> Stubber.whenever(x: T): T = `when`(x)
+
+// Casting to avoid a kotlin-NPE ( See : https://medium.com/@elye.project/befriending-kotlin-and-mockito-1c2e7b0ef791 )
+internal fun <T> any(): T {
+  Mockito.any<T>()
+  return null as T
+}
+
+internal fun <T> eq(value: T): T {
+  Mockito.eq(value)
+  return value
+}
+
+internal fun <T> isA(value: Class<T>): T {
+  Mockito.isA(value)
+  return null as T
+}
+
+internal fun <T, R : T> argThat(argumentMatcher: (R) -> Boolean): T {
+  Mockito.argThat { arg: R -> argumentMatcher(arg) }
+  return null as T
+}
+
+internal fun <T> ArgumentCaptor<T?>.captureNotNull(): T {
+  this.capture()
+  return null as T
+}