Lottie-android: RecycleView, wasAnimatingWhenDetached isn't set to false when view is attached

[YuriiTsap]

We have a related issue created few hours ago.
But the main issue isn't highlighted/described enough inside of the ticket.

_Let's start:_
I'm using LottieAnimationView.java inside RecyclerView.java. In my code every third item has an animation running, I'm operating with a dataset containing 60 items.
When LottieAnimationView.java is playing an animation and user starts scrolling we receive a onDetachedFromWindow() (_in case if view scroll offset is big enough_).

@Override 
protected void onDetachedFromWindow() {
    if (isAnimating()) {
      cancelAnimation();
      wasAnimatingWhenDetached = true;
    }
    super.onDetachedFromWindow();
  }

Here we are setting wasAnimatingWhenDetached to true. Afterwards, when user scrolls again and returns to the same item - onAttachedToWindow() callback is called:

@Override protected void onAttachedToWindow() {
    super.onAttachedToWindow();
    if (autoPlay || wasAnimatingWhenDetached) {
      playAnimation();
      // Autoplay from xml should only apply once.
      autoPlay = false;
    }
    if (Build.VERSION.SDK_INT < Build.VERSION_CODES.M) {
      // This is needed to mimic newer platform behavior.
      // https://stackoverflow.com/a/53625860/715633
      onVisibilityChanged(this, getVisibility());
    }
  }

In this callback we are relying on wasAnimatingWhenDetached and in our case we will play animation which wasn't finished. Until now everything was pretty ok, we are restarting animation which wasn't finished - pretty logical. But we aren't setting wasAnimatingWhenDetached flag to false - and that has some side effects.
In example we scroll back and forth, basically just returning to the same item - and it will again go through onDetachedFromWindow() && onAttachedToWindow()callbacks. And when the onAttachedToWindow() callback will be triggered we will re-run the animation again, evenif it was finished, which isn't the desired behavior(_at least as I see it_). That will happen because wasAnimatingWhenDetached isn't set to false in onAttachedToWindow() callback. Here is an example how it works in real-life(almost) app. Video is attached as link on google drive as Github doesn't supports mp4.
So, as you can see I have the bells animation which should be played only once. At first I want to show normal/desired flow:

• User scrolls to an item with animation
• Animation plays till it is finished
• User scrolls back and forth but animation doesn't restarts

Afterwards I wanna show wrong flow:

• User scrolls to an item with animation
• Animation starts
• User starts scrolling and animation is cancelled
• User scrolls back to that animation item and animation re-runs and is finished. We end up in finished animation state.
• Every next time user scrolls back and forth - animation re-runs. That is not desired behavior(IMHO).

I've cloned lottie project and imported it as module to mine project. I have an idea how that could be fixed(if thats a bug of course). We can add following line to onAttachedToWindow() callback:

@Override 
protected void onAttachedToWindow() {
    super.onAttachedToWindow();
    if (autoPlay || wasAnimatingWhenDetached) {
      playAnimation();
      // Autoplay from xml should only apply once.
      autoPlay = false;
     // this one line could be used to reset our flag
      wasAnimatingWhenDetached = false;
    }
    if (Build.VERSION.SDK_INT < Build.VERSION_CODES.M) {
      // This is needed to mimic newer platform behavior.
      // https://stackoverflow.com/a/53625860/715633
      onVisibilityChanged(this, getVisibility());
    }
  }

I haven't analyzed other cases, where this fix could create issues. But I'm almost sure other cases do not exist. Anyway it would be great to hear feedback or explanation on that.
Animation JSON file attached:
ringing_bell.json.zip

[gpeal]

@YuriiTsap It would be awesome if you are able to create a failing test case in FragmentVisibilityTests and also fix it! The visibility cases are very tricky and I've tried to get good espresso coverage on them since it's very fragile.

[venjirai]

In addition to setting wasAnimatingWhenDetachedto false in onAttachedToWindow() to fix the endless animation bug I would also introduce and expose a new flag that can turn restarting animations off altogether. It would then look something like this:

@Override 
protected void onDetachedFromWindow() {
    if (isAnimating()) {
      cancelAnimation();
      if (restartAnimationWhenDetached)
        wasAnimatingWhenDetached = true;
    }
    super.onDetachedFromWindow();
  }

[YuriiTsap]

@gpeal Sorry for late reply! I was kind of really far away from civilization and stable internet connection. Sure, I have few ideas how that espresso test could be written. Actually I've spent some time analyzing existing Tests inside FragmentVisibilityTests. I think my case is similar to testPausesWhenScrolledOffScreenAndResumesWhenComesBack() test, so I've taken it as an example and base for building mine. While playing around I've found an interesting thing inside that test.

        val scenario = launchFragmentInContainer<TestFragment>()
        onView(isAssignableFrom(RecyclerView::class.java)).check(matches(isDisplayed()))
        scenario.onFragment { assertTrue(it.animationView!!.isAnimating) }
        scenario.onFragment { it.requireView().scrollBy(0, 10_000) }
        scenario.onFragment { assertFalse(it.animationView!!.isAnimating) }
        scenario.onFragment { it.requireView().scrollBy(0, -10_000) }
        scenario.onFragment { assertTrue(it.animationView!!.isAnimating) }

In this code snippet we are scrolling back and forth and verifying that animationView restarts animating when it is visible. But that is kinda tricky, as it would be playing animation in any case, because onBindViewHolder()->bindLottieHolder() is called :

