Merge pull request #161 from rubensousa/scrolling_issue

Fix max edge alignment when there are few items in the layout

docs/changelog.md

@@ -2,6 +2,15 @@
 
 ## Version 1.1.0
 
+### 1.1.0-beta01
+
+2023-09-10
+
+#### Bug fixes
+
+- Fixed wrong scrolling behavior when the layout isn't completely filled and `Edge.MAX` alignment is used: ([#160](https://github.com/rubensousa/DpadRecyclerView/issues/160))
+- Fixed XML attribute `app:dpadRecyclerViewParentAlignmentPreferKeylineOverEdge` not being applied correctly
+
 ### 1.1.0-alpha03
 
 2023-08-04

dpadrecyclerview/src/androidTest/kotlin/com/rubensousa/dpadrecyclerview/test/tests/alignment/HorizontalAlignmentTest.kt

@@ -368,25 +368,26 @@ class HorizontalAlignmentTest : DpadRecyclerViewTest() {
             layoutConfiguration = getDefaultLayoutConfiguration().copy(
                 parentAlignment = ParentAlignment(
                     edge = Edge.MIN,
-                    offset = 100,
-                    fraction = 0f
+                    offset = 0,
+                    fraction = 0f,
+                    preferKeylineOverEdge = true
                 ),
                 childAlignment = ChildAlignment(
                     offset = 0,
                     fraction = 0f
                 )
             ),
-            adapterConfiguration = getDefaultAdapterConfiguration().copy(numberOfItems = 5)
+            adapterConfiguration = getDefaultAdapterConfiguration().copy(numberOfItems = 4)
         )
 
-        KeyEvents.pressRight(times = 4)
+        KeyEvents.pressRight(times = 3)
         waitForIdleScrollState()
-        val childBounds = getItemViewBounds(position = 4)
+        val childBounds = getItemViewBounds(position = 3)
         onRecyclerView("Request layout") { recyclerView ->
             recyclerView.requestLayout()
         }
         Espresso.onIdle()
-        assertThat(getItemViewBounds(position = 4)).isEqualTo(childBounds)
+        assertThat(getItemViewBounds(position = 3)).isEqualTo(childBounds)
     }
 
     @Test

dpadrecyclerview/src/androidTest/kotlin/com/rubensousa/dpadrecyclerview/test/tests/alignment/VerticalAlignmentTest.kt

@@ -202,7 +202,8 @@ class VerticalAlignmentTest : DpadRecyclerViewTest() {
             ParentAlignment(
                 edge = Edge.MAX,
                 offset = 0,
-                fraction = 0.5f
+                fraction = 0.5f,
+                preferKeylineOverEdge = true
             )
         )
 
@@ -360,28 +361,13 @@ class VerticalAlignmentTest : DpadRecyclerViewTest() {
         assertThat(viewBounds.centerY()).isEqualTo(getRecyclerViewBounds().centerY())
     }
 
