Fix focus listener not being invoked for first child when focus is se…

…nt to composables
rubensousa · Mar 17, 2024 · dab1b09 · dab1b09
1 parent b51f07f
commit dab1b09
Show file tree

Hide file tree

Showing 6 changed files with 126 additions and 12 deletions.
diff --git a/...ndroidTest/java/com/rubensousa/dpadrecyclerview/compose/DpadComposeFocusViewHolderTest.kt b/...ndroidTest/java/com/rubensousa/dpadrecyclerview/compose/DpadComposeFocusViewHolderTest.kt
@@ -31,9 +31,12 @@ import com.google.common.truth.Truth.assertThat
 import com.rubensousa.dpadrecyclerview.DpadRecyclerView
 import com.rubensousa.dpadrecyclerview.ExtraLayoutSpaceStrategy
 import com.rubensousa.dpadrecyclerview.compose.test.ComposeFocusTestActivity
+import com.rubensousa.dpadrecyclerview.testfixtures.DpadFocusEvent
 import com.rubensousa.dpadrecyclerview.testfixtures.recording.ScreenRecorderRule
 import com.rubensousa.dpadrecyclerview.testing.KeyEvents
+import com.rubensousa.dpadrecyclerview.testing.R
 import com.rubensousa.dpadrecyclerview.testing.actions.DpadRecyclerViewActions
+import com.rubensousa.dpadrecyclerview.testing.actions.DpadViewActions
 import com.rubensousa.dpadrecyclerview.testing.rules.DisableIdleTimeoutRule
 import org.junit.Rule
 import org.junit.Test
@@ -142,6 +145,49 @@ class DpadComposeFocusViewHolderTest {
         assertThat(disposals).contains(0)
     }
 
