From dc3c97c443f4fe4571e3b1edfb7a5ae9c2b387a4 Mon Sep 17 00:00:00 2001 From: Max Meyers Date: Thu, 30 Jul 2026 16:52:43 -0700 Subject: [PATCH 1/2] Add ScrollAnimation for duration-based scrolls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- CHANGELOG.md | 18 + ...crollCompletionHandlerViewController.swift | 136 ++++-- .../Demos/DemosRootViewController.swift | 8 + .../Internal/ScrollAnimationDriver.swift | 148 ++++++ ListableUI/Sources/ListActions.swift | 193 +++++++- .../Sources/ListView/ListView.Delegate.swift | 16 +- ListableUI/Sources/ListView/ListView.swift | 455 +++++++++++++++--- ListableUI/Sources/ScrollAnimation.swift | 85 ++++ ListableUI/Sources/ViewAnimation.swift | 4 +- ListableUI/Tests/ListView/ListViewTests.swift | 262 ++++++++++ 10 files changed, 1177 insertions(+), 148 deletions(-) create mode 100644 ListableUI/Sources/Internal/ScrollAnimationDriver.swift create mode 100644 ListableUI/Sources/ScrollAnimation.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 9806c6ed9..2021c7083 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,8 +2,26 @@ ### Fixed +- Fixed a scroll completion handler being stranded when an `animated: true` scroll was requested inside a `UIView.performWithoutAnimation` block. The suppressed scroll never produced a `scrollViewDidEndScrollingAnimation(_:)` callback, so the handler it was queued for was never reported. + ### Added +- Added `ScrollAnimation`, which lets a programmatic scroll run over a specific duration. `UIScrollView` provides no way to configure its built-in animation, so previously a scroll was either instant or ran at a fixed system speed. + ```swift + list.scrollToSection( + with: sectionIdentifier, + scrollPosition: ScrollPosition(position: .top), + animation: .duration(0.4) + ) + ``` + Every `animated: Bool` scrolling method on `ListView` and `ListActions.Scrolling` now has an `animation: ScrollAnimation` counterpart. `animated: true` and `false` remain equivalent to `.system` and `.none`, so existing callers are unaffected. + + A duration is honored even when the target has not been laid out yet — the scroll is deferred until the presentation state catches up, and the animation wraps that deferred content offset change rather than the initial request. The list advances its content offset on each frame, the same as it does for the system animation, so content stays laid out for the length of the scroll. + + A `.duration` 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. `ListActions.Scrolling.cancelScrollAnimation()` stops one explicitly. + +- Added `completion` handlers to `scrollToTop(...)` and `scrollToLastItem(...)`, which previously had no way to report when they finished. + ### Removed ### Changed diff --git a/Development/Sources/Demos/Demo Screens/ScrollCompletionHandlerViewController.swift b/Development/Sources/Demos/Demo Screens/ScrollCompletionHandlerViewController.swift index 56f41fcd0..7f17eb2bb 100644 --- a/Development/Sources/Demos/Demo Screens/ScrollCompletionHandlerViewController.swift +++ b/Development/Sources/Demos/Demo Screens/ScrollCompletionHandlerViewController.swift @@ -21,8 +21,8 @@ class ScrollCompletionHandlerViewController : UIViewController { fileprivate var sections: [Section] { [] } - fileprivate var animateScroll: Bool = true - + fileprivate var scrollAnimation: ScrollAnimation = .system + fileprivate var scrollPosition: ScrollPosition.Position = .top fileprivate var ifAlreadyVisible: ScrollPosition.IfAlreadyVisible = .scrollToPosition @@ -37,10 +37,6 @@ class ScrollCompletionHandlerViewController : UIViewController { UIBarButtonItem(title: "Axis", style: .plain, target: self, action: #selector(toggleDirection)) }() - fileprivate lazy var animationsButton: UIBarButtonItem = { - UIBarButtonItem(title: "Toggle Animations", style: .plain, target: self, action: #selector(toggleAnimations)) - }() - override func viewDidLoad() { super.viewDidLoad() let stackView = UIStackView(arrangedSubviews: [list, settingsPanel]) @@ -73,11 +69,6 @@ class ScrollCompletionHandlerViewController : UIViewController { assertionFailure("Override in subclasses.") } - @objc func toggleAnimations() { - animateScroll.toggle() - print("Scroll animations are \(animateScroll ? "on" : "off").") - } - @objc func toggleDirection() { if layoutDirection == .horizontal { layoutDirection = .vertical @@ -88,7 +79,7 @@ class ScrollCompletionHandlerViewController : UIViewController { } fileprivate var settingsControls: [UIView] { - [selectionPanel, alreadyVisiblePanel, positionPanel] + [selectionPanel, alreadyVisiblePanel, positionPanel, animationPanel] } /// This view contains all the configurable scroll settings. @@ -147,6 +138,30 @@ class ScrollCompletionHandlerViewController : UIViewController { return titledView(control, title: "If Visbile") }() + /// The label and segmented control for selecting the scroll animation. The durations + /// are deliberately slow enough to watch, since the point of `.duration` is to scroll + /// at a speed the system animation does not offer. + lazy var animationPanel: UIView = { + let control = UISegmentedControl( + items: [ + UIAction(title: "None") { [weak self] _ in + self?.scrollAnimation = .none + }, + UIAction(title: "System") { [weak self] _ in + self?.scrollAnimation = .system + }, + UIAction(title: "1s") { [weak self] _ in + self?.scrollAnimation = .duration(1) + }, + UIAction(title: "3s") { [weak self] _ in + self?.scrollAnimation = .duration(3) + }, + ] + ) + control.selectedSegmentIndex = 1 + return titledView(control, title: "Animation") + }() + /// The label and segmented control for selecting the scrolled item/section. /// Override in subclasses. fileprivate var selectionPanel: UIView { @@ -154,6 +169,16 @@ class ScrollCompletionHandlerViewController : UIViewController { return UIView() } + /// Prints the items that were on screen when the scroll finished. + fileprivate var printScrollCompletion: ListView.ScrollCompletion { + { changes in + let sortedItems = changes.positionInfo.visibleItems + .map { "\($0.identifier) "} + .sorted() + print("Scroll completion: \(sortedItems)") + } + } + /// A helper to add a label before `view`. fileprivate func titledView(_ view: UIView, title: String) -> UIView { let label = UILabel() @@ -206,9 +231,9 @@ class ScrollToItemCompletionHandlerViewController: ScrollCompletionHandlerViewCo super.viewDidLoad() // TODO: Fully add support for programmatic scrolling in horizontal layouts. // The axisButton is used in this demo because there are no section headers. - navigationItem.rightBarButtonItems = [scrollButton, animationsButton, axisButton] + navigationItem.rightBarButtonItems = [scrollButton, axisButton] } - + override func performScroll() { list.scrollTo( item: scrolledItem, @@ -216,28 +241,30 @@ class ScrollToItemCompletionHandlerViewController: ScrollCompletionHandlerViewCo position: scrollPosition, ifAlreadyVisible: ifAlreadyVisible ), - animated: animateScroll, - completion: { changes in - let sortedItems = changes.positionInfo.visibleItems - .map { "\($0.identifier) "} - .sorted() - print("Scroll completion: \(sortedItems)") - } + animation: scrollAnimation, + completion: printScrollCompletion ) } } /// A demo for showcasing scrolling to a particular section. class ScrollToSectionCompletionHandlerViewController: ScrollCompletionHandlerViewController { - + + /// How many sections the list has. Subclasses can raise this to move the far sections + /// out of the initially laid out content. + fileprivate var sectionCount: Int { 3 } + + /// How many items each section has. + fileprivate var itemsPerSection: Int { 101 } + override var sections: [Section] { _sections } - + private lazy var _sections: [Section] = { - (0...2).map { sectionIndex in + (0.. Void + + private let displayLinkTarget = DisplayLinkTarget() + + private var displayLink: CADisplayLink? + private var startTimestamp: CFTimeInterval? + private var hasStarted = false + + init( + scrollView: UIScrollView, + from startOffset: CGPoint, + to targetOffset: CGPoint, + duration: TimeInterval, + completion: @escaping () -> Void + ) { + self.scrollView = scrollView + self.startOffset = startOffset + self.targetOffset = targetOffset + self.duration = duration + self.completion = completion + + displayLinkTarget.driver = self + } + + deinit { + displayLink?.invalidate() + } + + /// Begins animating. The scroll view's content offset is expected to already be at + /// `startOffset`, so the first offset this applies is on the frame after the one the + /// scroll was requested during. + /// + /// A driver animates once and is then spent; starting it again does nothing. Restarting + /// could not do the right thing anyway, since `startOffset` was captured at init and the + /// content has since moved away from it. + func start() { + guard hasStarted == false else { + assertionFailure("A scroll animation cannot be started twice.") + return + } + + hasStarted = true + + let displayLink = CADisplayLink( + target: displayLinkTarget, + selector: #selector(DisplayLinkTarget.tick) + ) + + displayLink.add(to: .main, forMode: .common) + + self.displayLink = displayLink + } + + /// Stops the animation where it is, leaving the content offset untouched, and reports + /// its completion. + func cancel() { + guard displayLink != nil else { return } + + finish() + } + + fileprivate func tick(_ displayLink: CADisplayLink) { + guard let scrollView else { + finish() + return + } + + // The first tick lands a frame's worth of time after the animation began, so it + // establishes the clock rather than advancing it. Advancing here instead would + // skip the start of the animation. + guard let startTimestamp else { + startTimestamp = displayLink.timestamp + return + } + + let elapsed = displayLink.timestamp - startTimestamp + + guard elapsed < duration else { + scrollView.setContentOffset(targetOffset, animated: false) + finish() + return + } + + let progress = Self.eased(elapsed / duration) + + scrollView.setContentOffset( + CGPoint( + x: startOffset.x + (targetOffset.x - startOffset.x) * progress, + y: startOffset.y + (targetOffset.y - startOffset.y) * progress + ), + animated: false + ) + } + + private func finish() { + displayLink?.invalidate() + displayLink = nil + + completion() + } + + /// `UIScrollView` eases its own animation in and out, and so does this. This curve stays + /// within ~2% of UIKit's `.curveEaseInOut`, which cannot be evaluated directly — a + /// `CAMediaTimingFunction` only exposes its control points, not the value at a given time. + private static func eased(_ progress: Double) -> Double { + (1 - cos(.pi * progress)) / 2 + } +} + + +/// `CADisplayLink` retains its target and the runloop retains the link, so a driver that +/// targeted itself could not be deallocated while its animation was in flight — its +/// `deinit` would never run to invalidate the link. Targeting this instead keeps the only +/// reference back to the driver a weak one. +private final class DisplayLinkTarget { + + weak var driver: ScrollAnimationDriver? + + @objc func tick(_ displayLink: CADisplayLink) { + driver?.tick(displayLink) + } +} diff --git a/ListableUI/Sources/ListActions.swift b/ListableUI/Sources/ListActions.swift index 9a2266aa5..f503aa3c4 100644 --- a/ListableUI/Sources/ListActions.swift +++ b/ListableUI/Sources/ListActions.swift @@ -110,6 +110,30 @@ public final class ListActions { ) } + /// + /// Scrolls to the provided item, with the provided positioning, using the provided animation. + /// If the item is contained in the list, true is returned. If it is not, false is returned. + /// + @discardableResult + public func scrollTo( + item : AnyItem, + position : ScrollPosition, + animation : ScrollAnimation, + completion: ScrollCompletion? = nil + ) -> Bool + { + guard let listView = self.listView else { + return false + } + + return listView.scrollTo( + item: item, + position: position, + animation: animation, + completion: completion + ) + } + /// /// Scrolls to a custom vertical offset for the provided item. /// The adjustment receives the item's frame and visible content frame, @@ -131,7 +155,29 @@ public final class ListActions { completion: completion ) } - + + /// + /// Scrolls to a custom vertical offset for the provided item, using the provided animation. + /// The adjustment receives the item's frame and visible content frame, + /// then returns the vertical delta to apply. + /// If the item is contained in the list, true is returned. If it is not, false is returned. + /// + @discardableResult + public func scrollTo( + item : AnyItem, + contentOffsetAdjustment : @escaping ListItemScrollPositionAdjustment, + animation : ScrollAnimation, + completion: ScrollCompletion? = nil + ) -> Bool + { + self.scrollTo( + item: item.anyIdentifier, + contentOffsetAdjustment: contentOffsetAdjustment, + animation: animation, + completion: completion + ) + } + /// /// Scrolls to the item with the provided identifier, with the provided positioning. /// If there is more than one item with the same identifier, the list scrolls to the first. @@ -157,6 +203,32 @@ public final class ListActions { ) } + /// + /// Scrolls to the item with the provided identifier, with the provided positioning, + /// using the provided animation. + /// If there is more than one item with the same identifier, the list scrolls to the first. + /// If the item is contained in the list, true is returned. If it is not, false is returned. + /// + @discardableResult + public func scrollTo( + item : AnyIdentifier, + position : ScrollPosition, + animation : ScrollAnimation, + completion: ScrollCompletion? = nil + ) -> Bool + { + guard let listView = self.listView else { + return false + } + + return listView.scrollTo( + item: item, + position: position, + animation: animation, + completion: completion + ) + } + /// /// Scrolls to a custom vertical offset for the item with the provided identifier. /// The adjustment receives the item's frame and visible content frame, @@ -184,6 +256,34 @@ public final class ListActions { ) } + /// + /// Scrolls to a custom vertical offset for the item with the provided identifier, + /// using the provided animation. + /// The adjustment receives the item's frame and visible content frame, + /// then returns the vertical delta to apply. + /// If there is more than one item with the same identifier, the list scrolls to the first. + /// If the item is contained in the list, true is returned. If it is not, false is returned. + /// + @discardableResult + public func scrollTo( + item : AnyIdentifier, + contentOffsetAdjustment : @escaping ListItemScrollPositionAdjustment, + animation : ScrollAnimation, + completion: ScrollCompletion? = nil + ) -> Bool + { + guard let listView = self.listView else { + return false + } + + return listView.scrollTo( + item: item, + contentOffsetAdjustment: contentOffsetAdjustment, + animation: animation, + completion: completion + ) + } + /// /// Scrolls to the section with the given identifier, with the provided scroll and section positioning. /// @@ -220,36 +320,115 @@ public final class ListActions { completion: completion ) } + + /// + /// Scrolls to the section with the given identifier, with the provided scroll and + /// section positioning, using the provided animation. + /// + /// See `scrollToSection(with:sectionPosition:scrollPosition:animated:completion:)` + /// for how the list picks which part of the section to scroll to. + /// + @discardableResult + public func scrollToSection( + with identifier : AnyIdentifier, + sectionPosition : SectionPosition = .top, + scrollPosition : ScrollPosition, + animation: ScrollAnimation, + completion: ScrollCompletion? = nil + ) -> Bool + { + guard let listView = self.listView else { + return false + } + + return listView.scrollToSection( + with: identifier, + sectionPosition: sectionPosition, + scrollPosition: scrollPosition, + animation: animation, + completion: completion + ) + } /// Scrolls to the very top of the list, which includes displaying the list header. @discardableResult public func scrollToTop( - animated: Bool = false + animated: Bool = false, + completion: ScrollCompletion? = nil ) -> Bool { guard let listView = self.listView else { return false } - + return listView.scrollToTop( - animated: animated + animated: animated, + completion: completion + ) + } + + /// Scrolls to the very top of the list, which includes displaying the list header, + /// using the provided animation. + @discardableResult + public func scrollToTop( + animation: ScrollAnimation, + completion: ScrollCompletion? = nil + ) -> Bool + { + guard let listView = self.listView else { + return false + } + + return listView.scrollToTop( + animation: animation, + completion: completion ) } /// Scrolls to the last item in the list. If the list contains no items, no action is performed. @discardableResult public func scrollToLastItem( - animated: Bool = false + animated: Bool = false, + completion: ScrollCompletion? = nil ) -> Bool { guard let listView = self.listView else { return false } - + + return listView.scrollToLastItem( + animated: animated, + completion: completion + ) + } + + /// Scrolls to the last item in the list, using the provided animation. + /// If the list contains no items, no action is performed. + @discardableResult + public func scrollToLastItem( + animation: ScrollAnimation, + completion: ScrollCompletion? = nil + ) -> Bool + { + guard let listView = self.listView else { + return false + } + return listView.scrollToLastItem( - animated: animated + animation: animation, + completion: completion ) } + + /// Stops a scroll animation the list is driving itself, where it is. + /// + /// This has no effect on the scroll view's own animation, which is stopped by the + /// user taking hold of the list. Only a `ScrollAnimation.duration` scroll is + /// affected. Its completion handler is reported as it would be on a normal finish. + public func cancelScrollAnimation() + { + self.listView?.cancelScrollAnimation() + } } /// Provides access to view controller transitioning options in a list. diff --git a/ListableUI/Sources/ListView/ListView.Delegate.swift b/ListableUI/Sources/ListView/ListView.Delegate.swift index 18b8fc846..418b4a26d 100644 --- a/ListableUI/Sources/ListView/ListView.Delegate.swift +++ b/ListableUI/Sources/ListView/ListView.Delegate.swift @@ -100,17 +100,7 @@ extension ListView func scrollViewDidEndScrollingAnimation(_ scrollView: UIScrollView) { - let scrollCompletions = self.view.drainScrollCompletionHandlers() - - ListStateObserver.perform(self.view.stateObserver.onDidEndScrollingAnimation, "Did End Scrolling Animation", with: self.view) { actions in - ListStateObserver.DidEndScrollingAnimation( - actions: actions, - positionInfo: self.view.scrollPositionInfo - ) - } - - // Notify the ListView that scrolling ended. - self.view.performScrollCompletions(scrollCompletions) + self.view.didEndScrollingAnimation() } private var oldSelectedItems : Set = [] @@ -331,6 +321,10 @@ extension ListView { self.view.isUserScrollInProgress = true + // The user taking hold of the list ends a programmatic scroll, the same way it + // ends the scroll view's own scrolling animation. + self.view.cancelScrollAnimation() + self.view.liveCells.perform { $0.closeSwipeActions() } diff --git a/ListableUI/Sources/ListView/ListView.swift b/ListableUI/Sources/ListView/ListView.swift index 38eed10f6..8b34e16f1 100644 --- a/ListableUI/Sources/ListView/ListView.swift +++ b/ListableUI/Sources/ListView/ListView.swift @@ -137,9 +137,13 @@ public final class ListView : UIView } deinit - { + { self.keyboardObserver.remove(delegate: self) - + + // A driven scroll animation would otherwise keep running against a torn down list, + // because the display link driving it is retained by the main runloop. + self.cancelScrollAnimation() + /** Even though these are zeroing weak references in UIKIt as of iOS 9.0, @@ -560,6 +564,26 @@ public final class ListView : UIView ) } + /// + /// Scrolls to the provided item, with the provided positioning, using the provided animation. + /// If the item is contained in the list, true is returned. If it is not, false is returned. + /// + @discardableResult + public func scrollTo( + item : AnyItem, + position : ScrollPosition, + animation : ScrollAnimation, + completion: ScrollCompletion? = nil + ) -> Bool + { + self.scrollTo( + item: item.anyIdentifier, + position: position, + animation: animation, + completion: completion + ) + } + /// /// Scrolls to a custom vertical offset for the provided item. /// The adjustment receives the item's frame and visible content frame, @@ -581,6 +605,28 @@ public final class ListView : UIView completion: completion ) } + + /// + /// Scrolls to a custom vertical offset for the provided item, using the provided animation. + /// The adjustment receives the item's frame and visible content frame, + /// then returns the vertical delta to apply. + /// If the item is contained in the list, true is returned. If it is not, false is returned. + /// + @discardableResult + public func scrollTo( + item : AnyItem, + contentOffsetAdjustment : @escaping ListItemScrollPositionAdjustment, + animation : ScrollAnimation, + completion: ScrollCompletion? = nil + ) -> Bool + { + self.scrollTo( + item: item.anyIdentifier, + contentOffsetAdjustment: contentOffsetAdjustment, + animation: animation, + completion: completion + ) + } /// /// Scrolls to the item with the provided identifier, with the provided positioning. @@ -594,6 +640,46 @@ public final class ListView : UIView animated : Bool = false, completion: ScrollCompletion? = nil ) -> Bool + { + scrollTo( + item: item, + position: position, + animation: ScrollAnimation(animated: animated), + completion: completion + ) + } + + /// + /// Scrolls to the item with the provided identifier, with the provided positioning, + /// using the provided animation. + /// If there is more than one item with the same identifier, the list scrolls to the first. + /// If the item is contained in the list, true is returned. If it is not, false is returned. + /// + @discardableResult + public func scrollTo( + item : AnyIdentifier, + position : ScrollPosition, + animation : ScrollAnimation, + completion: ScrollCompletion? = nil + ) -> Bool + { + scrollTo( + item: item, + position: position, + resolvedAnimation: animation.resolvedForCurrentContext(), + completion: completion + ) + } + + /// - Parameter animation: Must already have been resolved against the calling + /// context by `ScrollAnimation.resolvedForCurrentContext()`. + @discardableResult + private func scrollTo( + item : AnyIdentifier, + position : ScrollPosition, + resolvedAnimation animation : ScrollAnimation, + completion: ScrollCompletion? = nil + ) -> Bool { // Make sure the item identifier is valid. @@ -602,9 +688,6 @@ public final class ListView : UIView return false } - // If user is performing this in a `UIView.performWithoutAnimation` block, respect that and don't animate, regardless of what the animated parameter is. - let shouldAnimate = animated && UIView.areAnimationsEnabled - return preparePresentationStateForScroll(to: toIndexPath, handlerWhenFailed: completion) { /// `preparePresentationStateForScroll(to:)` is asynchronous in some @@ -643,30 +726,40 @@ public final class ListView : UIView self.performScroll( to: itemFrameAdjustedForStickyHeaders, scrollPosition: position, - animated: shouldAnimate, + animation: animation, completion: completion ) } else { let scrollPosition = position.position.toUICollectionViewScrollPosition( for: self.collectionViewLayout.layout.direction ) - self.collectionView.scrollToItem( - at: toIndexPath, - at: scrollPosition, - animated: shouldAnimate + + // `willScroll` is derived from the pre-scroll viewport, so it has to be + // evaluated before the content offset changes. + let willScroll = self.willScroll( + for: scrollPosition, + itemFrame: itemFrame, + viewport: viewport.inset(by: self.collectionView.adjustedContentInset), + contentSize: self.contentSize ) - if let completion { - let willScroll = self.willScroll( - for: scrollPosition, - itemFrame: itemFrame, - viewport: viewport.inset(by: self.collectionView.adjustedContentInset), - contentSize: self.contentSize - ) - if willScroll { - self.handleScrollCompletion(reason: .scrolled(animated: animated), completion: completion) - } else { - self.handleScrollCompletion(reason: .cannotScroll, completion: completion) + + if willScroll { + self.applyScroll(with: animation, completion: completion) { animated in + self.collectionView.scrollToItem( + at: toIndexPath, + at: scrollPosition, + animated: animated + ) } + } else { + // The collection view is not expected to move, so there is no + // scroll-end callback to wait on. + self.collectionView.scrollToItem( + at: toIndexPath, + at: scrollPosition, + animated: animation.usesSystemAnimation + ) + self.handleScrollCompletion(reason: .cannotScroll, completion: completion) } } } @@ -687,6 +780,32 @@ public final class ListView : UIView completion: ScrollCompletion? = nil ) -> Bool { + scrollTo( + item: item, + contentOffsetAdjustment: contentOffsetAdjustment, + animation: ScrollAnimation(animated: animated), + completion: completion + ) + } + + /// + /// Scrolls to a custom vertical offset for the item with the provided identifier, + /// using the provided animation. + /// The adjustment receives the item's frame and visible content frame, + /// then returns the vertical delta to apply. + /// If there is more than one item with the same identifier, the list scrolls to the first. + /// If the item is contained in the list, true is returned. If it is not, false is returned. + /// + @discardableResult + public func scrollTo( + item : AnyIdentifier, + contentOffsetAdjustment : @escaping ListItemScrollPositionAdjustment, + animation : ScrollAnimation, + completion: ScrollCompletion? = nil + ) -> Bool + { + let animation = animation.resolvedForCurrentContext() + // Make sure the item identifier is valid. guard let toIndexPath = self.storage.allContent.firstIndexPathForItem(with: item) else { @@ -694,9 +813,6 @@ public final class ListView : UIView return false } - // If user is performing this in a `UIView.performWithoutAnimation` block, respect that and don't animate, regardless of what the animated parameter is. - let shouldAnimate = animated && UIView.areAnimationsEnabled - return preparePresentationStateForScroll(to: toIndexPath, handlerWhenFailed: completion) { /// `preparePresentationStateForScroll(to:)` is asynchronous in some @@ -720,7 +836,7 @@ public final class ListView : UIView self.performScroll( toContentOffset: resultOffset, - animated: shouldAnimate, + animation: animation, completion: completion ) } @@ -750,6 +866,33 @@ public final class ListView : UIView completion: ScrollCompletion? = nil ) -> Bool { + scrollToSection( + with: identifier, + sectionPosition: sectionPosition, + scrollPosition: scrollPosition, + animation: ScrollAnimation(animated: animated), + completion: completion + ) + } + + /// + /// Scrolls to the section with the given identifier, with the provided scroll and + /// section positioning, using the provided animation. + /// + /// See `scrollToSection(with:sectionPosition:scrollPosition:animated:completion:)` + /// for how the list picks which part of the section to scroll to. + /// + @discardableResult + public func scrollToSection( + with identifier : AnyIdentifier, + sectionPosition : SectionPosition = .top, + scrollPosition : ScrollPosition, + animation: ScrollAnimation, + completion: ScrollCompletion? = nil + ) -> Bool + { + let animation = animation.resolvedForCurrentContext() + let storageContent = storage.allContent // Make sure the section identifier is valid. @@ -807,21 +950,24 @@ public final class ListView : UIView self.performScroll( to: footerFrameAdjustedForStickyHeaders ?? targetSupplementaryView.defaultFrame, scrollPosition: scrollPosition, - animated: animated, + animation: animation, completion: completion ) } else if let adjacentItem = adjacentItem { + // This runs inside the presentation state update, where animations are + // suppressed, so the already-resolved animation must be passed straight + // through rather than resolved again. self.scrollTo( - item: adjacentItem, + item: adjacentItem.anyIdentifier, position: scrollPosition, - animated: animated, + resolvedAnimation: animation, completion: completion ) } else { self.performScroll( to: fallbackSupplementaryView.defaultFrame, scrollPosition: scrollPosition, - animated: animated, + animation: animation, completion: completion ) } @@ -831,49 +977,82 @@ public final class ListView : UIView /// Scrolls to the very top of the list, which includes displaying the list header. @discardableResult public func scrollToTop( - animated: Bool = false + animated: Bool = false, + completion: ScrollCompletion? = nil ) -> Bool { - + scrollToTop( + animation: ScrollAnimation(animated: animated), + completion: completion + ) + } + + /// Scrolls to the very top of the list, which includes displaying the list header, + /// using the provided animation. + @discardableResult + public func scrollToTop( + animation: ScrollAnimation, + completion: ScrollCompletion? = nil + ) -> Bool { + + let animation = animation.resolvedForCurrentContext() + // The rect we scroll to must have an area – an empty rect will result in no scrolling. let rect = CGRect(origin: .zero, size: CGSize(width: 1.0, height: 1.0)) - // If user is performing this in a `UIView.performWithoutAnimation` block, respect that and don't animate, regardless of what the animated parameter is. - let shouldAnimate = animated && UIView.areAnimationsEnabled - - return self.preparePresentationStateForScroll(to: IndexPath(item: 0, section: 0), handlerWhenFailed: nil) { - self.collectionView.scrollRectToVisible(rect, animated: shouldAnimate) + return self.preparePresentationStateForScroll(to: IndexPath(item: 0, section: 0), handlerWhenFailed: completion) { + self.applyScroll(with: animation, completion: completion) { animated in + self.collectionView.scrollRectToVisible(rect, animated: animated) + } } } /// Scrolls to the last item in the list. If the list contains no items, no action is performed. @discardableResult public func scrollToLastItem( - animated: Bool = false + animated: Bool = false, + completion: ScrollCompletion? = nil + ) -> Bool { + scrollToLastItem( + animation: ScrollAnimation(animated: animated), + completion: completion + ) + } + + /// Scrolls to the last item in the list, using the provided animation. + /// If the list contains no items, no action is performed. + @discardableResult + public func scrollToLastItem( + animation: ScrollAnimation, + completion: ScrollCompletion? = nil ) -> Bool { // Make sure we have a valid last index path. guard let toIndexPath = self.storage.allContent.lastIndexPath() else { + handleScrollCompletion(reason: .cannotScroll, completion: completion) return false } - // If user is performing this in a `UIView.performWithoutAnimation` block, respect that and don't animate, regardless of what the animated parameter is. - let shouldAnimate = animated && UIView.areAnimationsEnabled + let animation = animation.resolvedForCurrentContext() // Perform scrolling. - return self.preparePresentationStateForScroll(to: toIndexPath, handlerWhenFailed: nil) { + return self.preparePresentationStateForScroll(to: toIndexPath, handlerWhenFailed: completion) { let contentHeight = self.collectionViewLayout.collectionViewContentSize.height let contentFrameHeight = self.collectionView.visibleContentFrame.height guard contentHeight > contentFrameHeight else { + self.handleScrollCompletion(reason: .cannotScroll, completion: completion) return } let contentOffsetY = contentHeight - contentFrameHeight - self.collectionView.adjustedContentInset.top - let contentOffset = CGPoint(x: self.collectionView.contentOffset.x, y: contentOffsetY) - - self.collectionView.setContentOffset(contentOffset, animated: shouldAnimate) + + self.applyScroll( + with: animation, + to: CGPoint(x: self.collectionView.contentOffset.x, y: contentOffsetY), + completion: completion + ) } } @@ -885,7 +1064,7 @@ public final class ListView : UIView case cannotScroll case scrolled(animated: Bool) } - + /// This function is used by programmatic scrolling APIs that provide a scroll /// completion handler. This will execute the `completion` handler after scrolling /// is finished, or it will execute immediately if scrolling is not possible or if @@ -914,6 +1093,129 @@ public final class ListView : UIView } } + /// Moves the content offset to `targetOffset` using the provided `animation`, and + /// reports `completion` once it gets there. + /// + /// Callers that have already determined the list cannot scroll should report + /// `.cannotScroll` themselves instead of calling this. + private func applyScroll( + with animation: ScrollAnimation, + to targetOffset: CGPoint, + completion: ScrollCompletion? + ) { + // Whatever this scroll is, it supersedes one that is still running. + cancelScrollAnimation() + + switch animation.storage { + case .none, .system: + collectionView.setContentOffset(targetOffset, animated: animation.usesSystemAnimation) + handleScrollCompletion( + reason: .scrolled(animated: animation.usesSystemAnimation), + completion: completion + ) + + case .duration(let duration): + driveScroll(to: targetOffset, duration: duration, completion: completion) + } + } + + /// Applies a content offset change described by `changeOffset` using the provided + /// `animation`, and reports `completion` once that change has finished. + /// + /// Prefer `applyScroll(with:to:completion:)`. This variant exists for scrolls whose + /// destination only the collection view knows — `scrollToItem(at:at:animated:)` and + /// `scrollRectToVisible(_:animated:)` compute where they land internally, so a driven + /// animation has to perform the change unanimated and read the resulting offset back. + /// + /// `changeOffset` receives whether it should ask the scroll view to animate. + /// + /// Callers that have already determined the list cannot scroll should report + /// `.cannotScroll` themselves instead of calling this. + private func applyScroll( + with animation: ScrollAnimation, + completion: ScrollCompletion?, + changeOffset: (_ animated: Bool) -> Void + ) { + // Whatever this scroll is, it supersedes one that is still running. + cancelScrollAnimation() + + switch animation.storage { + case .none, .system: + changeOffset(animation.usesSystemAnimation) + handleScrollCompletion( + reason: .scrolled(animated: animation.usesSystemAnimation), + completion: completion + ) + + case .duration(let duration): + 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) + } + } + + /// Animates the content offset to `targetOffset` over `duration`, a frame at a time. + private func driveScroll( + to targetOffset: CGPoint, + duration: TimeInterval, + completion: ScrollCompletion? + ) { + let startOffset = collectionView.contentOffset + + guard startOffset != targetOffset else { + handleScrollCompletion(reason: .scrolled(animated: false), completion: completion) + return + } + + // The handler is queued exactly as it is for the scroll view's own animation. The + // driver reports through `didEndScrollingAnimation()` below, which drains it. + handleScrollCompletion(reason: .scrolled(animated: true), completion: completion) + + let driver = ScrollAnimationDriver( + scrollView: collectionView, + from: startOffset, + to: targetOffset, + duration: duration, + completion: { [weak self] in + guard let self else { return } + + self.scrollAnimationDriver = nil + + // Report the end of a driven animation the same way the delegate reports + // the end of the scroll view's own, so an observer cannot tell them apart. + self.didEndScrollingAnimation() + } + ) + + scrollAnimationDriver = driver + + driver.start() + } + + /// The driver for an in-flight `ScrollAnimation.duration` scroll, if there is one. + private var scrollAnimationDriver: ScrollAnimationDriver? + + /// Stops a driven scroll animation where it is, reporting its completion. + /// + /// A driven scroll is stopped by the same things that stop the scroll view's own + /// animation: the user taking hold of the list, or another scroll replacing it. + func cancelScrollAnimation() { + guard let driver = scrollAnimationDriver else { return } + + scrollAnimationDriver = nil + + driver.cancel() + } + private func performScrollCompletion(_ completion: ScrollCompletion, positionInfo: ListScrollPositionInfo) { let actions = ListActions() actions.listView = self @@ -930,11 +1232,20 @@ public final class ListView : UIView let positionInfo: ListScrollPositionInfo } - /// This is called by the `ListView.Delegate` and is used to notify the - /// `scrollCompletionHandler` that scrolling finished. This does nothing if there is - /// no handler set. - internal func didEndScrolling() { - self.performScrollCompletions(self.drainScrollCompletionHandlers()) + /// Called when a scrolling animation ends, whether the scroll view ran the animation + /// or the list drove it itself. Notifies the `ListStateObserver` and any scroll + /// completion handlers. + internal func didEndScrollingAnimation() { + let scrollCompletions = drainScrollCompletionHandlers() + + ListStateObserver.perform(stateObserver.onDidEndScrollingAnimation, "Did End Scrolling Animation", with: self) { actions in + ListStateObserver.DidEndScrollingAnimation( + actions: actions, + positionInfo: self.scrollPositionInfo + ) + } + + performScrollCompletions(scrollCompletions) } internal func drainScrollCompletionHandlers() -> ScrollCompletionBatch? { @@ -1559,6 +1870,10 @@ public final class ListView : UIView switch reason { case .contentChanged(_, let identifierChanged): if identifierChanged { + // The content this list is showing has been replaced, so a scroll toward a + // position within the old content no longer means anything. + cancelScrollAnimation() + let contentOffset = CGPoint(x: 0, y: -collectionView.adjustedContentInset.top) collectionView.setContentOffset(contentOffset, animated: false) } @@ -1726,7 +2041,7 @@ public final class ListView : UIView private func performScroll( to targetFrame : CGRect, scrollPosition : ScrollPosition, - animated: Bool = false, + animation: ScrollAnimation = .none, completion: ScrollCompletion? = nil ) { // If the item is already visible and that's good enough, return. @@ -1737,9 +2052,6 @@ public final class ListView : UIView return } - // If user is performing this in a `UIView.performWithoutAnimation` block, respect that and don't animate, regardless of what the animated parameter is. - let shouldAnimate = animated && UIView.areAnimationsEnabled - let topInset = collectionView.adjustedContentInset.top let contentFrameHeight = collectionView.visibleContentFrame.height let adjustedOriginY = targetFrame.origin.y - topInset @@ -1755,34 +2067,16 @@ public final class ListView : UIView resultOffset.y = adjustedOriginY - (contentFrameHeight - targetFrame.size.height) } - // Don't scroll past the bottom of the list. - - let maxOffsetHeight = collectionViewLayout.collectionViewContentSize.height - contentFrameHeight - topInset - resultOffset.y = min(resultOffset.y, maxOffsetHeight) - - // Don't scroll beyond the top of the list. - - resultOffset.y = max(resultOffset.y, -topInset) - - let roundedResultOffset = CGPoint( - x: round(resultOffset.x), - y: round(resultOffset.y) - ) - let roundedCurrentOffset = CGPoint( - x: round(collectionView.contentOffset.x), - y: round(collectionView.contentOffset.y) + performScroll( + toContentOffset: resultOffset, + animation: animation, + completion: completion ) - if roundedCurrentOffset != roundedResultOffset { - collectionView.setContentOffset(resultOffset, animated: shouldAnimate) - handleScrollCompletion(reason: .scrolled(animated: shouldAnimate), completion: completion) - } else { - handleScrollCompletion(reason: .cannotScroll, completion: completion) - } } private func performScroll( toContentOffset contentOffset : CGPoint, - animated: Bool = false, + animation: ScrollAnimation = .none, completion: ScrollCompletion? = nil ) { let resultOffset = clampedContentOffset(contentOffset) @@ -1795,12 +2089,13 @@ public final class ListView : UIView x: round(collectionView.contentOffset.x), y: round(collectionView.contentOffset.y) ) - if roundedCurrentOffset != roundedResultOffset { - collectionView.setContentOffset(resultOffset, animated: animated) - handleScrollCompletion(reason: .scrolled(animated: animated), completion: completion) - } else { + + guard roundedCurrentOffset != roundedResultOffset else { handleScrollCompletion(reason: .cannotScroll, completion: completion) + return } + + applyScroll(with: animation, to: resultOffset, completion: completion) } private func clampedContentOffset(_ contentOffset : CGPoint) -> CGPoint { diff --git a/ListableUI/Sources/ScrollAnimation.swift b/ListableUI/Sources/ScrollAnimation.swift new file mode 100644 index 000000000..8b26ef4a2 --- /dev/null +++ b/ListableUI/Sources/ScrollAnimation.swift @@ -0,0 +1,85 @@ +// +// ScrollAnimation.swift +// ListableUI +// + +import UIKit + + +/// Specifies how a list animates when it is asked to scroll programmatically. +/// +/// `UIScrollView` provides no way to control the duration of its built-in content +/// offset animation, so `.system` always runs at a fixed speed. Use `.duration(_:)` +/// when the scroll needs to take a specific amount of time — for example when the +/// scroll paces a sequence of steps the user is being walked through, and the +/// system speed is too fast or too slow to follow. +/// +/// Animations are suppressed entirely while `UIView.areAnimationsEnabled` is +/// `false`, matching how the list treats an `animated` flag. +/// +public struct ScrollAnimation : Equatable { + + enum Storage : Equatable { + case none + case system + case duration(TimeInterval) + } + + let storage: Storage + + /// The content offset changes immediately, without animating. + public static let none = ScrollAnimation(storage: .none) + + /// The content offset changes using `UIScrollView`'s built-in animation. + /// + /// The speed of this animation is determined by UIKit and cannot be configured. + public static let system = ScrollAnimation(storage: .system) + + /// The content offset changes over the provided duration, eased in and out. + /// + /// - Parameter duration: How long the scroll should take. A duration of zero or less + /// is treated as `.none`. + public static func duration(_ duration: TimeInterval) -> ScrollAnimation { + guard duration > 0 else { return .none } + + return ScrollAnimation(storage: .duration(duration)) + } + + /// Ands the animation with the provided bool, returning the animation if true, and `.none` if false. + public func and(with animated : Bool) -> ScrollAnimation { + if animated { + return self + } else { + return .none + } + } + + /// The animation to actually perform, accounting for whether animations are + /// currently enabled. A caller inside a `UIView.performWithoutAnimation` block + /// expects no animation, regardless of what this value describes. + /// + /// This has to be evaluated where the scroll is *requested*, not where the content + /// offset ends up changing. A scroll toward content that has not been laid out yet is + /// deferred until the presentation state catches up, and that deferred work runs on a + /// later runloop pass — outside the caller's `performWithoutAnimation` block, where + /// animations read as enabled again. Resolving late would silently ignore the + /// caller's request not to animate. + func resolvedForCurrentContext() -> ScrollAnimation { + and(with: UIView.areAnimationsEnabled) + } + + /// Whether the scroll view should run its own animation. A `.duration` animation + /// drives the content offset itself, so it asks for an unanimated change. + var usesSystemAnimation: Bool { + if case .system = storage { return true } else { return false } + } +} + + +extension ScrollAnimation { + + /// Creates an animation equivalent to the list's `animated` flag. + init(animated: Bool) { + self = .system.and(with: animated) + } +} diff --git a/ListableUI/Sources/ViewAnimation.swift b/ListableUI/Sources/ViewAnimation.swift index d056f1cf5..d1cf530a2 100644 --- a/ListableUI/Sources/ViewAnimation.swift +++ b/ListableUI/Sources/ViewAnimation.swift @@ -10,7 +10,9 @@ import UIKit /// Specifies the kind of animation to use when updating various parts of a list, -/// such as updating an item or scrolling to a given position. +/// such as updating an item or applying new content. +/// +/// See `ScrollAnimation` for animating a scroll to a given position. public enum ViewAnimation { /// No animation is performed. diff --git a/ListableUI/Tests/ListView/ListViewTests.swift b/ListableUI/Tests/ListView/ListViewTests.swift index 8f1cb9617..e8f01e255 100644 --- a/ListableUI/Tests/ListView/ListViewTests.swift +++ b/ListableUI/Tests/ListView/ListViewTests.swift @@ -1991,6 +1991,268 @@ class ListViewTests: XCTestCase } } + // MARK: Scroll animations + + /// Scrolls to a section far enough down the list that it has not been laid out yet, + /// and waits for the scroll's completion handler. + /// + /// - Returns: How many times the completion handler was reported. + @discardableResult + private func scrollToOffscreenSection( + of viewController: ViewController, + animation: ScrollAnimation, + timeout: TimeInterval = 1.0 + ) -> Int { + var completionCount = 0 + let scrolled = expectation(description: "Scroll completed") + + viewController.list.scrollToSection( + with: Section.identifier(with: "Section 4"), + sectionPosition: .top, + scrollPosition: ScrollPosition(position: .top, ifAlreadyVisible: .scrollToPosition), + animation: animation, + completion: { _ in + completionCount += 1 + scrolled.fulfill() + } + ) + + wait(for: [scrolled], timeout: timeout) + + return completionCount + } + + /// A `.duration` animation must report its completion handler exactly once. + /// + /// It drives the content offset itself with an unanimated change, which means the + /// scroll view never calls `scrollViewDidEndScrollingAnimation(_:)`. A handler + /// queued for that callback would be stranded, and one reported from both places + /// would fire twice. + func test_scroll_to_section_with_duration_animation_reports_completion_once() throws { + try testControllerCase("duration animation", sectionCount: 5, sectionHeader: true) { viewController in + let completionCount = scrollToOffscreenSection(of: viewController, animation: .duration(0.2)) + + // Give any stray second callback a chance to arrive before asserting. + waitForOneRunloop() + waitForOneRunloop() + + XCTAssertEqual(completionCount, 1) + } + } + + /// A `.duration` animation must land on the same content offset that the system + /// animation would have produced. The duration changes how the list gets there, + /// not where it ends up. + func test_scroll_to_section_with_duration_animation_matches_system_offset() throws { + func settledOffset(scrollingWith animation: ScrollAnimation) throws -> CGPoint { + var offset = CGPoint.zero + + try testControllerCase("offset parity", sectionCount: 5, sectionHeader: true) { viewController in + scrollToOffscreenSection(of: viewController, animation: animation) + + offset = viewController.list.collectionView.contentOffset + } + + return offset + } + + let system = try settledOffset(scrollingWith: .system) + let duration = try settledOffset(scrollingWith: .duration(0.2)) + + XCTAssertEqual(system.y, duration.y, accuracy: 0.5) + XCTAssertEqual(system.x, duration.x, accuracy: 0.5) + } + + /// This is the behavior the API exists for. + /// + /// Scrolling to a section that has not been laid out yet routes through + /// `preparePresentationStateForScrollToSection`, which defers the real content offset + /// change into an `updatePresentationState` completion. The animation has to wrap + /// *that* deferred change, otherwise the list hard-jumps to the target and the + /// requested duration is silently ignored. + func test_scroll_to_not_yet_laid_out_section_with_duration_animation_animates() throws { + try testControllerCase("animates offscreen section", sectionCount: 5, sectionHeader: true) { viewController in + let collectionView = viewController.list.collectionView + let startOffset = collectionView.contentOffset.y + + let duration: TimeInterval = 0.5 + + var midScroll: CGFloat = 0 + + // The offset change is deferred until the presentation state catches up, so + // let that land before sampling the scroll it should have animated. + DispatchQueue.main.asyncAfter(deadline: .now() + duration / 2) { + midScroll = collectionView.contentOffset.y + } + + scrollToOffscreenSection(of: viewController, animation: .duration(duration)) + + let target = collectionView.contentOffset.y + + XCTAssertGreaterThan(target, startOffset, "The list should have scrolled down.") + XCTAssertGreaterThan(midScroll, startOffset, "The scroll should have been underway.") + XCTAssertLessThan( + midScroll, + target, + "The scroll should have animated toward the target, not jumped to it." + ) + } + } + + /// A driven scroll has to advance the list's real content offset, so that the list + /// keeps laying out content as it passes by. + /// + /// Animating the content offset inside a `UIView` animation block looks equivalent but + /// is not: the offset lands on the target immediately, the list lays out its content + /// there, and only the layer's bounds animate back over a region that no longer has + /// anything in it. The scroll is smooth and completely blank. + func test_scroll_with_duration_animation_keeps_content_laid_out() throws { + try testControllerCase("lays out while scrolling", sectionCount: 5, sectionHeader: true) { viewController in + let duration: TimeInterval = 0.5 + + var itemsWhileAnimating: Set = [] + + DispatchQueue.main.asyncAfter(deadline: .now() + duration / 2) { + itemsWhileAnimating = viewController.list.scrollPositionInfo.visibleItems + } + + scrollToOffscreenSection(of: viewController, animation: .duration(duration)) + + XCTAssertFalse( + itemsWhileAnimating.isEmpty, + "The list should have had content on screen while scrolling." + ) + + // Content that is only laid out at the destination would report the same items + // throughout, because it never lays out anywhere else. + XCTAssertNotEqual( + itemsWhileAnimating, + viewController.list.scrollPositionInfo.visibleItems, + "The list should have laid out the content it passed over, not just the target's." + ) + } + } + + /// A driven scroll has to be interruptible, the way the scroll view's own animation is + /// when the user takes hold of the list. + func test_duration_scroll_animation_stops_where_it_is_when_cancelled() throws { + try testControllerCase("cancels mid-scroll", sectionCount: 5, sectionHeader: true) { viewController in + let collectionView = viewController.list.collectionView + + // The scroll is deferred until the presentation state catches up, so let it get + // underway before interrupting it. + DispatchQueue.main.asyncAfter(deadline: .now() + 0.3) { + viewController.list.cancelScrollAnimation() + } + + // A cancelled scroll still reports its completion. Leaving the handler queued + // would strand it, since no scroll-end callback is coming — and a two second + // animation could not have finished on its own by the time this returns. + scrollToOffscreenSection(of: viewController, animation: .duration(2), timeout: 1.0) + + let offsetAtCancellation = collectionView.contentOffset.y + + XCTAssertGreaterThan( + offsetAtCancellation, + 0, + "The scroll should have been underway when it was cancelled." + ) + + waitFor(duration: 0.3) + + XCTAssertEqual( + collectionView.contentOffset.y, + offsetAtCancellation, + accuracy: 0.5, + "A cancelled scroll should stay where it was stopped." + ) + } + } + + /// A non-positive duration cannot be animated, so it must behave exactly like `.none` + /// rather than producing a zero-length animation with different completion timing. + func test_scroll_animation_duration_of_zero_or_less_is_not_animated() { + XCTAssertEqual(ScrollAnimation.duration(0), .none) + XCTAssertEqual(ScrollAnimation.duration(-0.25), .none) + } + + /// A caller inside `UIView.performWithoutAnimation` expects no animation, and must + /// still have its completion handler reported. + /// + /// The section here has not been laid out yet, so the offset change is deferred onto a + /// later runloop pass — outside the `performWithoutAnimation` block, where animations + /// read as enabled again. The requested animation therefore has to be resolved against + /// the caller's context when the scroll is requested, not when the offset changes. + func test_scroll_with_duration_animation_is_not_animated_when_animations_disabled() throws { + try testControllerCase("animations disabled", sectionCount: 5, sectionHeader: true) { viewController in + let collectionView = viewController.list.collectionView + + UIView.performWithoutAnimation { + // An honored two second animation could not have completed within the + // timeout, so reaching it at all means the animation was suppressed. + scrollToOffscreenSection(of: viewController, animation: .duration(2), timeout: 0.4) + } + + let offsetAtCompletion = collectionView.contentOffset.y + + XCTAssertGreaterThan( + offsetAtCompletion, + 0, + "The list should still have scrolled, just without animating." + ) + + // A scroll that was still animating would keep moving past its completion. + waitFor(duration: 0.2) + + XCTAssertEqual( + collectionView.contentOffset.y, + offsetAtCompletion, + accuracy: 0.5, + "The deferred content offset change must honor the caller's suppressed animations." + ) + } + } + + /// The `animated` flag must keep mapping onto the animations it always described, so + /// that existing callers are unaffected by the introduction of `ScrollAnimation`. + func test_scroll_animation_from_animated_flag() { + XCTAssertEqual(ScrollAnimation(animated: true), .system) + XCTAssertEqual(ScrollAnimation(animated: false), .none) + } + + /// An in-flight driven animation must not keep itself alive. + /// + /// `CADisplayLink` retains its target and the main runloop retains the link, so a driver + /// that targeted itself could never be deallocated while animating — it would go on + /// writing to a scroll view belonging to a torn down list. + func test_scroll_animation_driver_does_not_retain_itself_while_animating() { + let scrollView = UIScrollView(frame: CGRect(x: 0, y: 0, width: 100, height: 100)) + scrollView.contentSize = CGSize(width: 100, height: 10_000) + + weak var weakDriver: ScrollAnimationDriver? + + autoreleasepool { + let driver = ScrollAnimationDriver( + scrollView: scrollView, + from: .zero, + to: CGPoint(x: 0, y: 500), + duration: 5, + completion: {} + ) + + weakDriver = driver + + driver.start() + + // Let the animation actually get underway before dropping the reference. + waitFor(duration: 0.1) + + XCTAssertNotNil(weakDriver) + } + + XCTAssertNil(weakDriver, "An animating driver should not be kept alive by its display link.") + } + func test_changing_identifier_resets_content_offset() { let listView = ListView(frame: CGRect(x: 0, y: 0, width: 200, height: 400)) From e013df44b4da8f58161ee5f270e45808a7cca977 Mon Sep 17 00:00:00 2001 From: Max Meyers Date: Fri, 31 Jul 2026 12:14:42 -0700 Subject: [PATCH 2/2] Report completion for a system scroll that does not move MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- ListableUI/Sources/ListView/ListView.swift | 33 ++++++++++++++++- ListableUI/Tests/ListView/ListViewTests.swift | 36 +++++++++++++++++++ 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/ListableUI/Sources/ListView/ListView.swift b/ListableUI/Sources/ListView/ListView.swift index 8b34e16f1..27601e62c 100644 --- a/ListableUI/Sources/ListView/ListView.swift +++ b/ListableUI/Sources/ListView/ListView.swift @@ -1108,6 +1108,15 @@ public final class ListView : UIView switch animation.storage { case .none, .system: + // A scroll with nowhere to go starts no animation, so the scroll view never + // calls `scrollViewDidEndScrollingAnimation(_:)` and a handler queued for that + // callback would wait forever. Report it the way an unanimated scroll is + // reported instead. + guard collectionView.contentOffset != targetOffset else { + handleScrollCompletion(reason: .scrolled(animated: false), completion: completion) + return + } + collectionView.setContentOffset(targetOffset, animated: animation.usesSystemAnimation) handleScrollCompletion( reason: .scrolled(animated: animation.usesSystemAnimation), @@ -1127,7 +1136,9 @@ public final class ListView : UIView /// `scrollRectToVisible(_:animated:)` compute where they land internally, so a driven /// animation has to perform the change unanimated and read the resulting offset back. /// - /// `changeOffset` receives whether it should ask the scroll view to animate. + /// `changeOffset` receives whether it should ask the scroll view to animate, and may be + /// invoked more than once — an unanimated call is how this discovers where the scroll + /// would land. /// /// Callers that have already determined the list cannot scroll should report /// `.cannotScroll` themselves instead of calling this. @@ -1141,6 +1152,26 @@ public final class ListView : UIView switch animation.storage { case .none, .system: + // As in `applyScroll(with:to:completion:)`, an animated scroll that does not + // move reports nothing, stranding a queued handler. Only this variant cannot + // see that in advance, so it has to perform the change unanimated to find out. + // That costs a layout pass, so it is only worth doing when there is in fact a + // handler to strand. + if animation.usesSystemAnimation, completion != nil { + let startOffset = collectionView.contentOffset + + changeOffset(false) + + let movesContent = collectionView.contentOffset != startOffset + + collectionView.setContentOffset(startOffset, animated: false) + + guard movesContent else { + handleScrollCompletion(reason: .scrolled(animated: false), completion: completion) + return + } + } + changeOffset(animation.usesSystemAnimation) handleScrollCompletion( reason: .scrolled(animated: animation.usesSystemAnimation), diff --git a/ListableUI/Tests/ListView/ListViewTests.swift b/ListableUI/Tests/ListView/ListViewTests.swift index e8f01e255..702041295 100644 --- a/ListableUI/Tests/ListView/ListViewTests.swift +++ b/ListableUI/Tests/ListView/ListViewTests.swift @@ -2213,6 +2213,42 @@ class ListViewTests: XCTestCase } } + /// A `.system` scroll that does not move must still report its completion. + /// + /// `setContentOffset(_:animated: true)` with the offset the scroll view is already at + /// starts no animation and never calls `scrollViewDidEndScrollingAnimation(_:)`, so a + /// handler queued for that callback waits forever. Both `applyScroll` overloads are + /// covered: `scrollToTop` computes its destination through the collection view, while + /// `scrollToLastItem` passes an explicit target offset. + func test_scroll_with_system_animation_that_does_not_move_reports_completion() throws { + try testControllerCase("no-op system scroll") { viewController in + let alreadyAtTop = expectation(description: "scrollToTop completed") + + viewController.list.scrollToTop(animation: .system) { _ in + alreadyAtTop.fulfill() + } + + wait(for: [alreadyAtTop], timeout: 1.0) + + // Land at the bottom without animating, so the next scroll has nowhere to go. + let atBottom = expectation(description: "scrollToLastItem completed") + + viewController.list.scrollToLastItem(animation: .none) { _ in + atBottom.fulfill() + } + + wait(for: [atBottom], timeout: 1.0) + + let alreadyAtBottom = expectation(description: "second scrollToLastItem completed") + + viewController.list.scrollToLastItem(animation: .system) { _ in + alreadyAtBottom.fulfill() + } + + wait(for: [alreadyAtBottom], timeout: 1.0) + } + } + /// The `animated` flag must keep mapping onto the animations it always described, so /// that existing callers are unaffected by the introduction of `ScrollAnimation`. func test_scroll_animation_from_animated_flag() {