-    @Test
-    fun testLayoutAlignsToMaxEdgeWhenThereAreNotManyItems() {
-        val parentAlignment = ParentAlignment(
-            edge = Edge.MAX,
-            offset = 0,
-            fraction = 0.5f,
-            preferKeylineOverEdge = false
-        )
-        launchFragment(
-            getDefaultLayoutConfiguration().copy(parentAlignment = parentAlignment),
-            getDefaultAdapterConfiguration().copy(numberOfItems = 3)
-        )
-        val viewBounds = getItemViewBounds(position = 2)
-        assertThat(viewBounds.bottom).isEqualTo(getRecyclerViewBounds().bottom)
-    }
-
     @Test
     fun testLayoutAlignsToKeylineInsteadOfMaxEdgeWhenThereAreNotManyItems() {
         val parentAlignment = ParentAlignment(
             edge = Edge.MAX,
             offset = 0,
-            fraction = 0.5f
+            fraction = 0.5f,
+            preferKeylineOverEdge = true
         )
         launchFragment(
             getDefaultLayoutConfiguration().copy(parentAlignment = parentAlignment),

dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/alignment/ParentAlignmentCalculator.kt

@@ -82,7 +82,11 @@ internal class ParentAlignmentCalculator {
         endScrollLimit = Int.MAX_VALUE
     }
 
-    fun updateStartLimit(edge: Int, viewAnchor: Int, alignment: ParentAlignment) {
+    fun updateStartLimit(
+        edge: Int,
+        viewAnchor: Int,
+        alignment: ParentAlignment
+    ) {
         startEdge = edge
         if (isStartUnknown) {
             startScrollLimit = Int.MIN_VALUE
@@ -91,22 +95,30 @@ internal class ParentAlignmentCalculator {
         val keyLine = calculateKeyline(alignment)
         startScrollLimit = if (shouldAlignViewToStart(viewAnchor, keyLine, alignment)) {
             calculateScrollOffsetToStartEdge(edge)
-        } else {
+        } else if (!isLayoutIncomplete() || alignment.preferKeylineOverEdge) {
             calculateScrollOffsetToKeyline(viewAnchor, keyLine)
+        } else {
+            0
         }
     }
 
-    fun updateEndLimit(edge: Int, viewAnchor: Int, alignment: ParentAlignment) {
-        this.endEdge = edge
+    fun updateEndLimit(
+        edge: Int,
+        viewAnchor: Int,
+        alignment: ParentAlignment,
+    ) {
+        endEdge = edge
         if (isEndUnknown) {
             endScrollLimit = Int.MAX_VALUE
             return
         }
         val keyline = calculateKeyline(alignment)
         endScrollLimit = if (shouldAlignViewToEnd(viewAnchor, keyline, alignment)) {
             calculateScrollOffsetToEndEdge(edge)
-        } else {
+        } else if (!isLayoutComplete() || alignment.preferKeylineOverEdge) {
             calculateScrollOffsetToKeyline(viewAnchor, keyline)
+        } else {
+            0
         }
     }
 
@@ -123,7 +135,10 @@ internal class ParentAlignmentCalculator {
      * Item will either be aligned to the keyline position or to either min or max edges
      * according to the current [alignment].
      */
-    fun calculateScrollOffset(viewAnchor: Int, alignment: ParentAlignment): Int {
+    fun calculateScrollOffset(
+        viewAnchor: Int,
+        alignment: ParentAlignment
+    ): Int {
         val keyline = calculateKeyline(alignment)
         val alignToStartEdge = shouldAlignViewToStart(viewAnchor, keyline, alignment)
         val alignToEndEdge = shouldAlignViewToEnd(viewAnchor, keyline, alignment)
@@ -171,12 +186,10 @@ internal class ParentAlignmentCalculator {
         if (isStartUnknown || !shouldAlignToStartEdge(alignment.edge)) {
             return false
         }
-        if (alignment.preferKeylineOverEdge
-            && isStartEdge(alignment.edge)
-            && (startEdge >= getLayoutStartEdge() && !isEndUnknown)) {
-            return false
+        if (!isLayoutIncomplete()) {
+            return viewAnchor + getLayoutStartEdge() <= startEdge + keyline
         }
-        return viewAnchor + getLayoutStartEdge() <= startEdge + keyline
+        return isLayoutIncomplete() && !alignment.preferKeylineOverEdge
     }
 
     private fun shouldAlignViewToEnd(
@@ -187,12 +200,10 @@ internal class ParentAlignmentCalculator {
         if (isEndUnknown || !shouldAlignToEndEdge(alignment.edge)) {
             return false
         }
-        if (alignment.preferKeylineOverEdge
-            && isEndEdge(alignment.edge)
-            && (endEdge <= getLayoutEndEdge() && !isStartUnknown)) {
-            return false
+        if (!isLayoutIncomplete()) {
+            return viewAnchor + getLayoutEndEdge() >= endEdge + keyline
         }
-        return viewAnchor + getLayoutEndEdge() >= endEdge + keyline
+        return isLayoutIncomplete() && !alignment.preferKeylineOverEdge
     }
 
     private fun calculateScrollOffsetToKeyline(anchor: Int, keyline: Int): Int {
@@ -207,6 +218,23 @@ internal class ParentAlignmentCalculator {
         return paddingStart
     }
 
+    private fun isLayoutComplete(): Boolean {
+        return endEdge - startEdge >= size - paddingEnd - paddingStart
+                && endEdge <= size - paddingEnd
+                && startEdge >= paddingStart
+    }
+
+    private fun isLayoutIncomplete(): Boolean {
+        if (isEndUnknown || isStartUnknown) {
+            return false
+        }
+        return if (!reverseLayout) {
+            endEdge < size - paddingEnd
+        } else {
+            startEdge > paddingStart
+        }
+    }
+
     private fun isStartEdge(edge: Edge): Boolean {
         return (!reverseLayout && edge == Edge.MIN) || (reverseLayout && edge == Edge.MAX)
     }

dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/layout/StructureEngineer.kt

@@ -627,26 +627,69 @@ internal abstract class StructureEngineer(
                 return true
             }
         }
+        /**
+         * Scenario: user selected a max edge alignment
+         *
+         * If both the start and end of the layout are inside the viewport
+         * and [ParentAlignment.preferKeylineOverEdge] is false, then fill the required space
+         * to collapse to the max edge.
+         */
         if (edge == ParentAlignment.Edge.MAX) {
             if (!layoutRequest.reverseLayout && endEdge <= layoutInfo.getEndAfterPadding()) {
                 if (startEdge >= layoutInfo.getStartAfterPadding() && preferKeylineOverEdge) {
                     return false
                 }
-                val distanceToEnd = layoutInfo.getEndAfterPadding() - endEdge
-                scrollBy(-distanceToEnd - remainingScroll, recycler, state, false)
+                fixEndGap(startEdge, endEdge, startView, remainingScroll, recycler, state)
                 return true
             } else if (layoutRequest.reverseLayout && startEdge >= layoutInfo.getStartAfterPadding()) {
                 if (endEdge <= layoutInfo.getEndAfterPadding() && preferKeylineOverEdge) {
                     return false
                 }
-                val distanceToStart = startEdge - layoutInfo.getStartAfterPadding()
-                scrollBy(distanceToStart - remainingScroll, recycler, state, false)
+                fixStartGap(startEdge, endEdge, endView, remainingScroll, recycler, state)
                 return true
             }
         }
         return false
     }
 
+    private fun fixEndGap(
+        startEdge: Int,
+        endEdge: Int,
+        startView: View,
+        remainingScroll: Int,
+        recycler: RecyclerView.Recycler,
+        state: RecyclerView.State
+    ) {
+        val distanceToEndEdge = max(0, layoutInfo.getEndAfterPadding() - endEdge)
+        layoutRequest.prepend(layoutInfo.getLayoutPositionOf(startView)) {
+            setCheckpoint(startEdge)
+            setFillSpace(distanceToEndEdge)
+        }
+        val newStartSpace = fill(layoutRequest, recyclerViewProvider, recycler, state)
+        var scrollOffset = -newStartSpace + min(0, startEdge) // Include the start edge if negative
+        scrollOffset = max(-distanceToEndEdge, scrollOffset)
+        scrollBy(scrollOffset - remainingScroll, recycler, state, recycleChildren = false)
+    }
+
+    private fun fixStartGap(
+        startEdge: Int,
+        endEdge: Int,
+        endView: View,
+        remainingScroll: Int,
+        recycler: RecyclerView.Recycler,
+        state: RecyclerView.State
+    ) {
+        val distanceToStart = max(0, startEdge - layoutInfo.getStartAfterPadding())
+        layoutRequest.append(layoutInfo.getLayoutPositionOf(endView)) {
+            setCheckpoint(endEdge)
+            setFillSpace(distanceToStart)
+        }
+        val newEndSpace = fill(layoutRequest, recyclerViewProvider, recycler, state)
+        var scrollOffset = newEndSpace + max(0, endEdge - layoutInfo.getEndAfterPadding())
+        scrollOffset = min(distanceToStart, scrollOffset)
+        scrollBy(scrollOffset - remainingScroll, recycler, state, recycleChildren = false)
+    }
+
     protected fun addView(view: View, layoutRequest: LayoutRequest) {
         if (!layoutRequest.isLayingOutScrap) {
             if (layoutRequest.isAppending()) {

dpadrecyclerview/src/main/res/values/attrs.xml

@@ -21,7 +21,7 @@
         <attr name="dpadRecyclerViewParentAlignmentOffset" format="dimension" />
         <attr name="dpadRecyclerViewParentAlignmentFraction" format="float" />
         <attr name="dpadRecyclerViewParentAlignmentFractionEnabled" format="boolean" />
-        <attr name="dpadRecyclerViewParentAlignmentPreferKeylineOverEdge" format="dimension" />
+        <attr name="dpadRecyclerViewParentAlignmentPreferKeylineOverEdge" format="boolean" />
         <attr name="dpadRecyclerViewFocusableDirection" format="enum">
             <enum name="standard" value="0" />
             <enum name="circular" value="1" />

dpadrecyclerview/src/test/java/com/rubensousa/dpadrecyclerview/test/layoutmanager/alignment/ParentAlignmentCalculatorTest.kt

@@ -328,6 +328,32 @@ class ParentAlignmentCalculatorTest {
         ).isEqualTo(distanceToKeyline)
     }
 
+    @Test
+    fun `end scroll limit should be zero when layout is not completely filled for max edge`() {
+        setLayoutProperties(orientation = RecyclerView.HORIZONTAL, reverseLayout = false)
+
+        val alignment = ParentAlignment(
+            edge = ParentAlignment.Edge.MAX,
+            offset = 0,
+            fraction = 0f,
+            preferKeylineOverEdge = true
+        )
+
+        alignmentCalculator.updateStartLimit(
+            edge = 0,
+            viewAnchor = 0,
+            alignment = alignment
+        )
+
+        alignmentCalculator.updateEndLimit(
+            edge = horizontalViewWidth * 3,
+            viewAnchor = 0,
+            alignment = alignment
+        )
+
+        assertThat(alignmentCalculator.endScrollLimit).isEqualTo(0)
+    }
+
     private fun setLayoutProperties(orientation: Int, reverseLayout: Boolean) {
         if (orientation == RecyclerView.VERTICAL) {
             alignmentCalculator.updateLayoutInfo(

dpadrecyclerview/src/test/java/com/rubensousa/dpadrecyclerview/test/layoutmanager/mock/LayoutManagerMock.kt

@@ -70,6 +70,8 @@ internal class LayoutManagerMock(
             }?.view
         }
         every { mock.paddingLeft }.answers { leftPadding }
+        every { mock.paddingStart }.answers { leftPadding }
+        every { mock.paddingEnd }.answers { rightPadding }
         every { mock.paddingTop }.answers { topPadding }
         every { mock.paddingRight }.answers { rightPadding }
         every { mock.paddingBottom }.answers { bottomPadding }

gradle.properties

@@ -22,4 +22,4 @@ kotlin.code.style=official
 # thereby reducing the size of the R class for that library
 android.nonTransitiveRClass=true
 android.enableR8.fullMode=true
-LIBRARY_VERSION=1.1.0-alpha03
+LIBRARY_VERSION=1.1.0-beta01

sample/src/main/java/com/rubensousa/dpadrecyclerview/sample/ui/screen/list/HorizontalListViewHolder.kt

@@ -52,6 +52,9 @@ class HorizontalListViewHolder(
             DpadLinearSpacingDecoration.create(
                 itemSpacing = itemView.resources.getDimensionPixelOffset(
                     R.dimen.horizontal_item_spacing
+                ),
+                edgeSpacing = itemView.resources.getDimensionPixelOffset(
+                    R.dimen.list_margin_start
                 )
             )
         )

sample/src/main/res/layout/horizontal_adapter_list.xml

@@ -18,7 +18,6 @@
         android:textSize="14sp"
         tools:text="Headline" />
 
-
     <com.rubensousa.dpadrecyclerview.DpadRecyclerView
         android:id="@+id/recyclerView"
         android:layout_width="match_parent"
@@ -32,7 +31,8 @@
         app:dpadRecyclerViewFocusOutFront="false"
         app:dpadRecyclerViewParentAlignmentEdge="max"
         app:dpadRecyclerViewParentAlignmentFraction="0"
-        app:dpadRecyclerViewParentAlignmentOffset="@dimen/list_margin_start" />
+        app:dpadRecyclerViewParentAlignmentOffset="@dimen/list_margin_start"
+        app:dpadRecyclerViewParentAlignmentPreferKeylineOverEdge="false" />
 
 
 </LinearLayout>