Merge branch 'master' of https://github.com/rubensousa/DpadRecyclerView…

… into grid_crash
rubensousa · May 30, 2024 · 12d85bd · 12d85bd
2 parents dbe8ef5 + f825086
commit 12d85bd
Show file tree

Hide file tree

Showing 6 changed files with 50 additions and 19 deletions.
diff --git a/...ndroidTest/java/com/rubensousa/dpadrecyclerview/compose/DpadComposeFocusViewHolderTest.kt b/...ndroidTest/java/com/rubensousa/dpadrecyclerview/compose/DpadComposeFocusViewHolderTest.kt
@@ -170,7 +170,7 @@ class DpadComposeFocusViewHolderTest {
     @Test
     fun testAllViewHoldersAreFocusedOnKeyPress() {
         // given
-        val events = 10
+        val events = 5
 
         // when
         repeat(events) {

diff --git a/...sting/src/androidTest/java/com/rubensousa/dpadrecyclerview/testing/ScrollToActionsTest.kt b/...sting/src/androidTest/java/com/rubensousa/dpadrecyclerview/testing/ScrollToActionsTest.kt
@@ -36,7 +36,7 @@ class ScrollToActionsTest : RecyclerViewTest() {
     fun testScrollDownToItem() {
         launchVerticalFragment()
 
-        val position = 15
+        val position = 5
         performActions(
             DpadRecyclerViewActions.scrollTo<RecyclerView.ViewHolder>(
                 hasDescendant(withText(position.toString()))
@@ -51,14 +51,17 @@ class ScrollToActionsTest : RecyclerViewTest() {
     fun testScrollUpToItem() {
         launchVerticalFragment()
 
-        performActions(DpadRecyclerViewActions.selectPosition(15, smooth = false))
+        performActions(
+            DpadRecyclerViewActions.scrollTo<RecyclerView.ViewHolder>(
+                hasDescendant(withText("5"))
+            )
+        )
 
         val position = 0
         performActions(
-            DpadRecyclerViewActions.waitForIdleScroll(),
             DpadRecyclerViewActions.scrollTo<RecyclerView.ViewHolder>(
                 hasDescendant(withText(position.toString()))
-            )
+            ),
         )
 
         assert(DpadRecyclerViewAssertions.isSelected(position))

diff --git a/...Test/kotlin/com/rubensousa/dpadrecyclerview/test/tests/layout/LayoutWhileScrollingTest.kt b/...Test/kotlin/com/rubensousa/dpadrecyclerview/test/tests/layout/LayoutWhileScrollingTest.kt
@@ -66,7 +66,6 @@ class LayoutWhileScrollingTest : DpadRecyclerViewTest() {
         // given
         var layoutCompleted = 0
         onRecyclerView("Disable layout during scroll") { recyclerView ->
-            recyclerView.setLayoutWhileScrollingEnabled(false)
             recyclerView.addOnLayoutCompletedListener(
                 object : DpadRecyclerView.OnLayoutCompletedListener {
                     override fun onLayoutCompleted(state: RecyclerView.State) {
@@ -95,7 +94,6 @@ class LayoutWhileScrollingTest : DpadRecyclerViewTest() {
         // given
         var layoutCompleted = 0
         onRecyclerView("Disable layout during scroll") { recyclerView ->
-            recyclerView.setLayoutWhileScrollingEnabled(false)
             recyclerView.addOnLayoutCompletedListener(
                 object : DpadRecyclerView.OnLayoutCompletedListener {
                     override fun onLayoutCompleted(state: RecyclerView.State) {

diff --git a/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/DpadRecyclerView.kt b/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/DpadRecyclerView.kt
@@ -21,6 +21,7 @@ import android.content.res.TypedArray
 import android.graphics.Canvas
 import android.graphics.Rect
 import android.util.AttributeSet
+import android.util.Log
 import android.view.Gravity
 import android.view.KeyEvent
 import android.view.MotionEvent
@@ -71,7 +72,7 @@ open class DpadRecyclerView @JvmOverloads constructor(
     private var isOverlappingRenderingEnabled = true
     private var isRetainingFocus = false
     private var startedTouchScroll = false
-    private var layoutWhileScrollingEnabled = true
+    private var layoutWhileScrollingEnabled = false
     private var hasPendingLayout = false
     private var touchInterceptListener: OnTouchInterceptListener? = null
     private var smoothScrollByBehavior: SmoothScrollByBehavior? = null
@@ -211,6 +212,7 @@ open class DpadRecyclerView @JvmOverloads constructor(
         pivotLayoutManager?.removeOnViewHolderSelectedListener(viewHolderTaskExecutor)
         pivotLayoutManager?.updateRecyclerView(null)
         if (pivotLayoutManager !== layout) {
+            pivotLayoutManager?.layoutCompletedListener = null
             pivotLayoutManager?.clearOnLayoutCompletedListeners()
             pivotLayoutManager?.clearOnViewHolderSelectedListeners()
         }
@@ -223,18 +225,32 @@ open class DpadRecyclerView @JvmOverloads constructor(
         }
         if (layout is PivotLayoutManager) {
             layout.updateRecyclerView(this)
+            layout.layoutCompletedListener = object : OnLayoutCompletedListener {
+                override fun onLayoutCompleted(state: State) {
+                    hasPendingLayout = false
+                }
+            }
             layout.addOnViewHolderSelectedListener(viewHolderTaskExecutor)
             pivotLayoutManager = layout
         }
     }
 
     final override fun requestLayout() {
-        if (layoutWhileScrollingEnabled || scrollState == SCROLL_STATE_IDLE) {
-            hasPendingLayout = false
+        if (isRequestLayoutAllowed()) {
+            if (DEBUG) {
+                Log.i(TAG, "Layout Requested")
+            }
             super.requestLayout()
-            return
+        } else {
+            hasPendingLayout = true
+            if (DEBUG) {
+                Log.i(TAG, "Layout suppressed until scroll is idle")
+            }
         }
-        hasPendingLayout = true
+    }
+
+    private fun isRequestLayoutAllowed(): Boolean {
+        return scrollState == SCROLL_STATE_IDLE || layoutWhileScrollingEnabled
     }
 
     // Overriding to prevent WRAP_CONTENT behavior by replacing it
@@ -431,14 +447,25 @@ open class DpadRecyclerView @JvmOverloads constructor(
             startedTouchScroll = false
             pivotLayoutManager?.setScrollingFromTouchEvent(false)
             if (hasPendingLayout) {
-                hasPendingLayout = false
-                requestLayout()
+                scheduleLayout()
             }
         } else if (startedTouchScroll) {
             pivotLayoutManager?.setScrollingFromTouchEvent(true)
         }
     }
 
+    private fun scheduleLayout() {
+        if (DEBUG) {
+            Log.i(TAG, "Scheduling pending layout request")
+        }
+        /**
+         * The delay here is intended because users can request selections
+         * while the layout was locked and in that case, we should honor those requests instead
+         * of just performing a full layout
+         */
+        post { requestLayout() }
+    }
+
     override fun onSizeChanged(w: Int, h: Int, oldw: Int, oldh: Int) {
         super.onSizeChanged(w, h, oldw, oldh)
         fadingEdge.onSizeChanged(w, h, oldw, oldh, this)
@@ -1267,15 +1294,13 @@ open class DpadRecyclerView @JvmOverloads constructor(
     fun getOnMotionInterceptListener(): OnMotionInterceptListener? = motionInterceptListener
 
     /**
-     * By default, [DpadRecyclerView] allows triggering a layout-pass during scrolling.
-     * However, there might be some cases where someone is interested in disabling this behavior,
-     * for example:
+     * By default, [DpadRecyclerView] skips layout requests during scrolling because of:
      * 1. Compose animations trigger a full unnecessary layout-pass
      * 2. Content jumping around while scrolling is not ideal sometimes
      *
      * @param enabled true if layout requests should be possible while scrolling,
      * or false if they should be postponed until [RecyclerView.SCROLL_STATE_IDLE].
-     * Default is true.
+     * Default is false.
      */
     fun setLayoutWhileScrollingEnabled(enabled: Boolean) {
         layoutWhileScrollingEnabled = enabled

diff --git a/...lerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/PivotLayoutManager.kt b/...lerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/PivotLayoutManager.kt
@@ -74,6 +74,7 @@ class PivotLayoutManager(properties: Properties) : RecyclerView.LayoutManager()
     private var hadFocusBeforeLayout = false
     private var recyclerView: RecyclerView? = null
     private var isScrollingFromTouchEvent = false
+    internal var layoutCompletedListener: DpadRecyclerView.OnLayoutCompletedListener? = null
 
     override fun checkLayoutParams(layoutParams: RecyclerView.LayoutParams?): Boolean {
         return layoutParams is DpadLayoutParams
@@ -128,6 +129,7 @@ class PivotLayoutManager(properties: Properties) : RecyclerView.LayoutManager()
         scroller.cancelSmoothScroller()
         pivotSelector.onLayoutChildren(state)
         pivotLayout.onLayoutChildren(recycler, state)
+        layoutCompletedListener?.onLayoutCompleted(state)
     }
 
     override fun onLayoutCompleted(state: RecyclerView.State) {

diff --git a/...recyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/PivotSelector.kt b/...recyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/PivotSelector.kt
@@ -79,7 +79,10 @@ internal class PivotSelector(
 
     fun consumePendingSelectionChanges(state: RecyclerView.State): Boolean {
         var consumed = false
-        if (position != RecyclerView.NO_POSITION && positionOffset != OFFSET_DISABLED) {
+        if (position != RecyclerView.NO_POSITION
+            && positionOffset != OFFSET_DISABLED
+            && positionOffset != 0
+        ) {
             applyPositionOffset(state.itemCount)
             subPosition = 0
             consumed = true