From 1d1409ea8e6d3991de5030838abeb229e1d3118b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=BAben=20Sousa?= Date: Fri, 22 Dec 2023 11:56:36 +0000 Subject: [PATCH 1/5] Fix alignment regression for some configurations --- .../alignment/HorizontalAlignmentTest.kt | 28 +++++++++++++++++++ .../alignment/ParentAlignmentCalculator.kt | 19 +++++-------- 2 files changed, 35 insertions(+), 12 deletions(-) diff --git a/dpadrecyclerview/src/androidTest/kotlin/com/rubensousa/dpadrecyclerview/test/tests/alignment/HorizontalAlignmentTest.kt b/dpadrecyclerview/src/androidTest/kotlin/com/rubensousa/dpadrecyclerview/test/tests/alignment/HorizontalAlignmentTest.kt index f6e7d47d..8214d5c1 100644 --- a/dpadrecyclerview/src/androidTest/kotlin/com/rubensousa/dpadrecyclerview/test/tests/alignment/HorizontalAlignmentTest.kt +++ b/dpadrecyclerview/src/androidTest/kotlin/com/rubensousa/dpadrecyclerview/test/tests/alignment/HorizontalAlignmentTest.kt @@ -416,4 +416,32 @@ class HorizontalAlignmentTest : DpadRecyclerViewTest() { } assertThat(getItemViewBounds(position = 0)).isEqualTo(childBounds) } + + @Test + fun testFirstItemIsAlignedCorrectlyWhenScrollingBack() { + launchFragment( + layoutConfiguration = getDefaultLayoutConfiguration().copy( + parentAlignment = ParentAlignment( + edge = Edge.MAX, + offset = 200, + fraction = 0f, + preferKeylineOverEdge = false + ), + childAlignment = ChildAlignment( + offset = 0, + fraction = 0f + ) + ), + adapterConfiguration = getDefaultAdapterConfiguration() + ) + + val childBounds = getItemViewBounds(position = 0) + assertThat(childBounds.left).isEqualTo(200) + KeyEvents.pressRight() + waitForIdleScrollState() + KeyEvents.pressLeft() + waitForIdleScrollState() + waitForIdleScrollState() + assertThat(getItemViewBounds(position = 0)).isEqualTo(childBounds) + } } diff --git a/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/alignment/ParentAlignmentCalculator.kt b/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/alignment/ParentAlignmentCalculator.kt index b0aef227..f7fe3739 100644 --- a/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/alignment/ParentAlignmentCalculator.kt +++ b/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/alignment/ParentAlignmentCalculator.kt @@ -97,7 +97,7 @@ internal class ParentAlignmentCalculator { calculateScrollOffsetToStartEdge(edge) } else if (isLayoutComplete() || alignment.preferKeylineOverEdge - || alignment.edge == Edge.NONE + || !shouldAlignToStartEdge(alignment.edge) ) { calculateScrollOffsetToKeyline(viewAnchor, keyLine) } else { @@ -120,7 +120,7 @@ internal class ParentAlignmentCalculator { calculateScrollOffsetToEndEdge(edge) } else if (isLayoutComplete() || alignment.preferKeylineOverEdge - || alignment.edge == Edge.NONE + || !shouldAlignToEndEdge(alignment.edge) ) { calculateScrollOffsetToKeyline(viewAnchor, keyline) } else { @@ -192,7 +192,7 @@ internal class ParentAlignmentCalculator { if (isStartUnknown || !shouldAlignToStartEdge(alignment.edge)) { return false } - if (!isLayoutIncomplete()) { + if (isLayoutComplete()) { return viewAnchor + getLayoutStartEdge() <= startEdge + keyline } return isLayoutIncomplete() && !alignment.preferKeylineOverEdge @@ -206,7 +206,7 @@ internal class ParentAlignmentCalculator { if (isEndUnknown || !shouldAlignToEndEdge(alignment.edge)) { return false } - if (!isLayoutIncomplete()) { + if (isLayoutComplete()) { return viewAnchor + getLayoutEndEdge() >= endEdge + keyline } return isLayoutIncomplete() && !alignment.preferKeylineOverEdge @@ -225,12 +225,7 @@ internal class ParentAlignmentCalculator { } private fun isLayoutComplete(): Boolean { - if (isEndUnknown || isStartUnknown) { - return false - } - return endEdge - startEdge >= size - paddingEnd - paddingStart - && endEdge <= size - paddingEnd - && startEdge >= paddingStart + return endEdge >= getLayoutEndEdge() && startEdge <= getLayoutStartEdge() } private fun isLayoutIncomplete(): Boolean { @@ -238,9 +233,9 @@ internal class ParentAlignmentCalculator { return false } return if (!reverseLayout) { - endEdge < size - paddingEnd + endEdge < getLayoutEndEdge() } else { - startEdge > paddingStart + startEdge > getLayoutStartEdge() } } From 6bd138ec7531364358b1f62608767e3780b25bd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=BAben=20Sousa?= Date: Sat, 23 Dec 2023 01:48:16 +0000 Subject: [PATCH 2/5] Adjust parent alignment calculations for incomplete layouts --- .../alignment/LayoutAlignment.kt | 35 ++-- .../alignment/ParentAlignmentCalculator.kt | 108 ++++++------ .../ParentAlignmentCalculatorTest.kt | 160 +++++++++--------- 3 files changed, 154 insertions(+), 149 deletions(-) diff --git a/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/alignment/LayoutAlignment.kt b/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/alignment/LayoutAlignment.kt index c21571cb..1b8907bf 100644 --- a/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/alignment/LayoutAlignment.kt +++ b/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/alignment/LayoutAlignment.kt @@ -27,7 +27,7 @@ import kotlin.math.sign internal class LayoutAlignment( private val layoutManager: LayoutManager, - private val layoutInfo: LayoutInfo + private val layoutInfo: LayoutInfo, ) { companion object { @@ -242,40 +242,41 @@ internal class LayoutAlignment( startViewAnchor = Int.MIN_VALUE } if (!reverseLayout) { + parentAlignmentCalculator.updateScrollLimits( + startEdge = startEdge, + endEdge = endEdge, + startViewAnchor = startViewAnchor, + endViewAnchor = endViewAnchor, + alignment = parentAlignment + ) if (layoutInfo.isLoopingAllowed) { // If we're looping, there's no end scroll limit parentAlignmentCalculator.invalidateEndLimit() - } else { - parentAlignmentCalculator.updateEndLimit(endEdge, endViewAnchor, parentAlignment) } if (layoutInfo.isLoopingStart) { parentAlignmentCalculator.invalidateStartLimit() - } else { - parentAlignmentCalculator.updateStartLimit( - startEdge, startViewAnchor, parentAlignment - ) } } else { + parentAlignmentCalculator.updateScrollLimits( + startEdge = endEdge, + endEdge = startEdge, + startViewAnchor = endViewAnchor, + endViewAnchor = startViewAnchor, + alignment = parentAlignment + ) if (layoutInfo.isLoopingAllowed) { parentAlignmentCalculator.invalidateStartLimit() - } else { - parentAlignmentCalculator.updateStartLimit(endEdge, endViewAnchor, parentAlignment) } if (layoutInfo.isLoopingStart) { parentAlignmentCalculator.invalidateEndLimit() - } else { - parentAlignmentCalculator.updateEndLimit( - startEdge, startViewAnchor, parentAlignment - ) } - } } private fun isEndAvailable( adapterPosition: Int, maxLayoutPosition: Int, - minLayoutPosition: Int + minLayoutPosition: Int, ): Boolean { return if (!reverseLayout) { adapterPosition == maxLayoutPosition @@ -287,7 +288,7 @@ internal class LayoutAlignment( private fun isStartAvailable( adapterPosition: Int, maxLayoutPosition: Int, - minLayoutPosition: Int + minLayoutPosition: Int, ): Boolean { return if (!reverseLayout) { adapterPosition == minLayoutPosition @@ -343,7 +344,7 @@ internal class LayoutAlignment( private fun calculateAdjustedAlignedScrollDistance( offset: Int, view: View, - childView: View + childView: View, ): Int { var scrollValue = offset val subPosition = getSubPositionOfView(view, childView) diff --git a/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/alignment/ParentAlignmentCalculator.kt b/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/alignment/ParentAlignmentCalculator.kt index f7fe3739..80bd763a 100644 --- a/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/alignment/ParentAlignmentCalculator.kt +++ b/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/alignment/ParentAlignmentCalculator.kt @@ -82,58 +82,60 @@ internal class ParentAlignmentCalculator { endScrollLimit = Int.MAX_VALUE } - fun updateStartLimit( - edge: Int, - viewAnchor: Int, + fun updateScrollLimits( + startEdge: Int, + endEdge: Int, + startViewAnchor: Int, + endViewAnchor: Int, alignment: ParentAlignment, ) { - startEdge = edge - if (isStartUnknown) { - startScrollLimit = Int.MIN_VALUE - return + this.startEdge = startEdge + this.endEdge = endEdge + val keyline = calculateKeyline(alignment) + startScrollLimit = when { + isStartUnknown -> Int.MIN_VALUE + shouldAlignViewToStart(startViewAnchor, keyline, alignment) -> { + calculateScrollOffsetToStartEdge(startEdge) + } + + shouldAlignStartToKeyline(alignment) -> { + calculateScrollOffsetToKeyline(startViewAnchor, keyline) + } + + else -> 0 } - val keyLine = calculateKeyline(alignment) - startScrollLimit = if (shouldAlignViewToStart(viewAnchor, keyLine, alignment)) { - calculateScrollOffsetToStartEdge(edge) - } else if (isLayoutComplete() - || alignment.preferKeylineOverEdge - || !shouldAlignToStartEdge(alignment.edge) - ) { - calculateScrollOffsetToKeyline(viewAnchor, keyLine) - } else { - 0 + endScrollLimit = when { + isEndUnknown -> Int.MAX_VALUE + shouldAlignViewToEnd(endViewAnchor, keyline, alignment) -> { + calculateScrollOffsetToEndEdge(endEdge) + } + + shouldAlignEndToKeyline(alignment) -> { + calculateScrollOffsetToKeyline(endViewAnchor, keyline) + } + + else -> 0 } } - 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 if (isLayoutComplete() - || alignment.preferKeylineOverEdge - || !shouldAlignToEndEdge(alignment.edge) - ) { - calculateScrollOffsetToKeyline(viewAnchor, keyline) - } else { - 0 - } + private fun shouldAlignStartToKeyline(alignment: ParentAlignment): Boolean { + return isLayoutComplete() + || alignment.preferKeylineOverEdge + || !shouldAlignToStartEdge(alignment.edge) + } + + private fun shouldAlignEndToKeyline(alignment: ParentAlignment): Boolean { + return isLayoutComplete() + || alignment.preferKeylineOverEdge + || !shouldAlignToEndEdge(alignment.edge) } private fun calculateScrollOffsetToEndEdge(edge: Int): Int { - return edge - getLayoutEndEdge() + return edge - getLayoutAbsoluteEnd() } private fun calculateScrollOffsetToStartEdge(edge: Int): Int { - return edge - getLayoutStartEdge() + return edge - getLayoutAbsoluteStart() } /** @@ -192,8 +194,8 @@ internal class ParentAlignmentCalculator { if (isStartUnknown || !shouldAlignToStartEdge(alignment.edge)) { return false } - if (isLayoutComplete()) { - return viewAnchor + getLayoutStartEdge() <= startEdge + keyline + if (isLayoutComplete() || isLayoutCompleteInOppositeDirection()) { + return viewAnchor + getLayoutAbsoluteStart() <= startEdge + keyline } return isLayoutIncomplete() && !alignment.preferKeylineOverEdge } @@ -206,8 +208,8 @@ internal class ParentAlignmentCalculator { if (isEndUnknown || !shouldAlignToEndEdge(alignment.edge)) { return false } - if (isLayoutComplete()) { - return viewAnchor + getLayoutEndEdge() >= endEdge + keyline + if (isLayoutComplete() || isLayoutCompleteInOppositeDirection()) { + return viewAnchor + getLayoutAbsoluteEnd() >= endEdge + keyline } return isLayoutIncomplete() && !alignment.preferKeylineOverEdge } @@ -216,16 +218,24 @@ internal class ParentAlignmentCalculator { return anchor - keyline } - private fun getLayoutEndEdge(): Int { + private fun getLayoutAbsoluteEnd(): Int { return size - paddingEnd } - private fun getLayoutStartEdge(): Int { + private fun getLayoutAbsoluteStart(): Int { return paddingStart } private fun isLayoutComplete(): Boolean { - return endEdge >= getLayoutEndEdge() && startEdge <= getLayoutStartEdge() + return endEdge >= getLayoutAbsoluteEnd() && startEdge <= getLayoutAbsoluteStart() + } + + private fun isLayoutCompleteInOppositeDirection(): Boolean { + return if (!reverseLayout) { + startEdge <= getLayoutAbsoluteStart() + } else { + endEdge >= getLayoutAbsoluteEnd() + } } private fun isLayoutIncomplete(): Boolean { @@ -233,9 +243,9 @@ internal class ParentAlignmentCalculator { return false } return if (!reverseLayout) { - endEdge < getLayoutEndEdge() + endEdge < getLayoutAbsoluteEnd() } else { - startEdge > getLayoutStartEdge() + startEdge > getLayoutAbsoluteStart() } } diff --git a/dpadrecyclerview/src/test/java/com/rubensousa/dpadrecyclerview/test/layoutmanager/alignment/ParentAlignmentCalculatorTest.kt b/dpadrecyclerview/src/test/java/com/rubensousa/dpadrecyclerview/test/layoutmanager/alignment/ParentAlignmentCalculatorTest.kt index 3e60c4e2..b189df04 100644 --- a/dpadrecyclerview/src/test/java/com/rubensousa/dpadrecyclerview/test/layoutmanager/alignment/ParentAlignmentCalculatorTest.kt +++ b/dpadrecyclerview/src/test/java/com/rubensousa/dpadrecyclerview/test/layoutmanager/alignment/ParentAlignmentCalculatorTest.kt @@ -64,9 +64,11 @@ class ParentAlignmentCalculatorTest { fun `keyline for child near start edge is the start edge`() { setLayoutProperties(orientation = RecyclerView.VERTICAL, reverseLayout = false) - alignmentCalculator.updateStartLimit( - edge = 0, - viewAnchor = verticalViewHeight / 2, + alignmentCalculator.updateScrollLimits( + startEdge = 0, + endEdge = height, + startViewAnchor = verticalViewHeight / 2, + endViewAnchor = verticalViewHeight / 2, alignment = centerParentAlignment ) @@ -82,9 +84,11 @@ class ParentAlignmentCalculatorTest { fun `keyline for child near end edge is the end edge`() { setLayoutProperties(orientation = RecyclerView.VERTICAL, reverseLayout = false) - alignmentCalculator.updateEndLimit( - edge = height, - viewAnchor = height - verticalViewHeight / 2, + alignmentCalculator.updateScrollLimits( + startEdge = 0, + endEdge = height, + startViewAnchor = 0, + endViewAnchor = height - verticalViewHeight / 2, alignment = centerParentAlignment ) @@ -137,14 +141,16 @@ class ParentAlignmentCalculatorTest { fun `child is aligned to end edge in regular layout direction`() { setLayoutProperties(orientation = RecyclerView.VERTICAL, reverseLayout = false) - alignmentCalculator.updateEndLimit( - edge = height / 2 + verticalViewHeight / 2, - viewAnchor = verticalViewHeight / 2, + alignmentCalculator.updateScrollLimits( + startEdge = 0, + endEdge = height + verticalViewHeight, + startViewAnchor = verticalViewHeight / 2, + endViewAnchor = height + verticalViewHeight / 2, alignment = centerParentAlignment ) assertThat( alignmentCalculator.calculateScrollOffset( - viewAnchor = height / 2, + viewAnchor = height + verticalViewHeight / 2, alignment = centerParentAlignment ) ).isEqualTo(alignmentCalculator.endScrollLimit) @@ -155,9 +161,11 @@ class ParentAlignmentCalculatorTest { fun `child is aligned to start edge in regular layout direction`() { setLayoutProperties(orientation = RecyclerView.VERTICAL, reverseLayout = false) - alignmentCalculator.updateStartLimit( - edge = height / 2 - verticalViewHeight / 2, - viewAnchor = verticalViewHeight / 2, + alignmentCalculator.updateScrollLimits( + startEdge = height / 2 - verticalViewHeight / 2, + endEdge = height / 2 + verticalViewHeight / 2, + startViewAnchor = verticalViewHeight / 2, + endViewAnchor = verticalViewHeight / 2, alignment = centerParentAlignment ) @@ -173,9 +181,11 @@ class ParentAlignmentCalculatorTest { fun `child is aligned to end edge in reverse layout`() { setLayoutProperties(orientation = RecyclerView.VERTICAL, reverseLayout = true) - alignmentCalculator.updateEndLimit( - edge = height / 2 + verticalViewHeight / 2, - viewAnchor = verticalViewHeight / 2, + alignmentCalculator.updateScrollLimits( + startEdge = height / 2 - verticalViewHeight / 2, + endEdge = height / 2 + verticalViewHeight / 2, + startViewAnchor = verticalViewHeight / 2, + endViewAnchor = verticalViewHeight / 2, alignment = centerParentAlignment ) @@ -194,18 +204,27 @@ class ParentAlignmentCalculatorTest { fun `child is aligned to start edge in reverse layout`() { setLayoutProperties(orientation = RecyclerView.VERTICAL, reverseLayout = true) - alignmentCalculator.updateStartLimit( - edge = height / 2 - verticalViewHeight / 2, - viewAnchor = verticalViewHeight / 2, - alignment = centerParentAlignment + val alignment = ParentAlignment( + edge = ParentAlignment.Edge.MAX, + fraction = 1.0f ) - assertThat(alignmentCalculator.startScrollLimit).isEqualTo(height / 2 - verticalViewHeight / 2) + alignmentCalculator.updateScrollLimits( + startEdge = height, + endEdge = verticalViewHeight, + startViewAnchor = verticalViewHeight, + endViewAnchor = verticalViewHeight, + alignment = alignment + ) + + assertThat(alignmentCalculator.startScrollLimit).isEqualTo( + verticalViewHeight + ) assertThat( alignmentCalculator.calculateScrollOffset( - viewAnchor = height / 2, - alignment = centerParentAlignment + viewAnchor = verticalViewHeight, + alignment = alignment ) ).isEqualTo(alignmentCalculator.startScrollLimit) } @@ -216,17 +235,13 @@ class ParentAlignmentCalculatorTest { val alignment = ParentAlignment( edge = ParentAlignment.Edge.MIN, offset = 0, - fraction = 0.5f, - preferKeylineOverEdge = false - ) - alignmentCalculator.updateStartLimit( - edge = verticalCenterKeyline - verticalViewHeight / 2, - viewAnchor = verticalCenterKeyline, - alignment = alignment + fraction = 0.5f ) - alignmentCalculator.updateEndLimit( - edge = verticalCenterKeyline + verticalViewHeight / 2, - viewAnchor = verticalCenterKeyline, + alignmentCalculator.updateScrollLimits( + startEdge = verticalCenterKeyline - verticalViewHeight / 2, + startViewAnchor = verticalCenterKeyline, + endEdge = verticalCenterKeyline + verticalViewHeight / 2, + endViewAnchor = verticalCenterKeyline, alignment = alignment ) @@ -249,14 +264,12 @@ class ParentAlignmentCalculatorTest { fraction = 0.5f, preferKeylineOverEdge = false ) - alignmentCalculator.updateStartLimit( - edge = -verticalViewHeight / 2, - viewAnchor = verticalViewHeight / 2, - alignment = alignment - ) - alignmentCalculator.updateEndLimit( - edge = verticalCenterKeyline + verticalViewHeight, - viewAnchor = verticalCenterKeyline + verticalViewHeight / 2, + + alignmentCalculator.updateScrollLimits( + startEdge = -verticalViewHeight / 2, + endEdge = verticalCenterKeyline + verticalViewHeight, + startViewAnchor = verticalViewHeight / 2, + endViewAnchor = verticalCenterKeyline + verticalViewHeight / 2, alignment = alignment ) @@ -277,25 +290,19 @@ class ParentAlignmentCalculatorTest { fraction = 0.5f, preferKeylineOverEdge = true ) - alignmentCalculator.updateStartLimit( - edge = verticalCenterKeyline - verticalViewHeight / 2, - viewAnchor = verticalCenterKeyline, + alignmentCalculator.updateScrollLimits( + startEdge = verticalCenterKeyline - verticalViewHeight / 2, + endEdge = verticalCenterKeyline + verticalViewHeight / 2, + startViewAnchor = verticalCenterKeyline, + endViewAnchor = verticalCenterKeyline, alignment = alignment ) - alignmentCalculator.updateEndLimit( - edge = verticalCenterKeyline + verticalViewHeight / 2, - viewAnchor = verticalCenterKeyline, - alignment = alignment - ) - - val distanceToKeyline = 0 - assertThat( alignmentCalculator.calculateScrollOffset( viewAnchor = height / 2, alignment = alignment ) - ).isEqualTo(distanceToKeyline) + ).isEqualTo(0) } @Test @@ -307,14 +314,11 @@ class ParentAlignmentCalculatorTest { fraction = 0.5f, preferKeylineOverEdge = true ) - alignmentCalculator.updateStartLimit( - edge = 0, - viewAnchor = verticalCenterKeyline, - alignment = alignment - ) - alignmentCalculator.updateEndLimit( - edge = verticalCenterKeyline + verticalViewHeight / 2, - viewAnchor = verticalCenterKeyline, + alignmentCalculator.updateScrollLimits( + startEdge = 0, + startViewAnchor = verticalCenterKeyline, + endEdge = verticalCenterKeyline + verticalViewHeight / 2, + endViewAnchor = verticalCenterKeyline, alignment = alignment ) @@ -338,16 +342,11 @@ class ParentAlignmentCalculatorTest { fraction = 0f, preferKeylineOverEdge = true ) - - alignmentCalculator.updateStartLimit( - edge = 0, - viewAnchor = 0, - alignment = alignment - ) - - alignmentCalculator.updateEndLimit( - edge = horizontalViewWidth * 3, - viewAnchor = 0, + alignmentCalculator.updateScrollLimits( + startEdge = 0, + endEdge = horizontalViewWidth * 3, + startViewAnchor = 0, + endViewAnchor = 0, alignment = alignment ) @@ -364,22 +363,17 @@ class ParentAlignmentCalculatorTest { fraction = 0.5f, ) - val viewAnchor = verticalCenterKeyline + verticalViewHeight / 2 - alignmentCalculator.updateStartLimit( - edge = 0, - viewAnchor = viewAnchor, - alignment = alignment - ) + val keyline = verticalCenterKeyline - alignmentCalculator.updateEndLimit( - edge = height - verticalViewHeight, - viewAnchor = height - verticalViewHeight / 2, + alignmentCalculator.updateScrollLimits( + startEdge = keyline - verticalViewHeight, + startViewAnchor = keyline - verticalViewHeight / 2, + endEdge = keyline, + endViewAnchor = keyline - verticalViewHeight / 2, alignment = alignment ) - assertThat(alignmentCalculator.startScrollLimit).isEqualTo( - viewAnchor - verticalCenterKeyline, - ) + assertThat(alignmentCalculator.startScrollLimit).isEqualTo(-verticalViewHeight / 2) } private fun setLayoutProperties(orientation: Int, reverseLayout: Boolean) { From e4de0249b14501c5c5ff98feb21ed8c7564bc020 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=BAben=20Sousa?= Date: Wed, 27 Dec 2023 00:35:33 +0000 Subject: [PATCH 3/5] Fix alignment regression in reverse layout --- .../alignment/ParentAlignmentCalculator.kt | 43 ++++++++----------- .../ParentAlignmentCalculatorTest.kt | 13 +++--- 2 files changed, 24 insertions(+), 32 deletions(-) diff --git a/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/alignment/ParentAlignmentCalculator.kt b/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/alignment/ParentAlignmentCalculator.kt index 80bd763a..59e4e8cf 100644 --- a/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/alignment/ParentAlignmentCalculator.kt +++ b/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/alignment/ParentAlignmentCalculator.kt @@ -119,23 +119,21 @@ internal class ParentAlignmentCalculator { } private fun shouldAlignStartToKeyline(alignment: ParentAlignment): Boolean { - return isLayoutComplete() - || alignment.preferKeylineOverEdge - || !shouldAlignToStartEdge(alignment.edge) + return (isLayoutComplete() && !shouldAlignToStartEdge(alignment.edge)) + || preferKeylineOverEdge(alignment) } private fun shouldAlignEndToKeyline(alignment: ParentAlignment): Boolean { - return isLayoutComplete() - || alignment.preferKeylineOverEdge - || !shouldAlignToEndEdge(alignment.edge) + return (isLayoutComplete() && !shouldAlignToEndEdge(alignment.edge)) + || preferKeylineOverEdge(alignment) } - private fun calculateScrollOffsetToEndEdge(edge: Int): Int { - return edge - getLayoutAbsoluteEnd() + private fun calculateScrollOffsetToEndEdge(anchor: Int): Int { + return anchor - getLayoutAbsoluteEnd() } - private fun calculateScrollOffsetToStartEdge(edge: Int): Int { - return edge - getLayoutAbsoluteStart() + private fun calculateScrollOffsetToStartEdge(anchor: Int): Int { + return anchor - getLayoutAbsoluteStart() } /** @@ -194,10 +192,10 @@ internal class ParentAlignmentCalculator { if (isStartUnknown || !shouldAlignToStartEdge(alignment.edge)) { return false } - if (isLayoutComplete() || isLayoutCompleteInOppositeDirection()) { + if (isLayoutComplete()) { return viewAnchor + getLayoutAbsoluteStart() <= startEdge + keyline } - return isLayoutIncomplete() && !alignment.preferKeylineOverEdge + return isLayoutStartKnown() && !preferKeylineOverEdge(alignment) } private fun shouldAlignViewToEnd( @@ -208,10 +206,10 @@ internal class ParentAlignmentCalculator { if (isEndUnknown || !shouldAlignToEndEdge(alignment.edge)) { return false } - if (isLayoutComplete() || isLayoutCompleteInOppositeDirection()) { + if (isLayoutComplete()) { return viewAnchor + getLayoutAbsoluteEnd() >= endEdge + keyline } - return isLayoutIncomplete() && !alignment.preferKeylineOverEdge + return isLayoutStartKnown() && !preferKeylineOverEdge(alignment) } private fun calculateScrollOffsetToKeyline(anchor: Int, keyline: Int): Int { @@ -230,22 +228,15 @@ internal class ParentAlignmentCalculator { return endEdge >= getLayoutAbsoluteEnd() && startEdge <= getLayoutAbsoluteStart() } - private fun isLayoutCompleteInOppositeDirection(): Boolean { - return if (!reverseLayout) { - startEdge <= getLayoutAbsoluteStart() - } else { - endEdge >= getLayoutAbsoluteEnd() - } + private fun preferKeylineOverEdge(alignment: ParentAlignment): Boolean { + return alignment.preferKeylineOverEdge || alignment.edge == Edge.NONE } - private fun isLayoutIncomplete(): Boolean { - if (isEndUnknown || isStartUnknown) { - return false - } + private fun isLayoutStartKnown(): Boolean { return if (!reverseLayout) { - endEdge < getLayoutAbsoluteEnd() + !isStartUnknown } else { - startEdge > getLayoutAbsoluteStart() + !isEndUnknown } } diff --git a/dpadrecyclerview/src/test/java/com/rubensousa/dpadrecyclerview/test/layoutmanager/alignment/ParentAlignmentCalculatorTest.kt b/dpadrecyclerview/src/test/java/com/rubensousa/dpadrecyclerview/test/layoutmanager/alignment/ParentAlignmentCalculatorTest.kt index b189df04..64c5f681 100644 --- a/dpadrecyclerview/src/test/java/com/rubensousa/dpadrecyclerview/test/layoutmanager/alignment/ParentAlignmentCalculatorTest.kt +++ b/dpadrecyclerview/src/test/java/com/rubensousa/dpadrecyclerview/test/layoutmanager/alignment/ParentAlignmentCalculatorTest.kt @@ -235,7 +235,8 @@ class ParentAlignmentCalculatorTest { val alignment = ParentAlignment( edge = ParentAlignment.Edge.MIN, offset = 0, - fraction = 0.5f + fraction = 0.5f, + preferKeylineOverEdge = false ) alignmentCalculator.updateScrollLimits( startEdge = verticalCenterKeyline - verticalViewHeight / 2, @@ -278,7 +279,7 @@ class ParentAlignmentCalculatorTest { viewAnchor = verticalCenterKeyline + verticalViewHeight / 2, alignment = alignment ) - ).isEqualTo(alignmentCalculator.endScrollLimit) + ).isEqualTo(-(height - (verticalCenterKeyline + verticalViewHeight))) } @Test @@ -333,24 +334,24 @@ class ParentAlignmentCalculatorTest { } @Test - fun `end scroll limit should be zero when layout is not completely filled for max edge`() { + fun `end scroll limit should be limited to distance to end edge when layout is not completely filled`() { setLayoutProperties(orientation = RecyclerView.HORIZONTAL, reverseLayout = false) val alignment = ParentAlignment( edge = ParentAlignment.Edge.MAX, offset = 0, fraction = 0f, - preferKeylineOverEdge = true + preferKeylineOverEdge = false ) alignmentCalculator.updateScrollLimits( startEdge = 0, endEdge = horizontalViewWidth * 3, startViewAnchor = 0, - endViewAnchor = 0, + endViewAnchor = horizontalViewWidth * 2, alignment = alignment ) - assertThat(alignmentCalculator.endScrollLimit).isEqualTo(0) + assertThat(alignmentCalculator.endScrollLimit).isEqualTo(horizontalViewWidth * 3 - width) } @Test From 750c92ec5fc81c8120e97fa19a93684fef61c20c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=BAben=20Sousa?= Date: Wed, 27 Dec 2023 01:20:25 +0000 Subject: [PATCH 4/5] Fix layout completeness check --- .../alignment/ParentAlignmentCalculator.kt | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/alignment/ParentAlignmentCalculator.kt b/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/alignment/ParentAlignmentCalculator.kt index 59e4e8cf..fb90ff61 100644 --- a/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/alignment/ParentAlignmentCalculator.kt +++ b/dpadrecyclerview/src/main/java/com/rubensousa/dpadrecyclerview/layoutmanager/alignment/ParentAlignmentCalculator.kt @@ -119,13 +119,11 @@ internal class ParentAlignmentCalculator { } private fun shouldAlignStartToKeyline(alignment: ParentAlignment): Boolean { - return (isLayoutComplete() && !shouldAlignToStartEdge(alignment.edge)) - || preferKeylineOverEdge(alignment) + return !shouldAlignToStartEdge(alignment.edge) || preferKeylineOverEdge(alignment) } private fun shouldAlignEndToKeyline(alignment: ParentAlignment): Boolean { - return (isLayoutComplete() && !shouldAlignToEndEdge(alignment.edge)) - || preferKeylineOverEdge(alignment) + return !shouldAlignToEndEdge(alignment.edge) || preferKeylineOverEdge(alignment) } private fun calculateScrollOffsetToEndEdge(anchor: Int): Int { @@ -225,7 +223,16 @@ internal class ParentAlignmentCalculator { } private fun isLayoutComplete(): Boolean { - return endEdge >= getLayoutAbsoluteEnd() && startEdge <= getLayoutAbsoluteStart() + if (isEndUnknown && isStartUnknown) { + return true + } + return if (!reverseLayout) { + (startEdge <= getLayoutAbsoluteStart() + && (endEdge >= getLayoutAbsoluteEnd() || isEndUnknown)) + } else { + (endEdge >= getLayoutAbsoluteEnd() + && (startEdge <= getLayoutAbsoluteStart() || isStartUnknown)) + } } private fun preferKeylineOverEdge(alignment: ParentAlignment): Boolean { From eb93af54df1c51380df86fad7068a4821af30832 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=BAben=20Sousa?= Date: Wed, 27 Dec 2023 01:31:17 +0000 Subject: [PATCH 5/5] Simplify UI test --- .../test/tests/alignment/VerticalAlignmentTest.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dpadrecyclerview/src/androidTest/kotlin/com/rubensousa/dpadrecyclerview/test/tests/alignment/VerticalAlignmentTest.kt b/dpadrecyclerview/src/androidTest/kotlin/com/rubensousa/dpadrecyclerview/test/tests/alignment/VerticalAlignmentTest.kt index baebb528..94d4c3c7 100644 --- a/dpadrecyclerview/src/androidTest/kotlin/com/rubensousa/dpadrecyclerview/test/tests/alignment/VerticalAlignmentTest.kt +++ b/dpadrecyclerview/src/androidTest/kotlin/com/rubensousa/dpadrecyclerview/test/tests/alignment/VerticalAlignmentTest.kt @@ -251,14 +251,15 @@ class VerticalAlignmentTest : DpadRecyclerViewTest() { fraction = 0f ) ) - KeyEvents.pressDown(times = 5) val recyclerViewBounds = getRecyclerViewBounds() val startPosition = 5 + selectPosition(startPosition) repeat(5) { val viewBounds = getItemViewBounds(position = startPosition + it) assertThat(viewBounds.top) .isEqualTo(recyclerViewBounds.top + containerOffset + abs(itemOffset)) KeyEvents.pressDown() + waitForIdleScrollState() } }