De-duplicate calls to onViewportChanged (#1623)

redwood-lazylayout-widget/src/commonMain/kotlin/app/cash/redwood/lazylayout/widget/LazyListScrollProcessor.kt

@@ -27,6 +27,10 @@ public abstract class LazyListScrollProcessor {
   /** Once we receive a user scroll, we stop forwarding programmatic scrolls. */
   private var userHasScrolled = false
 
+  /** De-duplicate calls to [onViewportChanged]. */
+  private var mostRecentFirstIndex = -1
+  private var mostRecentLastIndex = -1
+
   public fun onViewportChanged(onViewportChanged: (Int, Int) -> Unit) {
     this.onViewportChanged = onViewportChanged
   }
@@ -57,6 +61,12 @@ public abstract class LazyListScrollProcessor {
    */
   public fun onUserScroll(firstIndex: Int, lastIndex: Int) {
     if (firstIndex > 0) userHasScrolled = true
+
+    if (firstIndex == mostRecentFirstIndex && lastIndex == mostRecentLastIndex) return
+
+    this.mostRecentFirstIndex = firstIndex
+    this.mostRecentLastIndex = lastIndex
+
     onViewportChanged?.invoke(firstIndex, lastIndex)
   }
 

redwood-lazylayout-widget/src/commonTest/kotlin/app/cash/redwood/lazylayout/widget/FakeScrollProcessor.kt

@@ -0,0 +1,42 @@
+/*
+ * Copyright (C) 2023 Square, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package app.cash.redwood.lazylayout.widget
+
+class FakeScrollProcessor : LazyListScrollProcessor() {
+  private val events = mutableListOf<String>()
+
+  /** How many rows are in the list. */
+  var size = 0
+
+  init {
+    this.onViewportChanged { firstIndex, lastIndex ->
+      events += "userScroll($firstIndex, $lastIndex)"
+    }
+  }
+
+  override fun contentSize(): Int = size
+
+  override fun programmaticScroll(firstIndex: Int) {
+    require(firstIndex < size)
+    events += "programmaticScroll($firstIndex)"
+  }
+
+  fun takeEvents(): List<String> {
+    val result = events.toList()
+    events.clear()
+    return result
+  }
+}

redwood-lazylayout-widget/src/commonTest/kotlin/app/cash/redwood/lazylayout/widget/FakeProcessor.kt → redwood-lazylayout-widget/src/commonTest/kotlin/app/cash/redwood/lazylayout/widget/FakeUpdateProcessor.kt

@@ -21,7 +21,7 @@ import app.cash.redwood.widget.Widget
  * This fake simulates a real scroll window, which is completely independent of the window of loaded
  * items. Tests should call [scrollTo] to move the scroll window.
  */
-class FakeProcessor : LazyListUpdateProcessor<FakeProcessor.StringCell, String>() {
+class FakeUpdateProcessor : LazyListUpdateProcessor<FakeUpdateProcessor.StringCell, String>() {
   private var dataSize = 0
   private var scrollWindowOffset = 0
   private val scrollWindowCells = mutableListOf<StringCell>()

redwood-lazylayout-widget/src/commonTest/kotlin/app/cash/redwood/lazylayout/widget/LazyListScrollProcessorTest.kt

@@ -0,0 +1,96 @@
+/*
+ * Copyright (C) 2023 Square, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package app.cash.redwood.lazylayout.widget
+
+import app.cash.redwood.lazylayout.api.ScrollItemIndex
+import assertk.assertThat
+import assertk.assertions.containsExactly
+import assertk.assertions.isEmpty
+import kotlin.test.Test
+
+class LazyListScrollProcessorTest {
+  private val processor = FakeScrollProcessor()
+
+  @Test
+  fun programmaticScrollDeferredUntilOnEndChanges() {
+    processor.size = 30
+
+    // Don't apply the scroll immediately; it should be held until onEndChanges(). Otherwise we'll
+    // scroll while we're changing the list's content.
+    processor.scrollItemIndex(ScrollItemIndex(0, 10))
+    assertThat(processor.takeEvents()).isEmpty()
+
+    processor.onEndChanges()
+    assertThat(processor.takeEvents()).containsExactly("programmaticScroll(10)")
+  }
+
+  @Test
+  fun programmaticScrollDeferredUntilItsWithinContentSize() {
+    // Don't apply the scroll because we don't have enough rows!
+    processor.scrollItemIndex(ScrollItemIndex(0, 10))
+    processor.onEndChanges()
+    assertThat(processor.takeEvents()).isEmpty()
+
+    // Once we have enough rows we can apply the scroll.
+    processor.size = 30
+    processor.onEndChanges()
+    assertThat(processor.takeEvents()).containsExactly("programmaticScroll(10)")
+  }
+
+  @Test
+  fun programmaticScrollDiscardedAfterUserScroll() {
+    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()).isEmpty()
+  }
+
+  @Test
+  fun programmaticScrollOnlyTriggeredOnce() {
+    processor.size = 30
+
+    processor.scrollItemIndex(ScrollItemIndex(0, 10))
+    processor.onEndChanges()
+    assertThat(processor.takeEvents()).containsExactly("programmaticScroll(10)")
+
+    // Confirm onEndIndex() only applies its change once.
+    processor.onEndChanges()
+    assertThat(processor.takeEvents()).isEmpty()
+  }
+
+  @Test
+  fun userScrollDeduplicated() {
+    processor.size = 30
+
+    // Do a user scroll.
+    processor.onUserScroll(5, 14)
+    assertThat(processor.takeEvents()).containsExactly("userScroll(5, 14)")
+
+    // Another scroll with no change in visibility triggers no updates.
+    processor.onUserScroll(5, 14)
+    assertThat(processor.takeEvents()).isEmpty()
+
+    // But a proper scroll will trigger updates.
+    processor.onUserScroll(6, 15)
+    assertThat(processor.takeEvents()).containsExactly("userScroll(6, 15)")
+  }
+}

redwood-lazylayout-widget/src/commonTest/kotlin/app/cash/redwood/lazylayout/widget/LazyListUpdateProcessorTest.kt

@@ -22,7 +22,7 @@ import assertk.assertions.isEqualTo
 import kotlin.test.Test
 
 class LazyListUpdateProcessorTest {
-  private val processor = FakeProcessor()
+  private val processor = FakeUpdateProcessor()
     .apply {
       for (i in 0 until 10) {
         placeholder.insert(i, StringWidget("."))