From 0f65d126bda3d7dc50a99c603e7d75f842b9b09c Mon Sep 17 00:00:00 2001 From: Jesse Wilson Date: Mon, 23 Oct 2023 17:17:36 -0400 Subject: [PATCH] Support animated programmatic scrolls (#1624) In emoji search when the search terms is changed we scroll to the top of the list. This means we need to treat programmatic scrolls as something that can happen repeatedly, not just when we restore a lazy list. This moves the feature that prevents programmatic scrolls from colliding with user scrolls from host code to guest code. The benefit is it no longer risks getting the loaded window out of sync with the true scroll window. This also implements all scroll behavior for iOS. --- .../cash/redwood/lazylayout/api/properties.kt | 4 +- .../api/ScrollItemIndexSerializationTest.kt | 4 ++ .../redwood/lazylayout/compose/LazyList.kt | 5 +- .../lazylayout/compose/LazyListState.kt | 41 ++++++++---- .../lazylayout/uiview/UIViewLazyList.kt | 62 +++++++++++-------- .../redwood/lazylayout/view/ViewLazyList.kt | 24 ++++++- .../widget/LazyListScrollProcessor.kt | 15 ++--- .../lazylayout/widget/FakeScrollProcessor.kt | 4 +- .../widget/LazyListScrollProcessorTest.kt | 33 ++++++---- .../emojisearch/presenter/EmojiSearch.kt | 2 +- 10 files changed, 126 insertions(+), 68 deletions(-) diff --git a/redwood-lazylayout-api/src/commonMain/kotlin/app/cash/redwood/lazylayout/api/properties.kt b/redwood-lazylayout-api/src/commonMain/kotlin/app/cash/redwood/lazylayout/api/properties.kt index 1c1431b16d..4e3de5495b 100644 --- a/redwood-lazylayout-api/src/commonMain/kotlin/app/cash/redwood/lazylayout/api/properties.kt +++ b/redwood-lazylayout-api/src/commonMain/kotlin/app/cash/redwood/lazylayout/api/properties.kt @@ -25,6 +25,8 @@ import kotlinx.serialization.Serializable @[Immutable Serializable] @Poko public class ScrollItemIndex( - @Suppress("unused") private val id: Int, + public val id: Int, public val index: Int, + /** True to smoothly scroll to the new position. */ + public val animated: Boolean = false, ) diff --git a/redwood-lazylayout-api/src/commonTest/kotlin/app/cash/redwood/lazylayout/api/ScrollItemIndexSerializationTest.kt b/redwood-lazylayout-api/src/commonTest/kotlin/app/cash/redwood/lazylayout/api/ScrollItemIndexSerializationTest.kt index cbe685ca18..d5de709d2e 100644 --- a/redwood-lazylayout-api/src/commonTest/kotlin/app/cash/redwood/lazylayout/api/ScrollItemIndexSerializationTest.kt +++ b/redwood-lazylayout-api/src/commonTest/kotlin/app/cash/redwood/lazylayout/api/ScrollItemIndexSerializationTest.kt @@ -32,6 +32,10 @@ class ScrollItemIndexSerializationTest { ScrollItemIndex(3, 7), """{"id":3,"index":7}""", ) + assertRoundTrip( + ScrollItemIndex(3, 7, animated = true), + """{"id":3,"index":7,"animated":true}""", + ) } private fun assertRoundTrip(value: ScrollItemIndex, encoded: String) { diff --git a/redwood-lazylayout-compose/src/commonMain/kotlin/app/cash/redwood/lazylayout/compose/LazyList.kt b/redwood-lazylayout-compose/src/commonMain/kotlin/app/cash/redwood/lazylayout/compose/LazyList.kt index 47593b585e..73eef988b4 100644 --- a/redwood-lazylayout-compose/src/commonMain/kotlin/app/cash/redwood/lazylayout/compose/LazyList.kt +++ b/redwood-lazylayout-compose/src/commonMain/kotlin/app/cash/redwood/lazylayout/compose/LazyList.kt @@ -26,7 +26,6 @@ import androidx.compose.runtime.setValue import app.cash.redwood.Modifier import app.cash.redwood.layout.api.Constraint import app.cash.redwood.layout.api.CrossAxisAlignment -import app.cash.redwood.lazylayout.api.ScrollItemIndex import app.cash.redwood.ui.Margin import kotlin.jvm.JvmName @@ -70,7 +69,7 @@ internal fun LazyList( margin = margin, crossAxisAlignment = crossAxisAlignment, modifier = modifier, - scrollItemIndex = ScrollItemIndex(0, state.programmaticScrollIndex), + scrollItemIndex = state.programmaticScrollIndex, placeholder = { repeat(placeholderPoolSize) { placeholder() } }, items = { for (index in itemsBefore until itemCount - itemsAfter) { @@ -123,7 +122,7 @@ internal fun RefreshableLazyList( margin = margin, crossAxisAlignment = crossAxisAlignment, modifier = modifier, - scrollItemIndex = ScrollItemIndex(0, state.programmaticScrollIndex), + scrollItemIndex = state.programmaticScrollIndex, placeholder = { repeat(placeholderPoolSize) { placeholder() } }, pullRefreshContentColor = pullRefreshContentColor, items = { diff --git a/redwood-lazylayout-compose/src/commonMain/kotlin/app/cash/redwood/lazylayout/compose/LazyListState.kt b/redwood-lazylayout-compose/src/commonMain/kotlin/app/cash/redwood/lazylayout/compose/LazyListState.kt index e9d47dc4f0..4732546b7b 100644 --- a/redwood-lazylayout-compose/src/commonMain/kotlin/app/cash/redwood/lazylayout/compose/LazyListState.kt +++ b/redwood-lazylayout-compose/src/commonMain/kotlin/app/cash/redwood/lazylayout/compose/LazyListState.kt @@ -21,6 +21,7 @@ import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.saveable.Saver import androidx.compose.runtime.saveable.rememberSaveable import androidx.compose.runtime.setValue +import app.cash.redwood.lazylayout.api.ScrollItemIndex @Composable public fun rememberLazyListState(): LazyListState { @@ -34,19 +35,24 @@ private val saver: Saver = Saver( save = { it.firstIndex }, restore = { LazyListState().apply { - programmaticScroll(it) + programmaticScroll(firstIndex = it, animated = false, clobberUserScroll = false) } }, ) public open class LazyListState { /** - * Update this to trigger a programmatic scroll. Typically this is updated exactly once, when the - * previous scroll state is restored. + * Update this to trigger a programmatic scroll. This may be updated multiple times, including + * when the previous scroll state is restored. */ - public var programmaticScrollIndex: Int by mutableStateOf(0) + public var programmaticScrollIndex: ScrollItemIndex by mutableStateOf( + ScrollItemIndex(id = 0, index = 0, animated = false), + ) private set + /** Once we receive a user scroll, we limit which programmatic scrolls we apply. */ + private var userScrolled = false + /** Bounds of what the user is looking at. Everything else is placeholders! */ public var firstIndex: Int by mutableStateOf(0) private set @@ -54,19 +60,32 @@ public open class LazyListState { private set /** Perform a programmatic scroll. */ - public fun programmaticScroll(index: Int) { - require(index >= 0) - require(programmaticScrollIndex == 0) { "unexpected double restoreIndex()" } + public fun programmaticScroll( + firstIndex: Int, + animated: Boolean, + clobberUserScroll: Boolean = true, + ) { + require(firstIndex >= 0) + if (!clobberUserScroll && userScrolled) return - this.programmaticScrollIndex = index + val previous = programmaticScrollIndex + this.programmaticScrollIndex = ScrollItemIndex( + id = previous.id + 1, + index = firstIndex, + animated = animated, + ) - val delta = (lastIndex - firstIndex) - this.firstIndex = index - this.lastIndex = index + delta + val delta = (lastIndex - this.firstIndex) + this.firstIndex = firstIndex + this.lastIndex = firstIndex + delta } /** React to a user-initiated scroll. */ public fun onUserScroll(firstIndex: Int, lastIndex: Int) { + if (firstIndex > 0) { + userScrolled = true + } + this.firstIndex = firstIndex this.lastIndex = lastIndex } diff --git a/redwood-lazylayout-uiview/src/commonMain/kotlin/app/cash/redwood/lazylayout/uiview/UIViewLazyList.kt b/redwood-lazylayout-uiview/src/commonMain/kotlin/app/cash/redwood/lazylayout/uiview/UIViewLazyList.kt index 4a9c9feaf6..4ce0f6e061 100644 --- a/redwood-lazylayout-uiview/src/commonMain/kotlin/app/cash/redwood/lazylayout/uiview/UIViewLazyList.kt +++ b/redwood-lazylayout-uiview/src/commonMain/kotlin/app/cash/redwood/lazylayout/uiview/UIViewLazyList.kt @@ -26,6 +26,7 @@ import app.cash.redwood.layout.api.Constraint import app.cash.redwood.layout.api.CrossAxisAlignment import app.cash.redwood.lazylayout.api.ScrollItemIndex import app.cash.redwood.lazylayout.widget.LazyList +import app.cash.redwood.lazylayout.widget.LazyListScrollProcessor import app.cash.redwood.lazylayout.widget.LazyListUpdateProcessor import app.cash.redwood.lazylayout.widget.LazyListUpdateProcessor.Binding import app.cash.redwood.lazylayout.widget.RefreshableLazyList @@ -71,9 +72,7 @@ internal open class UIViewLazyList( override val value: UIView get() = tableView - protected var onViewportChanged: ((firstVisibleItemIndex: Int, lastVisibleItemIndex: Int) -> Unit)? = null - - private val processor = object : LazyListUpdateProcessor() { + private val updateProcessor = object : LazyListUpdateProcessor() { override fun insertRows(index: Int, count: Int) { // TODO(jwilson): pass a range somehow when 'count' is large? tableView.insertRowsAtIndexPaths( @@ -95,9 +94,24 @@ internal open class UIViewLazyList( } } - override val placeholder: Widget.Children = processor.placeholder + private var isDoingProgrammaticScroll = false + + private val scrollProcessor = object : LazyListScrollProcessor() { + override fun contentSize() = updateProcessor.size + + override fun programmaticScroll(firstIndex: Int, animated: Boolean) { + isDoingProgrammaticScroll = animated // Don't forward scroll updates to scrollProcessor. + tableView.scrollToRowAtIndexPath( + NSIndexPath.indexPathForItem(firstIndex.toLong(), 0), + UITableViewScrollPosition.UITableViewScrollPositionTop, + animated = animated, + ) + } + } + + override val placeholder: Widget.Children = updateProcessor.placeholder - override val items: Widget.Children = processor.items + override val items: Widget.Children = updateProcessor.items private val dataSource = object : NSObject(), UITableViewDataSourceProtocol { override fun tableView( @@ -105,7 +119,7 @@ internal open class UIViewLazyList( numberOfRowsInSection: NSInteger, ): Long { require(numberOfRowsInSection == 0L) - return processor.size.toLong() + return updateProcessor.size.toLong() } override fun tableView( @@ -113,7 +127,7 @@ internal open class UIViewLazyList( cellForRowAtIndexPath: NSIndexPath, ): LazyListContainerCell { val index = cellForRowAtIndexPath.item.toInt() - return processor.getOrCreateView(index) { binding -> + return updateProcessor.getOrCreateView(index) { binding -> createView(tableView, binding, index) } } @@ -136,15 +150,18 @@ internal open class UIViewLazyList( private val tableViewDelegate: UITableViewDelegateProtocol = object : NSObject(), UITableViewDelegateProtocol { override fun scrollViewDidScroll(scrollView: UIScrollView) { + if (isDoingProgrammaticScroll) return // Only notify of user scrolls. + val visibleIndexPaths = tableView.indexPathsForVisibleRows ?: return + if (visibleIndexPaths.isEmpty()) return + + val firstIndex = visibleIndexPaths.minOf { (it as NSIndexPath).item.toInt() } + val lastIndex = visibleIndexPaths.maxOf { (it as NSIndexPath).item.toInt() } + scrollProcessor.onUserScroll(firstIndex, lastIndex) + } - if (visibleIndexPaths.isNotEmpty()) { - // TODO: Optimize this for less operations - onViewportChanged?.invoke( - visibleIndexPaths.minOf { (it as NSIndexPath).item.toInt() }, - visibleIndexPaths.maxOf { (it as NSIndexPath).item.toInt() }, - ) - } + override fun scrollViewDidEndScrollingAnimation(scrollView: UIScrollView) { + isDoingProgrammaticScroll = false } } @@ -165,7 +182,7 @@ internal open class UIViewLazyList( } final override fun onViewportChanged(onViewportChanged: (Int, Int) -> Unit) { - this.onViewportChanged = onViewportChanged + scrollProcessor.onViewportChanged(onViewportChanged) } override fun isVertical(isVertical: Boolean) { @@ -191,25 +208,20 @@ internal open class UIViewLazyList( } override fun scrollItemIndex(scrollItemIndex: ScrollItemIndex) { - if (scrollItemIndex.index < processor.size) { - tableView.scrollToRowAtIndexPath( - NSIndexPath.indexPathForItem(scrollItemIndex.index.toLong(), 0), - UITableViewScrollPosition.UITableViewScrollPositionTop, - animated = false, - ) - } + scrollProcessor.scrollItemIndex(scrollItemIndex) } override fun itemsBefore(itemsBefore: Int) { - processor.itemsBefore(itemsBefore) + updateProcessor.itemsBefore(itemsBefore) } override fun itemsAfter(itemsAfter: Int) { - processor.itemsAfter(itemsAfter) + updateProcessor.itemsAfter(itemsAfter) } override fun onEndChanges() { - processor.onEndChanges() + updateProcessor.onEndChanges() + scrollProcessor.onEndChanges() } } diff --git a/redwood-lazylayout-view/src/main/kotlin/app/cash/redwood/lazylayout/view/ViewLazyList.kt b/redwood-lazylayout-view/src/main/kotlin/app/cash/redwood/lazylayout/view/ViewLazyList.kt index 0549eaefd9..1c5c102476 100644 --- a/redwood-lazylayout-view/src/main/kotlin/app/cash/redwood/lazylayout/view/ViewLazyList.kt +++ b/redwood-lazylayout-view/src/main/kotlin/app/cash/redwood/lazylayout/view/ViewLazyList.kt @@ -29,6 +29,7 @@ import androidx.core.view.doOnDetach import androidx.core.view.updateLayoutParams import androidx.core.view.updatePaddingRelative import androidx.recyclerview.widget.LinearLayoutManager +import androidx.recyclerview.widget.LinearSmoothScroller import androidx.recyclerview.widget.RecyclerView import androidx.swiperefreshlayout.widget.SwipeRefreshLayout import app.cash.redwood.Modifier @@ -84,11 +85,22 @@ internal open class ViewLazyList private constructor( } } + private var isDoingProgrammaticScroll = false + private val scrollProcessor = object : LazyListScrollProcessor() { override fun contentSize(): Int = processor.size - override fun programmaticScroll(firstIndex: Int) { - linearLayoutManager.scrollToPositionWithOffset(firstIndex, 0) + override fun programmaticScroll(firstIndex: Int, animated: Boolean) { + isDoingProgrammaticScroll = animated + if (animated) { + val smoothScroller: RecyclerView.SmoothScroller = object : LinearSmoothScroller(recyclerView.context) { + override fun getVerticalSnapPreference(): Int = SNAP_TO_START + } + smoothScroller.targetPosition = firstIndex + linearLayoutManager.startSmoothScroll(smoothScroller) + } else { + linearLayoutManager.scrollToPositionWithOffset(firstIndex, 0) + } } } @@ -108,7 +120,7 @@ internal open class ViewLazyList private constructor( addOnScrollListener( object : RecyclerView.OnScrollListener() { override fun onScrolled(recyclerView: RecyclerView, dx: Int, dy: Int) { - if (scrollState == RecyclerView.SCROLL_STATE_IDLE) return + if (isDoingProgrammaticScroll) return // Only notify of user scrolls. val firstIndex = linearLayoutManager.findFirstVisibleItemPosition() if (firstIndex == RecyclerView.NO_POSITION) return @@ -117,6 +129,12 @@ internal open class ViewLazyList private constructor( scrollProcessor.onUserScroll(firstIndex, lastIndex) } + + override fun onScrollStateChanged(recyclerView: RecyclerView, newState: Int) { + if (newState == RecyclerView.SCROLL_STATE_IDLE) { + isDoingProgrammaticScroll = false + } + } }, ) diff --git a/redwood-lazylayout-widget/src/commonMain/kotlin/app/cash/redwood/lazylayout/widget/LazyListScrollProcessor.kt b/redwood-lazylayout-widget/src/commonMain/kotlin/app/cash/redwood/lazylayout/widget/LazyListScrollProcessor.kt index c005242cda..5fb9ff9331 100644 --- a/redwood-lazylayout-widget/src/commonMain/kotlin/app/cash/redwood/lazylayout/widget/LazyListScrollProcessor.kt +++ b/redwood-lazylayout-widget/src/commonMain/kotlin/app/cash/redwood/lazylayout/widget/LazyListScrollProcessor.kt @@ -23,9 +23,7 @@ public abstract class LazyListScrollProcessor { /** We can't scroll to this index until we have enough data for it to display! */ private var deferredProgrammaticScrollIndex: Int = -1 - - /** Once we receive a user scroll, we stop forwarding programmatic scrolls. */ - private var userHasScrolled = false + private var deferredProgrammaticScrollAnimated: Boolean = false /** De-duplicate calls to [onViewportChanged]. */ private var mostRecentFirstIndex = -1 @@ -38,21 +36,20 @@ public abstract class LazyListScrollProcessor { public fun scrollItemIndex(scrollItemIndex: ScrollItemIndex) { // Defer until we have data in onEndChanges(). deferredProgrammaticScrollIndex = scrollItemIndex.index + deferredProgrammaticScrollAnimated = scrollItemIndex.animated } public fun onEndChanges() { // Do nothing: we don't have deferred scrolls. if (deferredProgrammaticScrollIndex == -1) return - // Do nothing: we don't do programmatic scrolls if the user has already scrolled manually. - if (userHasScrolled) return - // Do nothing: we can't scroll to this item because it hasn't loaded yet! if (contentSize() <= deferredProgrammaticScrollIndex) return // Do a programmatic scroll! - programmaticScroll(deferredProgrammaticScrollIndex) + programmaticScroll(deferredProgrammaticScrollIndex, deferredProgrammaticScrollAnimated) deferredProgrammaticScrollIndex = -1 + deferredProgrammaticScrollAnimated = false } /** @@ -60,8 +57,6 @@ public abstract class LazyListScrollProcessor { * scrolls. */ public fun onUserScroll(firstIndex: Int, lastIndex: Int) { - if (firstIndex > 0) userHasScrolled = true - if (firstIndex == mostRecentFirstIndex && lastIndex == mostRecentLastIndex) return this.mostRecentFirstIndex = firstIndex @@ -74,5 +69,5 @@ public abstract class LazyListScrollProcessor { public abstract fun contentSize(): Int /** Perform a programmatic scroll. */ - public abstract fun programmaticScroll(firstIndex: Int) + public abstract fun programmaticScroll(firstIndex: Int, animated: Boolean) } diff --git a/redwood-lazylayout-widget/src/commonTest/kotlin/app/cash/redwood/lazylayout/widget/FakeScrollProcessor.kt b/redwood-lazylayout-widget/src/commonTest/kotlin/app/cash/redwood/lazylayout/widget/FakeScrollProcessor.kt index f2db020729..50922a83be 100644 --- a/redwood-lazylayout-widget/src/commonTest/kotlin/app/cash/redwood/lazylayout/widget/FakeScrollProcessor.kt +++ b/redwood-lazylayout-widget/src/commonTest/kotlin/app/cash/redwood/lazylayout/widget/FakeScrollProcessor.kt @@ -29,9 +29,9 @@ class FakeScrollProcessor : LazyListScrollProcessor() { override fun contentSize(): Int = size - override fun programmaticScroll(firstIndex: Int) { + override fun programmaticScroll(firstIndex: Int, animated: Boolean) { require(firstIndex < size) - events += "programmaticScroll($firstIndex)" + events += "programmaticScroll(firstIndex = $firstIndex, animated = $animated)" } fun takeEvents(): List { diff --git a/redwood-lazylayout-widget/src/commonTest/kotlin/app/cash/redwood/lazylayout/widget/LazyListScrollProcessorTest.kt b/redwood-lazylayout-widget/src/commonTest/kotlin/app/cash/redwood/lazylayout/widget/LazyListScrollProcessorTest.kt index 4ba47b00d4..f53dbba43c 100644 --- a/redwood-lazylayout-widget/src/commonTest/kotlin/app/cash/redwood/lazylayout/widget/LazyListScrollProcessorTest.kt +++ b/redwood-lazylayout-widget/src/commonTest/kotlin/app/cash/redwood/lazylayout/widget/LazyListScrollProcessorTest.kt @@ -34,7 +34,9 @@ class LazyListScrollProcessorTest { assertThat(processor.takeEvents()).isEmpty() processor.onEndChanges() - assertThat(processor.takeEvents()).containsExactly("programmaticScroll(10)") + assertThat(processor.takeEvents()).containsExactly( + "programmaticScroll(firstIndex = 10, animated = false)", + ) } @Test @@ -47,34 +49,41 @@ class LazyListScrollProcessorTest { // Once we have enough rows we can apply the scroll. processor.size = 30 processor.onEndChanges() - assertThat(processor.takeEvents()).containsExactly("programmaticScroll(10)") + assertThat(processor.takeEvents()).containsExactly( + "programmaticScroll(firstIndex = 10, animated = false)", + ) } @Test - fun programmaticScrollDiscardedAfterUserScroll() { + fun eachProgrammaticScrollOnlyTriggeredOnce() { processor.size = 30 - // Do a user scroll. - processor.onUserScroll(5, 14) - assertThat(processor.takeEvents()).containsExactly("userScroll(5, 14)") - - // Don't apply the programmatic scroll. That fights the user. processor.scrollItemIndex(ScrollItemIndex(0, 10)) processor.onEndChanges() + assertThat(processor.takeEvents()).containsExactly( + "programmaticScroll(firstIndex = 10, animated = false)", + ) + + // Confirm onEndIndex() only applies its change once. + processor.onEndChanges() assertThat(processor.takeEvents()).isEmpty() } @Test - fun programmaticScrollOnlyTriggeredOnce() { + fun multipleProgrammaticScrolls() { processor.size = 30 processor.scrollItemIndex(ScrollItemIndex(0, 10)) processor.onEndChanges() - assertThat(processor.takeEvents()).containsExactly("programmaticScroll(10)") + assertThat(processor.takeEvents()).containsExactly( + "programmaticScroll(firstIndex = 10, animated = false)", + ) - // Confirm onEndIndex() only applies its change once. + processor.scrollItemIndex(ScrollItemIndex(1, 20)) processor.onEndChanges() - assertThat(processor.takeEvents()).isEmpty() + assertThat(processor.takeEvents()).containsExactly( + "programmaticScroll(firstIndex = 20, animated = false)", + ) } @Test diff --git a/samples/emoji-search/presenter/src/commonMain/kotlin/com/example/redwood/emojisearch/presenter/EmojiSearch.kt b/samples/emoji-search/presenter/src/commonMain/kotlin/com/example/redwood/emojisearch/presenter/EmojiSearch.kt index 06e61de765..b822a18a5c 100644 --- a/samples/emoji-search/presenter/src/commonMain/kotlin/com/example/redwood/emojisearch/presenter/EmojiSearch.kt +++ b/samples/emoji-search/presenter/src/commonMain/kotlin/com/example/redwood/emojisearch/presenter/EmojiSearch.kt @@ -101,7 +101,7 @@ private fun LazyColumn( val lazyListState = rememberLazyListState() LaunchedEffect(searchTerm) { - lazyListState.programmaticScroll(0) + lazyListState.programmaticScroll(0, animated = true) } LaunchedEffect(refreshSignal) {