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

… into nested_states

dpadrecyclerview-compose/src/androidTest/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) {

dpadrecyclerview-testing/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))

dpadrecyclerview/api/dpadrecyclerview.api

@@ -162,7 +162,6 @@ public class com/rubensousa/dpadrecyclerview/DpadRecyclerView : androidx/recycle
 	public final fun smoothScrollBy (II)V
 	public final fun smoothScrollBy (IILandroid/view/animation/Interpolator;)V
 	public fun startNestedScroll (II)Z
-	public fun stopNestedScroll ()V
 }
 
 public abstract interface class com/rubensousa/dpadrecyclerview/DpadRecyclerView$OnKeyInterceptListener {

dpadrecyclerview/src/androidTest/kotlin/com/rubensousa/dpadrecyclerview/test/TestGridFragment.kt

@@ -17,6 +17,7 @@
 package com.rubensousa.dpadrecyclerview.test
 
 import android.os.Bundle
+import android.util.Log
 import android.view.View
 import androidx.fragment.app.Fragment
 import androidx.recyclerview.widget.RecyclerView
@@ -71,6 +72,10 @@ open class TestGridFragment : Fragment(R.layout.dpadrecyclerview_test_container)
         val layoutConfig = args.getParcelable<TestLayoutConfiguration>(ARG_LAYOUT_CONFIG)!!
         val adapterConfig = args.getParcelable<TestAdapterConfiguration>(ARG_ADAPTER_CONFIG)!!
         val recyclerView = view.findViewById<DpadRecyclerView>(R.id.recyclerView)
+        val placeHolderView = view.findViewById<View>(R.id.focusPlaceholderView)
+        placeHolderView.setOnFocusChangeListener { v, hasFocus ->
+            Log.i(DpadRecyclerView.TAG, "placeholder focus changed: $hasFocus")
+        }
         if (layoutConfig.useCustomViewPool) {
             recyclerView.setRecycledViewPool(viewPool)
         }

dpadrecyclerview/src/androidTest/kotlin/com/rubensousa/dpadrecyclerview/test/tests/focus/FocusListenerTest.kt

@@ -23,8 +23,10 @@ import com.rubensousa.dpadrecyclerview.ChildAlignment
 import com.rubensousa.dpadrecyclerview.OnViewFocusedListener
 import com.rubensousa.dpadrecyclerview.ParentAlignment
 import com.rubensousa.dpadrecyclerview.test.TestLayoutConfiguration
+import com.rubensousa.dpadrecyclerview.test.helpers.assertIsNotFocused
 import com.rubensousa.dpadrecyclerview.test.helpers.onRecyclerView
 import com.rubensousa.dpadrecyclerview.test.helpers.selectPosition
+import com.rubensousa.dpadrecyclerview.test.helpers.waitForCondition
 import com.rubensousa.dpadrecyclerview.test.helpers.waitForIdleScrollState
 import com.rubensousa.dpadrecyclerview.test.tests.DpadRecyclerViewTest
 import com.rubensousa.dpadrecyclerview.testing.KeyEvents
@@ -155,4 +157,18 @@ class FocusListenerTest : DpadRecyclerViewTest() {
         assertThat(events.map { it.position }.sorted()).isEqualTo(List(50) { it })
     }
 
+    @Test
+    fun testRecyclerViewLosesFocusWhenItLosesContent() {
+        // when
+        executeOnFragment { fragment ->
+            fragment.clearAdapter()
+        }
+        waitForCondition("Waiting for 0 children") { recyclerView ->
+            recyclerView.childCount == 0
+        }
+
+        // then
+        assertIsNotFocused()
+    }
+
 }

dpadrecyclerview/src/androidTest/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) {

dpadrecyclerview/src/androidTest/kotlin/com/rubensousa/dpadrecyclerview/test/tests/mutation/AdapterMutationTest.kt

@@ -27,6 +27,8 @@ import com.rubensousa.dpadrecyclerview.test.helpers.assertFocusAndSelection
 import com.rubensousa.dpadrecyclerview.test.helpers.assertItemAtPosition
 import com.rubensousa.dpadrecyclerview.test.helpers.getRelativeItemViewBounds
 import com.rubensousa.dpadrecyclerview.test.helpers.selectLastPosition
+import com.rubensousa.dpadrecyclerview.test.helpers.selectPosition
+import com.rubensousa.dpadrecyclerview.test.helpers.waitForAdapterUpdate
 import com.rubensousa.dpadrecyclerview.test.helpers.waitForIdleScrollState
 import com.rubensousa.dpadrecyclerview.test.tests.AbstractTestAdapter
 import com.rubensousa.dpadrecyclerview.test.tests.DpadRecyclerViewTest
@@ -264,4 +266,24 @@ class AdapterMutationTest : DpadRecyclerViewTest() {
         Espresso.onIdle()
 
     }
+
+    @Test
+    fun testRemovalOfLargeInterval() {
+        // given
+        val startList = List(100) { it }
+        mutateAdapter { adapter ->
+            adapter.submitList(startList.toMutableList())
+        }
+        waitForAdapterUpdate()
+
+        // when
+        selectPosition(99, smooth = true)
+        mutateAdapter { adapter ->
+            adapter.submitList(List(25) { 1000 + it }.toMutableList())
+        }
+        waitForAdapterUpdate()
+
+        // then
+        assertFocusAndSelection(0)
+    }
 }

dpadrecyclerview/src/androidTest/kotlin/com/rubensousa/dpadrecyclerview/test/tests/scrolling/AccessibilityTest.kt

@@ -62,13 +62,6 @@ class AccessibilityTest : DpadRecyclerViewTest() {
 
     @Test
     fun testAccessibilityInfo() {
-        assertThat(accessibilityNodeInfo.actionList.find {
-            it.id == AccessibilityActionCompat.ACTION_SCROLL_DOWN.id
-        }).isNotNull()
-        assertThat(accessibilityNodeInfo.actionList.find {
-            it.id == AccessibilityActionCompat.ACTION_SCROLL_UP.id
-        }).isNull()
-
         repeat(10) {
             KeyEvents.pressDown()
         }
@@ -87,7 +80,10 @@ class AccessibilityTest : DpadRecyclerViewTest() {
     fun testVerticalScrollForward() {
         assertFocusAndSelection(position = 0)
 
+        // when
         performAccessibilityAction(AccessibilityAction.ACTION_SCROLL_FORWARD)
+
+        // then
         assertFocusAndSelection(position = spanCount)
     }
 
@@ -96,7 +92,10 @@ class AccessibilityTest : DpadRecyclerViewTest() {
         val position = 20
         selectPosition(position)
 
+        // when
         performAccessibilityAction(AccessibilityAction.ACTION_SCROLL_BACKWARD)
+
+        // then
         assertFocusAndSelection(position = position - spanCount)
     }
 

dpadrecyclerview/src/androidTest/kotlin/com/rubensousa/dpadrecyclerview/test/tests/scrolling/PendingAlignmentTest.kt

@@ -94,7 +94,7 @@ class PendingAlignmentTest : DpadRecyclerViewTest() {
     private fun setupRecyclerView(
         maxPendingAlignments: Int,
         maxPendingMoves: Int = 10,
-        scrollDuration: Int = 4000
+        scrollDuration: Int = 2000
     ) {
         onRecyclerView("Setup") { recyclerView ->
             recyclerView.setSmoothScrollMaxPendingMoves(maxPendingMoves)

dpadrecyclerview/src/androidTest/kotlin/com/rubensousa/dpadrecyclerview/test/tests/selection/SelectionTest.kt

@@ -29,7 +29,6 @@ import com.rubensousa.dpadrecyclerview.test.assertions.ViewHolderAlignmentCountA
 import com.rubensousa.dpadrecyclerview.test.helpers.assertFocusAndSelection
 import com.rubensousa.dpadrecyclerview.test.helpers.assertFocusPosition
 import com.rubensousa.dpadrecyclerview.test.helpers.assertIsFocused
-import com.rubensousa.dpadrecyclerview.test.helpers.assertIsNotFocused
 import com.rubensousa.dpadrecyclerview.test.helpers.assertSelectedPosition
 import com.rubensousa.dpadrecyclerview.test.helpers.assertViewHolderSelected
 import com.rubensousa.dpadrecyclerview.test.helpers.onRecyclerView
@@ -102,7 +101,6 @@ class SelectionTest : DpadRecyclerViewTest() {
         waitForCondition("Waiting for 0 children") { recyclerView ->
             recyclerView.childCount == 0
         }
-        assertIsNotFocused()
         assertSelectedPosition(RecyclerView.NO_POSITION)
 
         assertThat(getSelectionEvents()).isEqualTo(

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
@@ -129,10 +130,10 @@ open class DpadRecyclerView @JvmOverloads constructor(
         val layout = PivotLayoutManager(properties)
         layout.setFocusOutAllowed(
             throughFront = typedArray.getBoolean(
-                R.styleable.DpadRecyclerView_dpadRecyclerViewFocusOutFront, true
+                R.styleable.DpadRecyclerView_dpadRecyclerViewFocusOutFront, false
             ),
             throughBack = typedArray.getBoolean(
-                R.styleable.DpadRecyclerView_dpadRecyclerViewFocusOutBack, true
+                R.styleable.DpadRecyclerView_dpadRecyclerViewFocusOutBack, false
             )
         )
         layout.setFocusOutSideAllowed(
@@ -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,26 @@ 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()) {
             super.requestLayout()
-            return
+        } else {
+            hasPendingLayout = true
         }
-        hasPendingLayout = true
+    }
+
+    private fun isRequestLayoutAllowed(): Boolean {
+        return scrollState == SCROLL_STATE_IDLE || layoutWhileScrollingEnabled
     }
 
     // Overriding to prevent WRAP_CONTENT behavior by replacing it
@@ -363,22 +373,31 @@ open class DpadRecyclerView @JvmOverloads constructor(
     }
 
     final override fun removeView(view: View) {
-        isRetainingFocus = view.hasFocus() && isFocusable
-        if (isRetainingFocus) {
-            requestFocus()
-        }
+        preRemoveView(childHasFocus = view.hasFocus())
         super.removeView(view)
-        isRetainingFocus = false
+        postRemoveView()
     }
 
     final override fun removeViewAt(index: Int) {
-        val childHasFocus = getChildAt(index)?.hasFocus() ?: false
+        preRemoveView(childHasFocus = getChildAt(index)?.hasFocus() ?: false)
+        super.removeViewAt(index)
+        postRemoveView()
+    }
+
+    private fun preRemoveView(childHasFocus: Boolean) {
         isRetainingFocus = childHasFocus && isFocusable
+        pivotLayoutManager?.setIsRetainingFocus(isRetainingFocus)
         if (isRetainingFocus) {
             requestFocus()
         }
-        super.removeViewAt(index)
+    }
+
+    private fun postRemoveView() {
+        if (isRetainingFocus && childCount > 0 && !hasFocus()) {
+            requestFocus()
+        }
         isRetainingFocus = false
+        pivotLayoutManager?.setIsRetainingFocus(false)
     }
 
     final override fun setChildDrawingOrderCallback(
@@ -420,25 +439,31 @@ open class DpadRecyclerView @JvmOverloads constructor(
         return result
     }
 
-    override fun stopNestedScroll() {
-        super.stopNestedScroll()
-        startedTouchScroll = false
-    }
-
     override fun onScrollStateChanged(state: Int) {
         super.onScrollStateChanged(state)
         if (state == SCROLL_STATE_IDLE) {
+            if (hasPendingLayout && !startedTouchScroll) {
+                scheduleLayout()
+            }
             startedTouchScroll = false
             pivotLayoutManager?.setScrollingFromTouchEvent(false)
-            if (hasPendingLayout) {
-                hasPendingLayout = false
-                requestLayout()
-            }
         } 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
+         */
+        ViewCompat.postOnAnimation(this) { 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 +1292,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
@@ -1298,10 +1321,7 @@ open class DpadRecyclerView @JvmOverloads constructor(
     private fun removeSelectionForRecycledViewHolders() {
         addRecyclerListener { holder ->
             val position = holder.absoluteAdapterPosition
-            if (holder is DpadViewHolder
-                && position != NO_POSITION
-                && position == getSelectedPosition()
-            ) {
+            if (position != NO_POSITION && position == getSelectedPosition()) {
                 pivotLayoutManager?.removeCurrentViewHolderSelection()
             }
         }