Merge pull request #224 from rubensousa/grid_incomplete_focus

Do not jump focus across rows when trying to focus within the same span group

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

@@ -217,6 +217,30 @@ class GridSpanHeaderScrollTest {
         assertFocusAndSelection(defaultGrid.size - spanCount)
     }
 
+    @Test
+    fun testFocusDoesNotMoveForIncompleteRow() {
+        // given
+        val spanCount = 4
+        setContent(
+            items = listOf(
+                -1,
+                1,
+                -2,
+                3, 4, 5,
+                -3,
+                6, 7, 8, 9
+            ),
+            spanCount = spanCount
+        )
+        assertFocusAndSelection(position = 1)
+
+        // when
+        KeyEvents.pressRight(times = spanCount)
+
+        // then
+        assertFocusAndSelection(position = 1)
+    }
+
     private fun setContent(items: List<Int>, spanCount: Int) {
         onRecyclerView("Set content") { recyclerView ->
             recyclerView.setSpanCount(spanCount)

dpadrecyclerview/src/androidTest/res/layout/dpadrecyclerview_grid_header.xml

@@ -2,7 +2,7 @@
 <FrameLayout xmlns:android="http://schemas.android.com/apk/res/android"
     xmlns:tools="http://schemas.android.com/tools"
     android:layout_width="match_parent"
-    android:layout_height="200dp"
+    android:layout_height="48dp"
     android:focusable="false"
     android:focusableInTouchMode="false">
 

dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/focus/FocusDirection.kt

@@ -19,16 +19,16 @@ package com.rubensousa.dpadrecyclerview.layoutmanager.focus
 import android.view.View
 
 internal enum class FocusDirection {
-    PREVIOUS_ITEM,
-    NEXT_ITEM,
+    PREVIOUS_ROW,
+    NEXT_ROW,
     PREVIOUS_COLUMN,
     NEXT_COLUMN;
 
     fun getScrollSign(reverseLayout: Boolean): Int {
         if (this == NEXT_COLUMN || this == PREVIOUS_COLUMN) {
             return 0
         }
-        return if (this == NEXT_ITEM != reverseLayout) {
+        return if (this == NEXT_ROW != reverseLayout) {
             1
         } else {
             -1
@@ -60,8 +60,8 @@ internal enum class FocusDirection {
             val absoluteDirection = getAbsoluteDirection(direction, isVertical, isVertical)
             return if (isVertical) {
                 when (absoluteDirection) {
-                    View.FOCUS_UP -> if (reverseLayout) NEXT_ITEM else PREVIOUS_ITEM
-                    View.FOCUS_DOWN -> if (reverseLayout) PREVIOUS_ITEM else NEXT_ITEM
+                    View.FOCUS_UP -> if (reverseLayout) NEXT_ROW else PREVIOUS_ROW
+                    View.FOCUS_DOWN -> if (reverseLayout) PREVIOUS_ROW else NEXT_ROW
                     View.FOCUS_LEFT -> {
                         if (reverseLayout) NEXT_COLUMN else PREVIOUS_COLUMN
                     }
@@ -73,10 +73,10 @@ internal enum class FocusDirection {
             } else {
                 when (absoluteDirection) {
                     View.FOCUS_LEFT -> {
-                        if (reverseLayout) NEXT_ITEM else PREVIOUS_ITEM
+                        if (reverseLayout) NEXT_ROW else PREVIOUS_ROW
                     }
                     View.FOCUS_RIGHT -> {
-                        if (reverseLayout) PREVIOUS_ITEM else NEXT_ITEM
+                        if (reverseLayout) PREVIOUS_ROW else NEXT_ROW
                     }
                     View.FOCUS_UP -> if (reverseLayout) NEXT_COLUMN else PREVIOUS_COLUMN
                     View.FOCUS_DOWN -> if (reverseLayout) PREVIOUS_COLUMN else NEXT_COLUMN

dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/focus/FocusDispatcher.kt

@@ -176,7 +176,7 @@ internal class FocusDispatcher(
 
         val isScrolling = currentRecyclerView.scrollState != RecyclerView.SCROLL_STATE_IDLE
         when (focusDirection) {
-            FocusDirection.NEXT_ITEM -> {
+            FocusDirection.NEXT_ROW -> {
                 if (isScrolling || !configuration.focusOutBack) {
                     newFocusedView = focused
                 }
@@ -188,7 +188,7 @@ internal class FocusDispatcher(
                 }
             }
 
-            FocusDirection.PREVIOUS_ITEM -> {
+            FocusDirection.PREVIOUS_ROW -> {
                 if (isScrolling || !configuration.focusOutFront) {
                     newFocusedView = focused
                 }
@@ -390,9 +390,9 @@ internal class FocusDispatcher(
             }
             val position = layoutInfo.getAdapterPositionOf(child)
             val spanIndex = layoutInfo.getStartSpanIndex(position)
-            if (request.focusDirection == FocusDirection.NEXT_ITEM) {
+            if (request.focusDirection == FocusDirection.NEXT_ROW) {
                 child.addFocusables(views, direction, focusableMode)
-            } else if (request.focusDirection == FocusDirection.PREVIOUS_ITEM) {
+            } else if (request.focusDirection == FocusDirection.PREVIOUS_ROW) {
                 child.addFocusables(views, direction, focusableMode)
             } else if (request.focusDirection == FocusDirection.NEXT_COLUMN) {
                 // Add all focusable items after this item whose row index is bigger
@@ -424,14 +424,22 @@ internal class FocusDispatcher(
         direction: Int,
         focusableMode: Int
     ): Boolean {
-        if (configuration.spanCount == 1
-            || (movement != FocusDirection.PREVIOUS_ITEM && movement != FocusDirection.NEXT_ITEM)
-            || focusedPosition == RecyclerView.NO_POSITION
-        ) {
+        if (configuration.spanCount == 1 || focusedPosition == RecyclerView.NO_POSITION) {
             return false
         }
+
+        if (movement == FocusDirection.NEXT_COLUMN || movement == FocusDirection.PREVIOUS_COLUMN) {
+            return focusNextSpanColumn(
+                focusedPosition = focusedPosition,
+                next = movement == FocusDirection.NEXT_COLUMN,
+                views = views,
+                direction = direction,
+                focusableMode = focusableMode
+            )
+        }
+
         val reverseLayout = layoutInfo.shouldReverseLayout()
-        val edgeView = if (movement == FocusDirection.NEXT_ITEM != reverseLayout) {
+        val edgeView = if (movement == FocusDirection.NEXT_ROW != reverseLayout) {
             layoutInfo.getChildClosestToEnd()
         } else {
             layoutInfo.getChildClosestToStart()
@@ -445,7 +453,7 @@ internal class FocusDispatcher(
             nextPosition = spanFocusFinder.findNextSpanPosition(
                 focusedPosition = nextPosition,
                 spanSizeLookup = configuration.spanSizeLookup,
-                forward = movement == FocusDirection.NEXT_ITEM,
+                forward = movement == FocusDirection.NEXT_ROW,
                 edgePosition = edgePosition,
                 reverseLayout = layoutInfo.shouldReverseLayout()
             )
@@ -459,6 +467,36 @@ internal class FocusDispatcher(
 
     }
 
+    private fun focusNextSpanColumn(
+        focusedPosition: Int,
+        next: Boolean,
+        views: ArrayList<View>,
+        direction: Int,
+        focusableMode: Int
+    ): Boolean {
+        val positionIncrement = if (next xor layoutInfo.shouldReverseLayout()) {
+            1
+        } else {
+            -1
+        }
+        val nextPosition = focusedPosition + positionIncrement
+        if (nextPosition < 0 || nextPosition >= layout.itemCount) {
+            return false
+        }
+        val focusedRow = layoutInfo.getSpanGroupIndex(focusedPosition)
+        val nextRow = layoutInfo.getSpanGroupIndex(focusedPosition + positionIncrement)
+        if (focusedRow != nextRow) {
+            // Consume the focus since we can only focus within the same row here
+            return true
+        }
+        val nextView = layout.findViewByPosition(nextPosition) ?: return false
+        if (nextView.hasFocusable()) {
+            nextView.addFocusables(views, direction, focusableMode)
+            return true
+        }
+        return true
+    }
+
     class AddFocusableChildrenRequest(private val layoutInfo: LayoutInfo) {
 
         var start: Int = 0
@@ -476,7 +514,7 @@ internal class FocusDispatcher(
         var focusedAdapterPosition: Int = RecyclerView.NO_POSITION
             private set
 
-        var focusDirection: FocusDirection = FocusDirection.NEXT_ITEM
+        var focusDirection: FocusDirection = FocusDirection.NEXT_ROW
             private set
 
         var focusedSpanIndex: Int = RecyclerView.NO_POSITION
@@ -497,16 +535,16 @@ internal class FocusDispatcher(
                 RecyclerView.NO_POSITION
             }
 
-            increment = if (focusDirection == FocusDirection.NEXT_ITEM
+            increment = if (focusDirection == FocusDirection.NEXT_ROW
                 || focusDirection == FocusDirection.NEXT_COLUMN
             ) {
                 1
             } else {
                 -1
             }
             if (layoutInfo.shouldReverseLayout()
-                && (focusDirection == FocusDirection.NEXT_ITEM
-                        || focusDirection == FocusDirection.PREVIOUS_ITEM)
+                && (focusDirection == FocusDirection.NEXT_ROW
+                        || focusDirection == FocusDirection.PREVIOUS_ROW)
             ) {
                 increment *= -1
             }

dpadrecyclerview/src/test/java/com/rubensousa/dpadrecyclerview/test/layoutmanager/focus/FocusDirectionTest.kt

@@ -113,31 +113,31 @@ class FocusDirectionTest {
                 isVertical = true,
                 reverseLayout = false
             )
-        ).isEqualTo(FocusDirection.PREVIOUS_ITEM)
+        ).isEqualTo(FocusDirection.PREVIOUS_ROW)
 
         assertThat(
             FocusDirection.from(
                 direction = View.FOCUS_UP,
                 isVertical = true,
                 reverseLayout = true
             )
-        ).isEqualTo(FocusDirection.NEXT_ITEM)
+        ).isEqualTo(FocusDirection.NEXT_ROW)
 
         assertThat(
             FocusDirection.from(
                 direction = View.FOCUS_DOWN,
                 isVertical = true,
                 reverseLayout = false
             )
-        ).isEqualTo(FocusDirection.NEXT_ITEM)
+        ).isEqualTo(FocusDirection.NEXT_ROW)
 
         assertThat(
             FocusDirection.from(
                 direction = View.FOCUS_DOWN,
                 isVertical = true,
                 reverseLayout = true
             )
-        ).isEqualTo(FocusDirection.PREVIOUS_ITEM)
+        ).isEqualTo(FocusDirection.PREVIOUS_ROW)
 
         assertThat(
             FocusDirection.from(
@@ -196,31 +196,31 @@ class FocusDirectionTest {
                 isVertical = false,
                 reverseLayout = false
             )
-        ).isEqualTo(FocusDirection.PREVIOUS_ITEM)
+        ).isEqualTo(FocusDirection.PREVIOUS_ROW)
 
         assertThat(
             FocusDirection.from(
                 direction = View.FOCUS_RIGHT,
                 isVertical = false,
                 reverseLayout = false
             )
-        ).isEqualTo(FocusDirection.NEXT_ITEM)
+        ).isEqualTo(FocusDirection.NEXT_ROW)
 
         assertThat(
             FocusDirection.from(
                 direction = View.FOCUS_LEFT,
                 isVertical = false,
                 reverseLayout = true
             )
-        ).isEqualTo(FocusDirection.NEXT_ITEM)
+        ).isEqualTo(FocusDirection.NEXT_ROW)
 
         assertThat(
             FocusDirection.from(
                 direction = View.FOCUS_RIGHT,
                 isVertical = false,
                 reverseLayout = true
             )
-        ).isEqualTo(FocusDirection.PREVIOUS_ITEM)
+        ).isEqualTo(FocusDirection.PREVIOUS_ROW)
     }
 
     @Test
@@ -236,22 +236,22 @@ class FocusDirectionTest {
             )
         ).isEqualTo(0)
         assertThat(
-            FocusDirection.NEXT_ITEM.getScrollSign(
+            FocusDirection.NEXT_ROW.getScrollSign(
                 reverseLayout = false
             )
         ).isEqualTo(1)
         assertThat(
-            FocusDirection.NEXT_ITEM.getScrollSign(
+            FocusDirection.NEXT_ROW.getScrollSign(
                 reverseLayout = true
             )
         ).isEqualTo(-1)
         assertThat(
-            FocusDirection.PREVIOUS_ITEM.getScrollSign(
+            FocusDirection.PREVIOUS_ROW.getScrollSign(
                 reverseLayout = false
             )
         ).isEqualTo(-1)
         assertThat(
-            FocusDirection.PREVIOUS_ITEM.getScrollSign(
+            FocusDirection.PREVIOUS_ROW.getScrollSign(
                 reverseLayout = true
             )
         ).isEqualTo(1)