Fix: item change animations get canceled by ItemExpandAnimator

Saket Narayan · Saket Narayan · commit 354e5c119c66 · 2018-10-12T22:34:42.000+05:30
It's surprising that this bug flew under the radar for so long. ItemExpandAnimator registers a global layout listener on ExpandablePageLayout that was originally written to react to size changes and synchronize RecyclerView items with the page. As a side effect, it also used to override item add/remove/change/move animations. This commit modifies the implementation to ignore global layout callbacks unless the page's location or size changes.
diff --git a/inboxrecyclerview/src/main/java/me/saket/inboxrecyclerview/animation/ItemExpandAnimator.kt b/inboxrecyclerview/src/main/java/me/saket/inboxrecyclerview/animation/ItemExpandAnimator.kt
@@ -1,50 +1,27 @@
 package me.saket.inboxrecyclerview.animation
 
-import android.graphics.Rect
-import android.view.ViewTreeObserver
 import me.saket.inboxrecyclerview.InboxRecyclerView
-import me.saket.inboxrecyclerview.page.ExpandablePageLayout
 
 /**
  * Controls how [InboxRecyclerView] items are animated when its page is moving.
  * To create a custom animator, extend this and override [onPageMove].
  */
 abstract class ItemExpandAnimator {
 
-  private val pagePreDrawListener = object : ViewTreeObserver.OnPreDrawListener {
-    private var lastTranslationY = 0F
-    private var lastClippedDimens = Rect()
-    private var lastState = ExpandablePageLayout.PageState.COLLAPSED
-
-    override fun onPreDraw(): Boolean {
-      val page = recyclerView.page
-      if (lastTranslationY != page.translationY || lastClippedDimens != page.clippedDimens || lastState != page.currentState) {
-        onPageMove()
-      }
-
-      lastTranslationY = page.translationY
-      lastClippedDimens = page.clippedDimens
-      lastState = page.currentState
-      return true
-    }
-  }
-
-  private val pageLayoutChangeListener = {
-    // Changes in the page's dimensions will get handled here.
-    onPageMove()
-  }
-
   protected lateinit var recyclerView: InboxRecyclerView
+  private lateinit var changeDetector: PageLocationChangeDetector
 
   fun onAttachRecyclerView(recyclerView: InboxRecyclerView) {
     this.recyclerView = recyclerView
-    recyclerView.page.viewTreeObserver.addOnGlobalLayoutListener(pageLayoutChangeListener)
-    recyclerView.page.viewTreeObserver.addOnPreDrawListener(pagePreDrawListener)
+    this.changeDetector = PageLocationChangeDetector(recyclerView.page, changeListener = ::onPageMove)
+
+    recyclerView.page.viewTreeObserver.addOnGlobalLayoutListener(changeDetector)
+    recyclerView.page.viewTreeObserver.addOnPreDrawListener(changeDetector)
   }
 
   fun onDetachRecyclerView(recyclerView: InboxRecyclerView) {
-    recyclerView.page.viewTreeObserver.removeOnGlobalLayoutListener(pageLayoutChangeListener)
-    recyclerView.page.viewTreeObserver.removeOnPreDrawListener(pagePreDrawListener)
+    recyclerView.page.viewTreeObserver.removeOnGlobalLayoutListener(changeDetector)
+    recyclerView.page.viewTreeObserver.removeOnPreDrawListener(changeDetector)
   }
 
   /**
@@ -57,6 +34,9 @@ abstract class ItemExpandAnimator {
 
   companion object {
 
+    /**
+     * See [SplitExpandAnimator].
+     */
     @JvmStatic
     fun split() = SplitExpandAnimator()
   }
diff --git a/inboxrecyclerview/src/main/java/me/saket/inboxrecyclerview/animation/PageLocationChangeDetector.kt b/inboxrecyclerview/src/main/java/me/saket/inboxrecyclerview/animation/PageLocationChangeDetector.kt
@@ -0,0 +1,39 @@
+package me.saket.inboxrecyclerview.animation
+
+import android.graphics.Rect
+import android.view.ViewTreeObserver
+import me.saket.inboxrecyclerview.page.ExpandablePageLayout
+
+internal class PageLocationChangeDetector(
+    private val page: ExpandablePageLayout,
+    private val changeListener: () -> Unit
+) : ViewTreeObserver.OnPreDrawListener, ViewTreeObserver.OnGlobalLayoutListener {
+
+  private var lastTranslationY = 0F
+  private var lastClippedDimens = Rect()
+  private var lastState = ExpandablePageLayout.PageState.COLLAPSED
+
+  override fun onPreDraw(): Boolean {
+    dispatchCallbackIfNeeded()
+    return true
+  }
+
+  override fun onGlobalLayout() {
+    // Changes in the page's dimensions will get handled here.
+    dispatchCallbackIfNeeded()
+  }
+
+  private fun dispatchCallbackIfNeeded() {
+    val moved = lastTranslationY != page.translationY
+    val dimensionsChanged = lastClippedDimens != page.clippedDimens
+    val stateChanged = lastState != page.currentState
+
+    if (page.isCollapsed.not() && (moved || dimensionsChanged || stateChanged)) {
+      changeListener()
+    }
+
+    lastTranslationY = page.translationY
+    lastClippedDimens = page.clippedDimens
+    lastState = page.currentState
+  }
+}
