Accept negative offsets in LazyGridState scrolling
Similar to Iceb907f268a19db3e9315154ebd136764ad975a3 for lists.
Relnote: Now it is allowed to pass negative scroll offsets into LazyGridState.scrollToItem() and LazyGridState.animateScrollToItem().
Test: LazyScrollTest
Bug: 211753558
Change-Id: I025c608ce2eef36f90ad657bba78229b62bcd725
diff --git a/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/grid/LazyScrollTest.kt b/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/grid/LazyScrollTest.kt
index 6a697ff..3afdbf4 100644
--- a/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/grid/LazyScrollTest.kt
+++ b/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/grid/LazyScrollTest.kt
@@ -27,10 +27,9 @@
import androidx.compose.foundation.lazy.GridCells
import androidx.compose.foundation.lazy.LazyGridState
import androidx.compose.foundation.lazy.LazyVerticalGrid
-import androidx.compose.foundation.lazy.items
import androidx.compose.foundation.lazy.rememberLazyGridState
import androidx.compose.ui.test.junit4.createComposeRule
-import androidx.compose.ui.unit.dp
+import androidx.compose.ui.unit.Dp
import androidx.test.filters.MediumTest
import com.google.common.truth.Truth.assertThat
import kotlinx.coroutines.Dispatchers
@@ -39,7 +38,6 @@
import org.junit.Before
import org.junit.Rule
import org.junit.Test
-import kotlin.math.roundToInt
@MediumTest
// @RunWith(Parameterized::class)
@@ -51,11 +49,19 @@
private val vertical: Boolean
get() = true // orientation == Orientation.Vertical
- private val items = (1..20).toList()
+ private val itemsCount = 20
private lateinit var state: LazyGridState
+ private val itemSizePx = 100
+ private var itemSizeDp = Dp.Unspecified
+ private var containerSizeDp = Dp.Unspecified
+
@Before
fun setup() {
+ with(rule.density) {
+ itemSizeDp = itemSizePx.toDp()
+ containerSizeDp = itemSizeDp * 3
+ }
rule.setContent {
state = rememberLazyGridState()
TestContent()
@@ -63,14 +69,14 @@
}
@Test
- fun testSetupWorks() {
+ fun setupWorks() {
assertThat(state.firstVisibleItemIndex).isEqualTo(0)
assertThat(state.firstVisibleItemScrollOffset).isEqualTo(0)
assertThat(state.firstVisibleItemIndex).isEqualTo(0)
}
@Test
- fun snapToItemTest() = runBlocking {
+ fun scrollToItem() = runBlocking {
withContext(Dispatchers.Main + AutoTestFrameClock()) {
state.scrollToItem(2)
}
@@ -85,14 +91,57 @@
}
@Test
- fun smoothScrollByTest() = runBlocking {
- fun Int.dpToPx(): Int = with(rule.density) { dp.toPx().roundToInt() }
- val scrollDistance = 320.dpToPx()
- val itemSize = 101.dpToPx()
+ fun scrollToItemWithOffset() = runBlocking {
+ withContext(Dispatchers.Main + AutoTestFrameClock()) {
+ state.scrollToItem(6, 10)
+ }
+ assertThat(state.firstVisibleItemIndex).isEqualTo(6)
+ assertThat(state.firstVisibleItemScrollOffset).isEqualTo(10)
+ }
- val expectedLine = scrollDistance / itemSize // resolves to 3
+ @Test
+ fun scrollToItemWithNegativeOffset() = runBlocking {
+ withContext(Dispatchers.Main + AutoTestFrameClock()) {
+ state.scrollToItem(6, -10)
+ }
+ assertThat(state.firstVisibleItemIndex).isEqualTo(4)
+ val item6Offset = state.layoutInfo.visibleItemsInfo.first { it.index == 6 }.offset.y
+ assertThat(item6Offset).isEqualTo(10)
+ }
+
+ @Test
+ fun scrollToItemWithPositiveOffsetLargerThanAvailableSize() = runBlocking {
+ withContext(Dispatchers.Main + AutoTestFrameClock()) {
+ state.scrollToItem(itemsCount - 6, 10)
+ }
+ assertThat(state.firstVisibleItemIndex).isEqualTo(itemsCount - 6)
+ assertThat(state.firstVisibleItemScrollOffset).isEqualTo(0) // not 10
+ }
+
+ @Test
+ fun scrollToItemWithNegativeOffsetLargerThanAvailableSize() = runBlocking {
+ withContext(Dispatchers.Main + AutoTestFrameClock()) {
+ state.scrollToItem(1, -(itemSizePx + 10))
+ }
+ assertThat(state.firstVisibleItemIndex).isEqualTo(0)
+ assertThat(state.firstVisibleItemScrollOffset).isEqualTo(0) // not -10
+ }
+
+ @Test
+ fun scrollToItemWithIndexLargerThanItemsCount() = runBlocking {
+ withContext(Dispatchers.Main + AutoTestFrameClock()) {
+ state.scrollToItem(itemsCount + 4)
+ }
+ assertThat(state.firstVisibleItemIndex).isEqualTo(itemsCount - 6)
+ }
+
+ @Test
+ fun animateScrollBy() = runBlocking {
+ val scrollDistance = 320
+
+ val expectedLine = scrollDistance / itemSizePx // resolves to 3
val expectedItem = expectedLine * 2 // resolves to 6
- val expectedOffset = scrollDistance % itemSize // resolves to ~17.dp.toIntPx()
+ val expectedOffset = scrollDistance % itemSizePx // resolves to 20px
withContext(Dispatchers.Main + AutoTestFrameClock()) {
state.animateScrollBy(scrollDistance.toFloat())
@@ -102,25 +151,64 @@
}
@Test
- fun smoothScrollToItemTest() = runBlocking {
+ fun animateScrollToItem() = runBlocking {
withContext(Dispatchers.Main + AutoTestFrameClock()) {
state.animateScrollToItem(10, 10)
}
assertThat(state.firstVisibleItemIndex).isEqualTo(10)
assertThat(state.firstVisibleItemScrollOffset).isEqualTo(10)
+ }
+
+ @Test
+ fun animateScrollToItemWithOffset() = runBlocking {
withContext(Dispatchers.Main + AutoTestFrameClock()) {
- state.animateScrollToItem(0, 10)
- state.animateScrollToItem(11, 10)
+ state.animateScrollToItem(6, 10)
}
- // assertThat(state.firstVisibleItemIndex).isEqualTo(10)
- // assertThat(state.firstVisibleItemScrollOffset).isEqualTo(10)
+ assertThat(state.firstVisibleItemIndex).isEqualTo(6)
+ assertThat(state.firstVisibleItemScrollOffset).isEqualTo(10)
+ }
+
+ @Test
+ fun animateScrollToItemWithNegativeOffset() = runBlocking {
+ withContext(Dispatchers.Main + AutoTestFrameClock()) {
+ state.animateScrollToItem(6, -10)
+ }
+ assertThat(state.firstVisibleItemIndex).isEqualTo(4)
+ val item6Offset = state.layoutInfo.visibleItemsInfo.first { it.index == 6 }.offset.y
+ assertThat(item6Offset).isEqualTo(10)
+ }
+
+ @Test
+ fun animateScrollToItemWithPositiveOffsetLargerThanAvailableSize() = runBlocking {
+ withContext(Dispatchers.Main + AutoTestFrameClock()) {
+ state.animateScrollToItem(itemsCount - 6, 10)
+ }
+ assertThat(state.firstVisibleItemIndex).isEqualTo(itemsCount - 6)
+ assertThat(state.firstVisibleItemScrollOffset).isEqualTo(0) // not 10
+ }
+
+ @Test
+ fun animateScrollToItemWithNegativeOffsetLargerThanAvailableSize() = runBlocking {
+ withContext(Dispatchers.Main + AutoTestFrameClock()) {
+ state.animateScrollToItem(2, -(itemSizePx + 10))
+ }
+ assertThat(state.firstVisibleItemIndex).isEqualTo(0)
+ assertThat(state.firstVisibleItemScrollOffset).isEqualTo(0) // not -10
+ }
+
+ @Test
+ fun animateScrollToItemWithIndexLargerThanItemsCount() = runBlocking {
+ withContext(Dispatchers.Main + AutoTestFrameClock()) {
+ state.animateScrollToItem(itemsCount + 2)
+ }
+ assertThat(state.firstVisibleItemIndex).isEqualTo(itemsCount - 6)
}
@Composable
private fun TestContent() {
if (vertical) {
- LazyVerticalGrid(GridCells.Fixed(2), Modifier.height(300.dp), state) {
- items(items) {
+ LazyVerticalGrid(GridCells.Fixed(2), Modifier.height(containerSizeDp), state) {
+ items(itemsCount) {
ItemContent()
}
}
@@ -136,9 +224,9 @@
@Composable
private fun ItemContent() {
val modifier = if (vertical) {
- Modifier.height(101.dp)
+ Modifier.height(itemSizeDp)
} else {
- Modifier.width(101.dp)
+ Modifier.width(itemSizeDp)
}
Spacer(modifier)
}
diff --git a/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/list/LazyScrollTest.kt b/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/list/LazyScrollTest.kt
index bf3cf61..766053b 100644
--- a/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/list/LazyScrollTest.kt
+++ b/compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/lazy/list/LazyScrollTest.kt
@@ -32,7 +32,6 @@
import androidx.compose.runtime.rememberCoroutineScope
import androidx.compose.ui.test.junit4.createComposeRule
import androidx.compose.ui.unit.Dp
-import androidx.compose.ui.unit.dp
import androidx.test.filters.MediumTest
import com.google.common.truth.Truth.assertThat
import com.google.common.truth.Truth.assertWithMessage
@@ -142,11 +141,10 @@
@Test
fun animateScrollBy() = runBlocking {
- fun Int.dpToPx(): Int = with(rule.density) { dp.toPx().roundToInt() }
- val scrollDistance = 320.dpToPx()
+ val scrollDistance = 320
val expectedIndex = scrollDistance / itemSizePx // resolves to 3
- val expectedOffset = scrollDistance % itemSizePx // resolves to ~17.dp.toIntPx()
+ val expectedOffset = scrollDistance % itemSizePx // resolves to 20px
withContext(Dispatchers.Main + AutoTestFrameClock()) {
state.animateScrollBy(scrollDistance.toFloat())
diff --git a/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/LazyGridState.kt b/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/LazyGridState.kt
index e463d58..02db167 100644
--- a/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/LazyGridState.kt
+++ b/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/LazyGridState.kt
@@ -204,17 +204,14 @@
* Instantly brings the item at [index] to the top of the viewport, offset by [scrollOffset]
* pixels.
*
- * Cancels the currently running scroll, if any, and suspends until the cancellation is
- * complete.
- *
- * @param index the data index to snap to. Must be between 0 and the number of elements.
- * @param scrollOffset the number of pixels past the start of the item to snap to. Must
- * not be negative.
+ * @param index the index to which to scroll. Must be non-negative.
+ * @param scrollOffset the offset that the item should end up after the scroll. Note that
+ * positive offset refers to forward scroll, so in a top-to-bottom list, positive offset will
+ * scroll the item further upward (taking it partly offscreen).
*/
suspend fun scrollToItem(
/*@IntRange(from = 0)*/
index: Int,
- /*@IntRange(from = 0)*/
scrollOffset: Int = 0
) {
return scrollableState.scroll {
@@ -235,9 +232,6 @@
* performed within a [scroll] block (even if they don't call any other methods on this
* object) in order to guarantee that mutual exclusion is enforced.
*
- * Cancels the currently running scroll, if any, and suspends until the cancellation is
- * complete.
- *
* If [scroll] is called from elsewhere, this will be canceled.
*/
override suspend fun scroll(
@@ -332,16 +326,14 @@
/**
* Animate (smooth scroll) to the given item.
*
- * @param index the index to which to scroll
- * @param scrollOffset the offset that the item should end up after the scroll (same as
- * [scrollToItem]) - note that positive offset refers to forward scroll, so in a
- * top-to-bottom grid, positive offset will scroll the item further upward (taking it partly
- * offscreen)
+ * @param index the index to which to scroll. Must be non-negative.
+ * @param scrollOffset the offset that the item should end up after the scroll. Note that
+ * positive offset refers to forward scroll, so in a top-to-bottom list, positive offset will
+ * scroll the item further upward (taking it partly offscreen).
*/
suspend fun animateScrollToItem(
/*@IntRange(from = 0)*/
index: Int,
- /*@IntRange(from = 0)*/
scrollOffset: Int = 0
) {
doSmoothScrollToItem(index, scrollOffset)
diff --git a/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/LazyListState.kt b/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/LazyListState.kt
index 7581a4f..ff33032 100644
--- a/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/LazyListState.kt
+++ b/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/LazyListState.kt
@@ -321,7 +321,7 @@
* @param index the index to which to scroll. Must be non-negative.
* @param scrollOffset the offset that the item should end up after the scroll. Note that
* positive offset refers to forward scroll, so in a top-to-bottom list, positive offset will
- * scroll the item further upward (taking it partly offscreen)
+ * scroll the item further upward (taking it partly offscreen).
*/
suspend fun animateScrollToItem(
/*@IntRange(from = 0)*/
diff --git a/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/grid/LazyGridScrollPosition.kt b/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/grid/LazyGridScrollPosition.kt
index 50dcf3e..bd6578d 100644
--- a/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/grid/LazyGridScrollPosition.kt
+++ b/compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/lazy/grid/LazyGridScrollPosition.kt
@@ -59,9 +59,11 @@
// state would be lost and overridden with zeros.
if (hadFirstNotEmptyLayout || measureResult.totalItemsCount > 0) {
hadFirstNotEmptyLayout = true
+ val scrollOffset = measureResult.firstVisibleLineScrollOffset
+ check(scrollOffset >= 0f) { "scrollOffset should be non-negative ($scrollOffset)" }
update(
ItemIndex(measureResult.firstVisibleLine?.items?.firstOrNull()?.index?.value ?: 0),
- measureResult.firstVisibleLineScrollOffset
+ scrollOffset
)
}
}
@@ -71,7 +73,7 @@
* composing the items during the next measure pass and will be updated by the real
* position calculated during the measurement. This means that there is guarantee that
* exactly this index and offset will be applied as it is possible that:
- * a) there will no item at this index in reality
+ * a) there will be no item at this index in reality
* b) item at this index will be smaller than the asked scrollOffset, which means we would
* switch to the next item
* c) there will be not enough items to fill the viewport after the requested index, so we
@@ -96,7 +98,6 @@
private fun update(index: ItemIndex, scrollOffset: Int) {
require(index.value >= 0f) { "Index should be non-negative (${index.value})" }
- require(scrollOffset >= 0f) { "scrollOffset should be non-negative ($scrollOffset)" }
if (index != this.index) {
this.index = index
indexState.value = index.value