Add ScrollAnimation for duration-based scrolls - #628
Conversation
f848587 to
5d0ecaa
Compare
UIScrollView exposes only a Bool for setContentOffset(_:animated:), so a programmatic scroll was either instant or ran at a fixed system speed. A caller that needs the scroll to take a specific amount of time — one pacing a sequence of steps a user is being walked through, say — had no way to ask for it. Add ScrollAnimation, with .none, .system and .duration(_:), and give every animated: Bool scrolling method on ListView and ListActions.Scrolling an animation: counterpart. animated: true and false remain equivalent to .system and .none, so existing callers are unaffected. A .duration scroll is driven by a CADisplayLink, which assigns contentOffset on every frame. The widely cited alternative — animating contentOffset inside a UIView animation block — does not work: the assignment lands on the target immediately, so the list lays out its content there and recycles every cell it was showing, and only the layer's bounds animate back across a region that has nothing in it. The scroll is smooth and completely blank. Assigning per frame is what UIScrollView does for its own animation, and it drives the layout passes that keep content on screen throughout. The duration is honored even when the target has not been laid out yet. Those scrolls are deferred until the presentation state catches up, so the animation has to wrap the deferred content offset change rather than the initial request. For the same reason the animation is resolved against UIView.areAnimationsEnabled where the scroll is requested: the deferred work runs on a later runloop pass, outside any performWithoutAnimation block the caller made the request from, where animations read as enabled again. The curve is a hardcoded ease-in-out, matching what UIScrollView does for its own scroll. It is a cosine rather than UIKit's bezier because a CAMediaTimingFunction only exposes its control points, not its value at a given time, and a Newton-iteration bezier solver is a lot of machinery to carry for the 2% of fidelity that buys. A driven scroll is interrupted by the same things that interrupt the scroll view's own animation — the user taking hold of the list, or another scroll replacing it — and reports its completion handler either way. Route scrollToTop and scrollToLastItem through the same path, which also gives them the completion handlers they never had, and fix a completion handler being stranded when animated: true was requested with animations disabled. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
5d0ecaa to
dc3c97c
Compare
johnnewman-square
left a comment
There was a problem hiding this comment.
Neat! This change looks good on my end and the demo works great. I left a couple comments: the one from my agent about the scroll completion handler feels like one worth investigating before we merge.
| let startOffset = collectionView.contentOffset | ||
|
|
||
| changeOffset(false) | ||
|
|
||
| let targetOffset = collectionView.contentOffset | ||
|
|
||
| // Back to where the scroll started, so the driver can animate the distance | ||
| // itself. Restoring here rather than on the driver's first frame avoids | ||
| // displaying a frame at the target. | ||
| collectionView.setContentOffset(startOffset, animated: false) | ||
|
|
||
| driveScroll(to: targetOffset, duration: duration, completion: completion) |
There was a problem hiding this comment.
Not a blocker at all, but one edge case occurred to me: if the cell heights dynamically change during the driven scroll animation, we may animate to the wrong end location. It might be difficult to add a test for this case and I'm happy to treat it as an edge case.
There was a problem hiding this comment.
🤖 Agreed it's an edge case, and worth noting the system animation has it too: setContentOffset(_:animated:) commits to a target offset up front just as the driver does, so content that resizes mid-scroll can land off-target either way. The driven scroll is no worse than .system here rather than introducing a new failure mode, which is why I've left it. Clamping to the content size each frame would diverge from what the caller asked for, so if this ever needs solving I'd rather it be solved for both paths at once.
🤖 Addressed by Claude Code
setContentOffset(_:animated: true) with the offset the scroll view is already at starts no animation, so scrollViewDidEndScrollingAnimation(_:) never arrives and a handler queued for it waits forever. scrollToTop and scrollToLastItem take a completion handler as of this change, which is what exposes this: scrolling to the top while already at the top, or to the last item while already at the bottom, moves nothing. Both now report the way an unanimated scroll reports, on the next runloop pass. The two applyScroll variants need different treatment. The one given an explicit target offset can compare it against the current offset and answer immediately. The one given a closure cannot, because only the collection view knows where scrollToItem(at:at:animated:) and scrollRectToVisible(_:animated:) land, and an animated call leaves the content offset at its start value either way. That variant performs the change unanimated to discover the destination, then restores — the same trick the duration path already uses. Since that costs a layout pass, it runs only when a completion handler was supplied and so only for callers who would otherwise be stranded. The duration path was already correct; driveScroll has always short circuited when there is no distance to cover. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
UIScrollViewgives you aBoolforsetContentOffset(_:animated:)and nothing else, so a programmatic Listable scroll is either instant or runs at a fixed system speed. This addsScrollAnimationso a caller can say how long the scroll should take.The motivating case is a guided flow in Square Point of Sale: as the user completes each step, the list scrolls to the next one. At system speed the scroll is fast enough that it reads as a jump, and there's no way to slow it down.
What's here
ScrollAnimationhas three states —.none,.system, and.duration(_:)— and everyanimated: Boolscrolling method onListViewandListActions.Scrollingnow has ananimation:counterpart.animated: trueandfalsemap to.systemand.none, so existing callers behave exactly as before.Two things came along with it, because the new path needed them and the old ones didn't have them:
scrollToTop(...)andscrollToLastItem(...)now take acompletionhandler. They previously had no way to report finishing.animated: truewas requested inside aUIView.performWithoutAnimationblock. The suppressed scroll produced noscrollViewDidEndScrollingAnimation(_:)callback, so the handler queued for it was never called.Why a display link, and not
UIView.animateThe usual suggestion for this problem is:
That animates, but on a
UICollectionViewthe scroll is completely blank. The assignment lands the realcontentOffseton the target immediately, so the collection view lays out at the destination and recycles every cell it was showing. Only the layer'sboundsanimate back — across a region that no longer has any cells in it. I had this built and passing tests before noticing it on screen; at ~100 rows and at ~800 rows of travel, content vanished for the whole animation and reappeared at the end.Unit tests can't see it, which is worth knowing if this code is ever revisited:
layer.animationKeys()containsbounds.originandlayer.presentation()interpolates correctly the entire time the screen is empty.So
ScrollAnimationDriverdoes whatUIScrollViewdoes for its own animation — assignscontentOffseton every frame from aCADisplayLink. Each assignment drives a layout pass, so cells dequeue as they scroll into view.ListViewTestshas a regression test for this specifically (test_scroll_with_duration_animation_keeps_content_laid_out), asserting that the visible-item set mid-scroll is both non-empty and different from the set at the end — content laid out only at the destination would report the same items throughout.The curve is a hardcoded ease-in-out, matching what
UIScrollViewdoes for its own scroll. It's a cosine rather than UIKit's bezier because aCAMediaTimingFunctiononly exposes its control points, not its value at a given time — solving the bezier per frame would mean carrying a Newton-iteration solver in Listable to land within 2% of where a one-line closed form already lands. There's no parameter for it: an exposed curve would have needed a bezier solver to be honest about the names, and no caller wanted one.Two details worth a reviewer's attention
Where the animation is resolved. A scroll toward content that hasn't been laid out yet is deferred until the presentation state catches up, and that deferred work runs on a later runloop pass — outside any
performWithoutAnimationblock the caller made the request from, whereUIView.areAnimationsEnabledreadstrueagain. So the animation is resolved against the current context where the scroll is requested, not where the offset changes. Resolving late would silently ignore a caller's request not to animate.Interruption. A driven scroll is stopped by the same things that stop the scroll view's own animation — the user taking hold of the list (
scrollViewWillBeginDragging), or another scroll superseding it — and reports its completion handler either way. It's also cancelled when the list's identifier changes, since the offset reset there invalidates the target, and inListView.deinit, because the main runloop retains the display link and would otherwise keep driving a torn down list.Testing
Nine tests in
ListViewTestsunder// MARK: Scroll animations, covering completion-exactly-once, offset parity with.system, mid-scroll sampling proving it eases rather than jumps, the blank-scroll regression above, cancellation, non-positive durations, suppressed animations, and that the driver doesn't retain itself while animating.Worth noting for anyone extending these: a display-link animation is immune to
layer.speed, so thelayer.speed = 4thatshow(vc:)applies to the test host doesn't affect it. Durations in these tests are real wall-clock seconds.Also verified by hand in the demo app.
ScrollToSectionCompletionHandlerViewControllerand its newScrollToOffscreenSectionCompletionHandlerViewControllersubclass (40 sections, so the far ones are nowhere near the initial layout) both have an animation picker with None / System / 1s / 3s.Checklist
Please do the following before merging:
Mainsection.