private fun bindLottieHolder(holder: RecyclerView.ViewHolder) {
                          animationView = holder.itemView as LottieAnimationView
                          (holder.itemView as LottieAnimationView).apply {
                                repeatCount = LottieDrawable.INFINITE
                                setAnimation(R.raw.heart)
                                playAnimation()
                                IdlingRegistry.getInstance().register(LottieIdlingResource(this, name = "Lottie ${Random.nextFloat()}"))
                            }
                        }

So as result playAnimation() is called and in any case animationView!!.isAnimating will return true. Per my mind - this test is testing wrong flow, it should be adjusted. As usually - I could be wrong.
Going back to mine case - the best way here is to create a PR, it would be easier to review changes. Anyway I'll explain mine approach and solution here:

• I've introduced new test(sorry for naming, I'm not a naming guru):

@Test
fun testPausesWhenScrolledOffScreenAndResumesWhenComesBackWithoutRepeatingWhenFinished() {

        class TestFragment : Fragment() {
            var animationView: LottieAnimationView? = null
            override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View? {
                return RecyclerView(requireContext()).apply {
                    layoutManager = LinearLayoutManager(requireContext(), LinearLayoutManager.VERTICAL, false)
                    adapter = object : RecyclerView.Adapter<RecyclerView.ViewHolder>() {

                        var animationWasPlayed = false

                        override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): RecyclerView.ViewHolder {
                            return when (viewType) {
                                0 -> object : RecyclerView.ViewHolder(
                                        LottieAnimationView(parent.context)
                                                .apply { id = R.id.animation_view }
                                ) {}
                                else -> object : RecyclerView.ViewHolder(TextView(parent.context)) {}
                            }
                        }

                        override fun getItemCount(): Int = 1000

                        override fun getItemViewType(position: Int) = position

                        override fun onBindViewHolder(holder: RecyclerView.ViewHolder, position: Int) {
                            if (holder.itemViewType == 0) bindLottieHolder(holder)
                            else bindOtherViewHolder(holder, position)
                        }

                        private fun bindLottieHolder(holder: RecyclerView.ViewHolder) {
                            if (!animationWasPlayed) {
                                animationView = holder.itemView as LottieAnimationView
                                (holder.itemView as LottieAnimationView).apply {
                                    setAnimation(R.raw.heart)
                                    playAnimation()
                                    animationWasPlayed = true
                                    IdlingRegistry.getInstance().register(LottieIdlingResource(this, name = "Lottie ${Random.nextFloat()}"))
                                }
                            } else {
                                IdlingRegistry.getInstance().register(LottieIdlingAnimationResource(animationView, name = "Lottie finished animation ${Random.nextFloat()}"))
                            }
                        }

                        private fun bindOtherViewHolder(holder: RecyclerView.ViewHolder, position: Int) {
                            (holder.itemView as TextView).text = "Item $position"
                        }
                    }
                }
            }
        }

        val scenario = launchFragmentInContainer<TestFragment>()
        onView(isAssignableFrom(RecyclerView::class.java)).check(matches(isDisplayed()))
        scenario.onFragment { assertTrue(it.animationView!!.isAnimating) }
        scenario.onFragment { it.requireView().scrollBy(0, 10_000) }
        scenario.onFragment { assertFalse(it.animationView!!.isAnimating) }
        scenario.onFragment { it.requireView().scrollBy(0, -10_000) }
        scenario.onFragment { assertTrue(it.animationView!!.isAnimating) }
        onView(withId(R.id.animation_view)).check(matches(isDisplayed()))
        scenario.onFragment { assertFalse(it.animationView!!.isAnimating) }
        scenario.onFragment { it.requireView().scrollBy(0, 10_000) }
        scenario.onFragment { it.requireView().scrollBy(0, -10_000) }
        scenario.onFragment { assertFalse(it.animationView!!.isAnimating) }
    }

• I've introduced new IdlingResource, which is waiting until lottie animation is finished:

class LottieIdlingAnimationResource(animationView: LottieAnimationView?, private val name: String = "Lottie") : IdlingResource {

    init {
        animationView?.addAnimatorListener(object : AnimatorListenerAdapter() {
            override fun onAnimationStart(animation: Animator) {
                isIdle = false
            }

            override fun onAnimationEnd(animation: Animator) {
                isIdle = true
                callback?.onTransitionToIdle()
                animationView.removeAllAnimatorListeners()
                IdlingRegistry.getInstance().unregister(this@LottieIdlingAnimationResource)
            }
        })
    }

    private var callback: IdlingResource.ResourceCallback? = null
    private var isIdle = animationView?.isAnimating?.not() ?: true


    override fun getName() = name

    override fun isIdleNow() = isIdle

    override fun registerIdleTransitionCallback(callback: IdlingResource.ResourceCallback?) {
        this.callback = callback
        if (isIdle) callback?.onTransitionToIdle()
    }
}

• Added one line that fixes the issue at LottieAnimationView.java 286 line inside onAttachedToWindow() callback:

wasAnimatingWhenDetached = false;

BTW testPausesWhenScrolledOffScreenAndResumesWhenComesBack() test could be adjusted to use animationWasPlayed boolean(as in mine test) and that will fix it.
Probably my comment is not a small one, because of that I prefer creating a PR.
Thanks a lot!

[YuriiTsap]

@gpeal Any updates on this thread? Is it possible to create a PR?

[gpeal]

@YuriiTsap Feel free to make a PR :)

[sahibjaspal]

I'm still seeing this in 3.1.0. Using post { lottieView.playAnimation() } works for now (https://github.com/airbnb/lottie-android/issues/1284#issuecomment-507229013).

[DavidDTA]

I am likewise still seeing #1284 in 3.3.1