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.

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

Animation does not play when startAnimation is called in onBindViewHolder of an existing view #1495

Closed
DavidDTA opened this issue Feb 7, 2020 · 10 comments

Comments

@DavidDTA
Copy link

DavidDTA commented Feb 7, 2020

Describe the bug
When calling startAnimation in a RecyclerView's onBindViewHolder, the animation does not play. This issue was partially fixed in #1324, but that fix only handles the case when the LottieAnimationView is first created, and not when it is rebound with different data.

The cause of the issue appears to be that when onBindViewHolder is called, the view is in an unusual state where it is attached to the window, but isShown is false because it has no parent.

Steps To Reproduce
Here is a unit test that demonstrates the issue:

diff --git a/LottieSample/src/androidTest/java/com/airbnb/lottie/samples/FragmentVisibilityTests.kt b/LottieSample/src/androidTest/java/com/airbnb/lottie/samples/FragmentVisibilityTests.kt
index fc338b93..c48c684d 100644
--- a/LottieSample/src/androidTest/java/com/airbnb/lottie/samples/FragmentVisibilityTests.kt
+++ b/LottieSample/src/androidTest/java/com/airbnb/lottie/samples/FragmentVisibilityTests.kt
@@ -195,6 +195,51 @@ class FragmentVisibilityTests {
         onView(withId(R.id.animation_view)).check(matches(isAnimating()))
     }
 
+    @Test
+    fun testRecyclerViewCanAutoPlayInOnBindRebind() {
+        class TestFragment : Fragment() {
+            override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View? {
+                return RecyclerView(requireContext()).apply {
+                    layoutManager = LinearLayoutManager(requireContext(), LinearLayoutManager.VERTICAL, false)
+                    // Setting itemAnimator to null is important for this test in order to
+                    // prevent the recyclerview from creating an additional viewholder for the
+                    // purposes of animation.
+                    itemAnimator = null
+                    adapter = object : RecyclerView.Adapter<RecyclerView.ViewHolder>() {
+                        override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): RecyclerView.ViewHolder {
+                            return object : RecyclerView.ViewHolder(LottieAnimationView(parent.context).apply {
+                                id = R.id.animation_view
+                                setAnimation(R.raw.heart)
+                                IdlingRegistry.getInstance().register(LottieIdlingResource(this))
+                            }) {}
+                        }
+
+                        override fun getItemCount(): Int = 1
+
+                        override fun onBindViewHolder(holder: RecyclerView.ViewHolder, position: Int) {
+                            (holder.itemView as LottieAnimationView).apply {
+                                // Cancel first, then play. This prevents the animation from
+                                // carrying over from the initial binding.
+                                cancelAnimation()
+                                playAnimation()
+                            }
+                        }
+                    }
+                }
+            }
+        }
+
+        val fragmentScenario = launchFragmentInContainer<TestFragment>()
+        // I wasn't able to figure out exactly what was needed to create an idling resource.
+        // Waiting for [RecyclerView.doOnLayout] was insufficient.
+        Thread.sleep(500)
+        fragmentScenario.onFragment { fragment ->
+            (fragment.view as RecyclerView).adapter!!.notifyItemChanged(0)
+        }
+        Thread.sleep(500)
+        onView(withId(R.id.animation_view)).check(matches(isAnimating()))
+    }
+
     @Test
     fun testDoesntAutoplay() {
         class TestFragment : Fragment() {
@DavidDTA
Copy link
Author

DavidDTA commented Feb 10, 2020

This is the workaround I'm using:

+    if (isAttachedToWindow && !rootView.containsView(this)) {
+      // This is a workaround for https://github.com/airbnb/lottie-android/issues/1495.
+      doOnPreDraw { playAnimation() }
+      return
+    }

@gpeal
Copy link
Collaborator

gpeal commented Feb 16, 2020

@DavidDTA This is a very tricky case due to the way RecyclerView manages views (as you have mentioned). I'm not sure what the best course of action is that wouldn't cause other problems. Any ideas?

@DavidDTA
Copy link
Author

DavidDTA commented Feb 16, 2020

One option that feels hacky but might work is to override onDraw and call onVisibilityChanged(changedView, int visibility) before delegating to the superclass.

Another option may be onStartTemporaryDetach and onFinishTemporaryDetach. It looks like there is code in RecyclerView for calling these, but it is disabled by a hardcoded flag. This may require filing an issue against recyclerview.

Another option may be onVisibilityAggregated. It looks like this will be called when the view is attached. It is API24+, however.

@gpeal
Copy link
Collaborator

gpeal commented Feb 22, 2020

@DavidDTA Calling onVIsibilityChanged from onDraw would walk the entire parent view hierarchy on every draw call for all animations.

If using onVisibilityAggregated fixes this, even just for 24+, that might be acceptable.

@DavidDTA
Copy link
Author

DavidDTA commented Feb 24, 2020

I think onVisibilityChanged would only have to be called when wasAnimatingWhenNotShown is true.

@oissaq
Copy link

oissaq commented Apr 9, 2020

I'm having the same issue described here and in #1284
My workaround for now is to use post { icon.playAnimation() }
That seems to work but wondering if there's a better solution.

@LandChanning
Copy link

LandChanning commented Jun 23, 2020

@MainThread
  public void playAnimation() {
    if (isShown()) {
      lottieDrawable.playAnimation();
      enableOrDisableHardwareLayer();
    } else {
      playAnimationWhenShown = true;
    }
  }

function isShown() may return false because parent == null, but visiblity does not changed, so onVisibilityChanged() not be triggered, and 'playAnimationWhenShown' not used.

There should be a better solution than post {}

@SiXiWanZi
Copy link

SiXiWanZi commented Sep 7, 2020

testView.post { testView.playAnimation() }

@j2sun
Copy link

j2sun commented Nov 23, 2020

Same issue for me. Instead of RecyclerView, I encountered this problem in ViewPager. It works when using "post".

@gpeal
Copy link
Collaborator

gpeal commented Jan 14, 2022

I re-checked the test in the description (thank you for that) after landing #1981 and the test now passes!

@gpeal gpeal closed this as completed Jan 14, 2022
@gpeal gpeal added this to the 5.0 milestone Jan 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants