Skip to content

Commit

Permalink
Fix DpadRecyclerView losing focus after some item animations
Browse files Browse the repository at this point in the history
  • Loading branch information
rubensousa committed May 30, 2024
1 parent 30662a0 commit 6f8c0c1
Show file tree
Hide file tree
Showing 12 changed files with 113 additions and 61 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -43,28 +43,16 @@ class DpadRecyclerViewAssertionsTest : RecyclerViewTest() {
launchSubPositionFragment()

performActions(
DpadRecyclerViewActions.selectPosition(position = 3),
DpadRecyclerViewActions.selectPosition(position = 1),
DpadRecyclerViewActions.waitForIdleScroll()
)
assert(DpadRecyclerViewAssertions.isSelected(position = 3))
assert(DpadRecyclerViewAssertions.isSelected(position = 1))

performActions(
DpadRecyclerViewActions.selectPosition(position = 3, subPosition = 1, smooth = false),
DpadRecyclerViewActions.selectPosition(position = 2, subPosition = 1, smooth = false),
DpadRecyclerViewActions.waitForIdleScroll()
)
assert(DpadRecyclerViewAssertions.isSelected(position = 3, subPosition = 1))

performActions(
DpadRecyclerViewActions.selectPosition(position = 3, subPosition = 2),
DpadRecyclerViewActions.waitForIdleScroll()
)
assert(DpadRecyclerViewAssertions.isSelected(position = 3, subPosition = 2))

performActions(
DpadRecyclerViewActions.selectPosition(position = 6, subPosition = 1),
DpadRecyclerViewActions.waitForIdleScroll()
)
assert(DpadRecyclerViewAssertions.isSelected(position = 6, subPosition = 1))
assert(DpadRecyclerViewAssertions.isSelected(position = 2, subPosition = 1))
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand All @@ -87,7 +80,10 @@ class AccessibilityTest : DpadRecyclerViewTest() {
fun testVerticalScrollForward() {
assertFocusAndSelection(position = 0)

// when
performAccessibilityAction(AccessibilityAction.ACTION_SCROLL_FORWARD)

// then
assertFocusAndSelection(position = spanCount)
}

Expand All @@ -96,7 +92,10 @@ class AccessibilityTest : DpadRecyclerViewTest() {
val position = 20
selectPosition(position)

// when
performAccessibilityAction(AccessibilityAction.ACTION_SCROLL_BACKWARD)

// then
assertFocusAndSelection(position = position - spanCount)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -102,7 +101,6 @@ class SelectionTest : DpadRecyclerViewTest() {
waitForCondition("Waiting for 0 children") { recyclerView ->
recyclerView.childCount == 0
}
assertIsNotFocused()
assertSelectedPosition(RecyclerView.NO_POSITION)

assertThat(getSelectionEvents()).isEqualTo(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,15 +237,9 @@ open class DpadRecyclerView @JvmOverloads constructor(

final override fun requestLayout() {
if (isRequestLayoutAllowed()) {
if (DEBUG) {
Log.i(TAG, "Layout Requested")
}
super.requestLayout()
} else {
hasPendingLayout = true
if (DEBUG) {
Log.i(TAG, "Layout suppressed until scroll is idle")
}
}
}

Expand Down Expand Up @@ -379,22 +373,33 @@ 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
isRetainingFocus = childHasFocus && isFocusable
preRemoveView(childHasFocus = getChildAt(index)?.hasFocus() ?: false)
super.removeViewAt(index)
postRemoveView()
}

private fun preRemoveView(childHasFocus: Boolean) {
isRetainingFocus = (childHasFocus && isFocusable) || hasFocus()
pivotLayoutManager?.setIsRetainingFocus(isRetainingFocus)
if (isRetainingFocus) {
descendantFocusability = ViewGroup.FOCUS_BEFORE_DESCENDANTS
requestFocus()
}
}

private fun postRemoveView() {
descendantFocusability = ViewGroup.FOCUS_AFTER_DESCENDANTS
if (isRetainingFocus && childCount > 0) {
requestFocus()
}
super.removeViewAt(index)
isRetainingFocus = false
pivotLayoutManager?.setIsRetainingFocus(false)
}

final override fun setChildDrawingOrderCallback(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,7 @@ internal class LayoutConfiguration(properties: Properties) {
var isFocusSearchDisabled = false
private set

var isFocusSearchEnabledDuringAnimations = false
private set
private var isFocusSearchEnabledDuringAnimations = false

// Number of items to prefetch when first coming on screen with new data
var initialPrefetchItemCount = 4
Expand Down Expand Up @@ -141,6 +140,11 @@ internal class LayoutConfiguration(properties: Properties) {
setReverseLayout(properties.reverseLayout)
}

fun isFocusSearchDisabled(recyclerView: RecyclerView): Boolean {
return isFocusSearchDisabled
|| (!isFocusSearchEnabledDuringAnimations && recyclerView.isAnimating)
}

fun setRecycleChildrenOnDetach(recycle: Boolean) {
recycleChildrenOnDetach = recycle
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ class PivotLayoutManager(properties: Properties) : RecyclerView.LayoutManager()
override fun onLayoutCompleted(state: RecyclerView.State) {
pivotLayout.onLayoutCompleted(state)
if (hadFocusBeforeLayout) {
focusDispatcher.focusSelectedView()
focusDispatcher.focusSelectedView(recyclerView)
}
pivotSelector.onLayoutCompleted()
hadFocusBeforeLayout = false
Expand Down Expand Up @@ -315,15 +315,18 @@ class PivotLayoutManager(properties: Properties) : RecyclerView.LayoutManager()
fun onRequestFocusInDescendants(
direction: Int,
previouslyFocusedRect: Rect?
): Boolean = focusDispatcher.onRequestFocusInDescendants(direction, previouslyFocusedRect)
): Boolean {
return focusDispatcher.onRequestFocusInDescendants(direction, previouslyFocusedRect)
}

override fun onRequestChildFocus(
parent: RecyclerView,
state: RecyclerView.State,
child: View,
focused: View?
): Boolean {
return focusDispatcher.onRequestChildFocus(parent, child, focused)
focusDispatcher.onRequestChildFocus(parent, child, focused)
return true
}

// Disabled since only this LayoutManager knows how to position views
Expand Down Expand Up @@ -426,6 +429,10 @@ class PivotLayoutManager(properties: Properties) : RecyclerView.LayoutManager()
pivotSelector.removeCurrentViewHolderSelection(clearSelection = isScrollingFromTouchEvent)
}

internal fun setIsRetainingFocus(isRetainingFocus: Boolean) {
pivotSelector.isRetainingFocus = isRetainingFocus
}

fun setChildrenDrawingOrderEnabled(enabled: Boolean) {
configuration.setChildDrawingOrderEnabled(enabled)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ internal class PivotSelector(
const val OFFSET_DISABLED = Int.MIN_VALUE
}

var isRetainingFocus = false

var position: Int = RecyclerView.NO_POSITION
private set

Expand Down Expand Up @@ -100,7 +102,7 @@ internal class PivotSelector(
fun focus(view: View) {
view.requestFocus()
// Exit early if there's no one listening for focus events
if (focusListeners.isEmpty()) {
if (focusListeners.isEmpty() || isRetainingFocus) {
return
}
val currentRecyclerView = recyclerView ?: return
Expand Down Expand Up @@ -319,6 +321,7 @@ internal class PivotSelector(
fun clear() {
val hadPivot = position != RecyclerView.NO_POSITION
position = RecyclerView.NO_POSITION
subPosition = 0
positionOffset = 0
if (hadPivot) {
dispatchViewHolderSelected()
Expand Down
Loading

0 comments on commit 6f8c0c1

Please sign in to comment.