+    @Test
+    fun testFocusEventIsReceivedForFirstChild() {
+        var focusEvents: List<DpadFocusEvent> = emptyList()
+        composeTestRule.activityRule.scenario.onActivity { activity ->
+            focusEvents = activity.getFocusEvents()
+        }
+
+        // when
+        onView(ViewMatchers.withId(R.id.recyclerView))
+            .perform(
+                DpadViewActions.waitForCondition<DpadRecyclerView>(
+                    description = "Wait for focus event",
+                    condition = { recyclerView -> focusEvents.isNotEmpty() }
+                )
+            )
+
+        assertThat(focusEvents).hasSize(1)
+        val event = focusEvents.first()
+        assertThat(event.position).isEqualTo(0)
+        assertThat(event.child).isInstanceOf(ComposeView::class.java)
+    }
+
+    @Test
+    fun testAllViewHoldersAreFocusedOnKeyPress() {
+        // given
+        val events = 10
+
+        // when
+        repeat(events) {
+            KeyEvents.pressDown()
+            waitForIdleScroll()
+        }
+
+        // then
+        var focusEvents: List<DpadFocusEvent> = emptyList()
+        composeTestRule.activityRule.scenario.onActivity { activity ->
+            focusEvents = activity.getFocusEvents()
+        }
+
+        assertThat(focusEvents).hasSize(events + 1)
+        assertThat(focusEvents.map { it.position }).isEqualTo(List(events + 1) { it })
+    }
+
     private fun waitForIdleScroll() {
         onView(ViewMatchers.isAssignableFrom(DpadRecyclerView::class.java))
             .perform(DpadRecyclerViewActions.waitForIdleScroll())

diff --git a/...src/androidTest/java/com/rubensousa/dpadrecyclerview/compose/DpadComposeViewHolderTest.kt b/...src/androidTest/java/com/rubensousa/dpadrecyclerview/compose/DpadComposeViewHolderTest.kt
@@ -29,9 +29,12 @@ import com.google.common.truth.Truth.assertThat
 import com.rubensousa.dpadrecyclerview.DpadRecyclerView
 import com.rubensousa.dpadrecyclerview.ExtraLayoutSpaceStrategy
 import com.rubensousa.dpadrecyclerview.compose.test.ViewFocusTestActivity
+import com.rubensousa.dpadrecyclerview.testfixtures.DpadFocusEvent
 import com.rubensousa.dpadrecyclerview.testfixtures.recording.ScreenRecorderRule
 import com.rubensousa.dpadrecyclerview.testing.KeyEvents
+import com.rubensousa.dpadrecyclerview.testing.R
 import com.rubensousa.dpadrecyclerview.testing.actions.DpadRecyclerViewActions
+import com.rubensousa.dpadrecyclerview.testing.actions.DpadViewActions
 import com.rubensousa.dpadrecyclerview.testing.rules.DisableIdleTimeoutRule
 import org.junit.Rule
 import org.junit.Test
@@ -145,6 +148,49 @@ class DpadComposeViewHolderTest {
         assertThat(disposals).contains(0)
     }
 
+    @Test
+    fun testFocusEventIsReceivedForFirstChild() {
+        var focusEvents: List<DpadFocusEvent> = emptyList()
+        composeTestRule.activityRule.scenario.onActivity { activity ->
+            focusEvents = activity.getFocusEvents()
+        }
+
+        // when
+        onView(ViewMatchers.withId(R.id.recyclerView))
+            .perform(
+                DpadViewActions.waitForCondition<DpadRecyclerView>(
+                    description = "Wait for focus event",
+                    condition = { recyclerView -> focusEvents.isNotEmpty() }
+                )
+            )
+
+        assertThat(focusEvents).hasSize(1)
+        val event = focusEvents.first()
+        assertThat(event.position).isEqualTo(0)
+        assertThat(event.child).isInstanceOf(DpadComposeView::class.java)
+    }
+
+    @Test
+    fun testAllViewHoldersAreFocusedOnKeyPress() {
+        // given
+        val events = 10
+
+        // when
+        repeat(events) {
+            KeyEvents.pressDown()
+            waitForIdleScroll()
+        }
+
+        // then
+        var focusEvents: List<DpadFocusEvent> = emptyList()
+        composeTestRule.activityRule.scenario.onActivity { activity ->
+            focusEvents = activity.getFocusEvents()
+        }
+
+        assertThat(focusEvents).hasSize(events + 1)
+        assertThat(focusEvents.map { it.position }).isEqualTo(List(events + 1) { it })
+    }
+
     private fun waitForIdleScroll() {
         onView(ViewMatchers.isAssignableFrom(DpadRecyclerView::class.java))
             .perform(DpadRecyclerViewActions.waitForIdleScroll())

diff --git a/...androidTest/java/com/rubensousa/dpadrecyclerview/compose/test/ComposeFocusTestActivity.kt b/...androidTest/java/com/rubensousa/dpadrecyclerview/compose/test/ComposeFocusTestActivity.kt
@@ -43,17 +43,17 @@ class ComposeFocusTestActivity : AppCompatActivity() {
         super.onCreate(savedInstanceState)
         setContentView(R.layout.compose_test)
         recyclerView = findViewById(R.id.recyclerView)
+        recyclerView.addOnViewFocusedListener(object : OnViewFocusedListener {
+            override fun onViewFocused(parent: RecyclerView.ViewHolder, child: View) {
+                focusEvents.add(DpadFocusEvent(parent, child, parent.layoutPosition))
+            }
+        })
         recyclerView.adapter = Adapter(
             items = List(100) { it },
             onDispose = { item ->
                 disposals.add(item)
             }
         )
-        recyclerView.addOnViewFocusedListener(object : OnViewFocusedListener {
-            override fun onViewFocused(parent: RecyclerView.ViewHolder, child: View) {
-                focusEvents.add(DpadFocusEvent(parent, child, parent.layoutPosition))
-            }
-        })
         recyclerView.requestFocus()
     }
 
@@ -73,6 +73,8 @@ class ComposeFocusTestActivity : AppCompatActivity() {
         return disposals
     }
 
+    fun getFocusEvents(): List<DpadFocusEvent> = focusEvents.toList()
+
     fun removeAdapter() {
         recyclerView.adapter = null
     }

diff --git a/...rc/androidTest/java/com/rubensousa/dpadrecyclerview/compose/test/ViewFocusTestActivity.kt b/...rc/androidTest/java/com/rubensousa/dpadrecyclerview/compose/test/ViewFocusTestActivity.kt
@@ -27,19 +27,27 @@ import androidx.compose.ui.unit.dp
 import androidx.core.view.children
 import androidx.recyclerview.widget.RecyclerView
 import com.rubensousa.dpadrecyclerview.DpadRecyclerView
+import com.rubensousa.dpadrecyclerview.OnViewFocusedListener
 import com.rubensousa.dpadrecyclerview.compose.DpadComposeViewHolder
 import com.rubensousa.dpadrecyclerview.compose.TestComposable
+import com.rubensousa.dpadrecyclerview.testfixtures.DpadFocusEvent
 
 class ViewFocusTestActivity : AppCompatActivity() {
 
     private lateinit var recyclerView: DpadRecyclerView
+    private val focusEvents = arrayListOf<DpadFocusEvent>()
     private val clicks = ArrayList<Int>()
     private val disposals = ArrayList<Int>()
 
     override fun onCreate(savedInstanceState: Bundle?) {
         super.onCreate(savedInstanceState)
         setContentView(R.layout.compose_test)
         recyclerView = findViewById(R.id.recyclerView)
+        recyclerView.addOnViewFocusedListener(object : OnViewFocusedListener {
+            override fun onViewFocused(parent: RecyclerView.ViewHolder, child: View) {
+                focusEvents.add(DpadFocusEvent(parent, child, parent.layoutPosition))
+            }
+        })
         recyclerView.adapter = Adapter(
             items = List(100) { it },
             onDispose = { item ->
@@ -49,6 +57,8 @@ class ViewFocusTestActivity : AppCompatActivity() {
         recyclerView.requestFocus()
     }
 
+    fun getFocusEvents(): List<DpadFocusEvent> = focusEvents.toList()
+
     fun requestFocus() {
         recyclerView.requestFocus()
     }

diff --git a/...recyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/PivotSelector.kt b/...recyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/PivotSelector.kt
@@ -101,16 +101,26 @@ internal class PivotSelector(
             return
         }
         val currentRecyclerView = recyclerView ?: return
-        // Do not notify listeners for views that are not a direct child of this RecyclerView
-        // This will happen when a parent RecyclerView
-        // finds a focusable inside a nested RecyclerView
+        /**
+         * Do not notify listeners for views that are not a direct child of this RecyclerView
+         * This will happen when a parent RecyclerView
+         * finds a focusable inside a nested RecyclerView
+         */
         if (findParentRecyclerView(view) !== currentRecyclerView) {
             return
         }
-        // If the view didn't receive focus directly,
-        // skip the callback since focus was handled by a nested RecyclerView
+
+        /**
+         * If the view didn't receive focus directly,
+         * we need to verify if the focused view is actually part of this RecyclerView.
+         */
         if (view.hasFocus() && !view.isFocused) {
-            return
+            val focusedView = view.findFocus()
+            if (focusedView != null
+                && findParentRecyclerView(focusedView) !== currentRecyclerView
+            ) {
+                return
+            }
         }
         val focusedViewHolder = currentRecyclerView.findContainingViewHolder(view) ?: return
         focusListeners.forEach { listener ->

diff --git a/sample/src/main/java/com/rubensousa/dpadrecyclerview/sample/ui/screen/main/MainFragment.kt b/sample/src/main/java/com/rubensousa/dpadrecyclerview/sample/ui/screen/main/MainFragment.kt
@@ -56,7 +56,7 @@ class MainFragment : Fragment(R.layout.screen_main) {
                 parent: RecyclerView.ViewHolder,
                 child: View,
             ) {
-                Timber.i("Feature list focused: ${parent.layoutPosition}")
+                Timber.i("Feature list focused: ${parent.layoutPosition}, view: $child")
             }
         })