Skip to content

Conversation

@johnnewman-square
Copy link
Contributor

@johnnewman-square johnnewman-square commented Jun 17, 2025

This PR adds a completion handler to the scrollTo(item:position:animated:) API, turning it into scrollTo(item:position:animated:completion:).

This also adds unit tests to assert the expected items are visible in the completion callback. A new demo allows you to use this API in a number of layout contexts, leveraging console logs when the scroll completion handler is executed:
Screenshot 2025-06-17 at 12 19 01 PM Screenshot 2025-06-17 at 12 19 08 PM

Checklist

Please do the following before merging:

  • Ensure any public-facing changes are reflected in the changelog. Include them in the Main section.

@johnnewman-square johnnewman-square marked this pull request as ready for review June 17, 2025 18:26
@johnnewman-square johnnewman-square requested a review from a team June 17, 2025 18:33
@robmaceachern robmaceachern self-requested a review June 18, 2025 15:34
Adding the completion handler to ListActions.
@johnnewman-square
Copy link
Contributor Author

I've pushed up a change to add the completion handler to the equivalent ListActions scrolling APIs so that they're accessible in both ListView and ListActions.

* main:
  Remove iOS 19 (26) cap of the collection view first responder workaround (#581)
  Bumping versions to 16.1.0 (#583)
  Update for Blueprint 6.0.0 (#580)
Adding tests for queued handlers.
Adding a scroll test for a floating point offset.
Comment on lines +834 to +843
/// This function will determine if a call to `collectionView.scrollToItem(...)`
/// will result in an adjusted content offset. This is necessary because when the
/// item is already at the expected position, `UICollectionView` will not scroll
/// and will not execute its `scrollViewDidEndScrollingAnimation(_:)` delegate.
func willScroll(
for scrollPosition: UICollectionView.ScrollPosition,
itemFrame: CGRect,
viewport: CGRect,
contentSize: CGSize
) -> Bool {
Copy link
Member

@robmaceachern robmaceachern Jun 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm just thinking through what it would mean if the scroll view behavior ever changed slightly and introduced a mismatch:

  • We return false but the scroll view does actually scroll
    • Completion will fire early
  • We return true but the scroll view doesn't actually scroll
    • Completion won't fire at all (right?) (edit: or would potentially fire on some other scroll event later on?)

Do you figure the risk of this getting out of sync is pretty low?

Copy link
Contributor Author

@johnnewman-square johnnewman-square Jun 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, you're right on both cases. For the case where true is returned but the scroll view doesn't actually scroll, we would end up running the handler after the next call to scrollTo(...) runs.

UICollectionView won't animate when the distance to scroll is under 0.5 points. To future proof this function, we can floor/ceil each of these measurements so that they must be greater than or equal to 1.0 points when calculating both the distance to scroll and whether the collection view can move in a particular direction.

This will result in the completion handler running before the collection view moves in some cases where the distance is under 1.0 points, but I think this is fine. This should reduce risks related to slight changes in UIKit's implementation.

Here's a commit with the update: c1659c8. I can take this in a different direction if there are any concerns!

Copy link
Contributor

@g-mark g-mark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

} else {
// Dispatch so that scrolling without an animation executes the closure
// on the next runloop execution, similar to scrolling with an animation.
DispatchQueue.main.async {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we ever have a non-animated handleScrollCompletion when scrollCompletionHandlers isn't empty? I'm not sure it would really matter, but curious if this came up at all.

Copy link
Contributor Author

@johnnewman-square johnnewman-square Jun 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for raising this question. This shouldn't be possible because right before handleScrollCompletion(...) is executed with .scrolled(false) as the reason, the collection view's offset will have just been adjusted without an animation. This offset adjustment cancels any ongoing scroll animation and empties the scrollCompletionHandlers.

Adding a guard statement to didEndScrolling and improving docs.
@johnnewman-square johnnewman-square force-pushed the johnnewman/feature/scroll-completion branch from 4f2398e to e35c751 Compare June 27, 2025 16:45
@johnnewman-square johnnewman-square merged commit ce0bc0c into main Jun 30, 2025
4 checks passed
@johnnewman-square johnnewman-square deleted the johnnewman/feature/scroll-completion branch June 30, 2025 17:52
@johnnewman-square johnnewman-square restored the johnnewman/feature/scroll-completion branch June 30, 2025 17:52
johnnewman-square added a commit that referenced this pull request Jun 30, 2025
* main:
  Adding a programmatic scroll completion handler (#582)
johnnewman-square added a commit that referenced this pull request Jul 2, 2025
This builds on the changes in
#582, adding a completion handler
to the `scrollToSection(...)` API. This also adds a demo to showcase
this API with various configurations:
<img width="300" alt="Screenshot 2025-06-30 at 4 33 33 PM"
src="https://github.com/user-attachments/assets/03f04302-ead2-4bba-b773-ce096106adba"
/>
kyleve added a commit that referenced this pull request Jul 24, 2025
…-headerfooters

* origin/main:
  build: set up tuist (#584)
  Bumping versions to 16.3.0 (#589)
  Reset scroll position if list identifier changes (#588)
  Bumping versions to 16.2.0 (#587)
  Adding a completion handler to the `scrollToSection` API. (#585)
  Adding a programmatic scroll completion handler (#582)
  Remove iOS 19 (26) cap of the collection view first responder workaround (#581)
  Bumping versions to 16.1.0 (#583)
  Update for Blueprint 6.0.0 (#580)
  Bumping versions to 16.0.4. (#579)
  Update the first responder resignation workaround to be enabled by default and cap at < iOS 19 [UI-8849] (#578)
  Bumping versions to 16.0.3 (#577)
  Bottom gravity and autoscroll improvements (#576)
  Bumping versions to 16.0.2 (#574)
  Addressing an AutoScrollAction issue when using VerticalLayoutGravity.bottom (#572)
  release: Prepare 16.0.1 release (#569)
  Fix reordering crash introduced in 16.0 (#568)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants