Skip to content

Commit fe16cb1

Browse files
tmmampagent
andauthored
fix(zero-swift): keep rows on screen in place when rows added above push taller rows out of the page at the end (#102)
Amp-Thread-ID: https://ampcode.com/threads/T-0b362c1a-8212-40b1-91d0-00c96e61b977 Co-authored-by: Amp <amp@ampcode.com>
1 parent cb11e82 commit fe16cb1

3 files changed

Lines changed: 78 additions & 4 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'zero-swift': patch
3+
---
4+
5+
`VirtualList` and `VirtualListView` no longer move the rows on screen when rows are added above them while the list rests at its end, past its first page. The added rows pushed the last rows out of the page. Their estimated height then took their place below the page, so when they were taller than estimated, the content got shorter under the scroll position and the view clamped it. Once the next page brought those rows back, everything on screen had shifted by the difference. The space below the page now keeps the height the rows were laid out at until the page reaches them again.

‎swift/Sources/ZeroVirtual/VirtualizerCore.swift‎

Lines changed: 46 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,13 @@ package final class VirtualizerCore<
278278
// which would stop a fling. Space short of the estimate is given back with
279279
// a scroll once the list rests.
280280
private var spaceAdjust = 0.0
281+
// How far the space after the rows is off its estimate: rows that leave
282+
// the window's end, as rows added above push them out of the page, keep
283+
// the space they were laid out in until the window reaches them again.
284+
// Their estimate in its place could be shorter, which shrinks the content
285+
// under a list parked at its end: the view clamps the offset, and what's
286+
// on screen jumps.
287+
private var spaceAfterAdjust = 0.0
281288
// The list opened away from its start (a permalink, a restore) and hasn't
282289
// been there since: its indexes count from where it opened.
283290
private var openedAway: Bool
@@ -376,10 +383,39 @@ package final class VirtualizerCore<
376383
return
377384
}
378385
pendingReanchor = nil
379-
let oldSpace = manual && host != nil ? snapshot.spaceBefore : nil
386+
let oldSpace = manual && host != nil ? (snapshot.spaceBefore, snapshot.spaceAfter) : nil
380387
rows = new
381388
version += 1
382-
if let oldSpace { carrySpace(from: old, oldSpace) }
389+
if let oldSpace {
390+
carrySpace(from: old, oldSpace.0)
391+
carrySpaceAfter(from: old, oldSpace.1)
392+
}
393+
}
394+
395+
/// Keeps the space after the rows where it was through rows leaving the
396+
/// window's end: the rows gone, as laid out, stand in it. Any other
397+
/// change to the window's end goes back to the estimate.
398+
private func carrySpaceAfter(from old: RowsSnapshot<Row, StartRow>, _ oldSpace: Double) {
399+
let oldEnd = old.firstRowIndex + old.rowsLength
400+
let newEnd = rows.firstRowIndex + rows.rowsLength
401+
guard isListContextCurrent, !rows.atEnd, !old.rowsEmpty, !rows.rowsEmpty,
402+
let oldLast = old.rowAt(oldEnd - 1).map(options.rowKey),
403+
let newLast = rows.rowAt(newEnd - 1).map(options.rowKey)
404+
else {
405+
spaceAfterAdjust = 0
406+
return
407+
}
408+
if oldLast == newLast { return }
409+
guard
410+
(0..<old.rowsLength).contains(where: {
411+
old.rowAt(old.firstRowIndex + $0).map(options.rowKey) == newLast
412+
}), let host, let from = frame(host, key: AnyHashable(newLast)),
413+
let to = frame(host, key: AnyHashable(oldLast)), to.maxY > from.maxY
414+
else {
415+
spaceAfterAdjust = 0
416+
return
417+
}
418+
spaceAfterAdjust = oldSpace + to.maxY - from.maxY - estimatedSpaceAfter
383419
}
384420

385421
/// Keeps the space before the rows where it was through new rows: less
@@ -754,10 +790,16 @@ package final class VirtualizerCore<
754790
return max(paging.estimatedTotal, end)
755791
}
756792

793+
// The space after the rows, as their count estimates it.
794+
private var estimatedSpaceAfter: Double {
795+
if rows.atEnd { return 0 }
796+
return Double(max(0, effectiveEstimatedTotal - rows.firstRowIndex - rows.rowsLength))
797+
* rowEstimate
798+
}
799+
757800
private func buildSnapshot() -> VirtualSnapshot<Row, Key> {
758801
let total = effectiveEstimatedTotal
759802
let end = rows.firstRowIndex + rows.rowsLength
760-
let after = rows.atEnd ? 0 : max(0, total - end)
761803
var items: [VirtualItem<Row, Key>] = []
762804
items.reserveCapacity(rows.rowsLength)
763805
for index in rows.firstRowIndex..<end {
@@ -769,7 +811,7 @@ package final class VirtualizerCore<
769811
// At the start, space left over from above stands before the first row
770812
// until the list rests.
771813
s.spaceBefore = max(0, estimatedSpaceBefore + spaceAdjust)
772-
s.spaceAfter = Double(after) * rowEstimate
814+
s.spaceAfter = rows.atEnd ? 0 : max(0, estimatedSpaceAfter + spaceAfterAdjust)
773815
s.complete = rows.complete
774816
s.rowsEmpty = rows.rowsEmpty
775817
s.permalinkNotFound = rows.permalinkNotFound

‎swift/Tests/ZeroVirtualTests/LayoutHostTests.swift‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,33 @@ final class LayoutScrollView {
177177
#expect(view.rowTop(key) == top)
178178
}
179179

180+
@Test("rows added above, at the end of a list past its first page, don't move what's on screen")
181+
func insertsAbovePushTallRowsOutOfThePage() {
182+
// Parked at the end, rows added above push the last rows, on screen and
183+
// taller than estimated, out of the page: the space after the rows
184+
// keeps their height, not their estimate, or the content shrinks under
185+
// the offset and the view clamps it.
186+
let h = Harness(rowCount: 300) {
187+
$0.anchoring = .manual
188+
$0.minPageSize = 40
189+
}
190+
let view = LayoutScrollView(h)
191+
for i in 290..<300 { view.heights["r\(i)"] = 60 }
192+
view.settle()
193+
// To the end, a page at a time.
194+
for _ in 0..<20 {
195+
view.userScroll(.infinity)
196+
view.settle()
197+
}
198+
let key = h.core.firstVisibleItem()!.key
199+
let top = view.rowTop(key)
200+
h.data.insert(contentsOf: (0..<25).map { TestRow(id: "x\($0)") }, at: 280)
201+
view.settle(holding: key)
202+
h.scheduler.advance(by: 200)
203+
view.settle(holding: key)
204+
#expect(view.rowTop(key) == top)
205+
}
206+
180207
@Test("at the start, rows shorter than estimated above take their space with them, not at rest")
181208
func shrinkageAtTheStart() {
182209
let h = Harness(rowCount: 300) { $0.anchoring = .manual }

0 commit comments

Comments
 (0)