uias / Pageboy

📖 A simple, highly informative page view controller
MIT License
1.99k stars 155 forks source link

Missing didFinishAnimating in delegate didScrollToPage. #181

Closed ChristianSteffens closed 5 years ago

ChristianSteffens commented 6 years ago

Hi,

I'm trying to replace our existing UIPageViewController with your PageboyViewController. Unfortunately there seems to be one case, that we can't replace with a counterpart in Pageboy.

Usecase: User swipes to a new page, but in the middle of the transition the user cancels this action by lifting his fingers.

In UIPageViewController there's this delegate call: pageViewController(_:didFinishAnimating:previousViewControllers:transitionCompleted:)

TransitionCompleted tells us, if the action actually did complete.

Is there a (direct) counterpart for this situation in Pageboy? I'll found a potential workaround by using the didScrollToPostion >= 0.5 delegate, but that doesn't seem very clean or is it?

msaps commented 6 years ago

@LordNali interesting - I think we might be able to achieve this with the PageboyViewControllerDelegate. Basically didScrollToPageAt directly maps to the didFinishAnimating UIPageViewController delegate method, with the transitionCompleted flag set to true (and also a bit more reliable).

I think you could use the willScrollToPageAt delegate function to get an expectedPageIndex or something and then use pageboyViewController(didScrollTo position: direction: animated:), verify that you are on a page index (i.e. 1.0 / 2.0 / 3.0 etc.) and compare it to the expectedPageIndex`?

This will give you the comparison when the user arrives back at the origin page or is that too late?

ChristianSteffens commented 6 years ago

Don't get me wrong, I'm perfectly fine implementing the behaviour as you proposed. But I would recommend to imitate the behaviour from the original UIPageViewController, i.e. adding a new property transitionCompleted to the delegate call.

My guess is - at least it was the case for us - most of the Pageboy users are coming in fact from an UIPageViewController environment. So to make the migration as smooth as possible Pageboy should provide at least the same information in its delegate - if not more (as with the precise scrolling position).

I'm willing to write a PR if you're interested.

msaps commented 6 years ago

@LordNali I guess this would need quite a conceptual change to how the PageboyViewControllerDelegate works...

As it currently stands didScrollToPageAt... is the delegate function which maps to didFinishAnimating from UIPageViewController the closest, with the exception that didScrollToPageAt is only called when the transition was completed and the user is on the new page - it isn't called for example on returning the previous page if the scroll transition was incomplete.

These are the core functions of the current delegate:

I'm not really sure how we fit in a 'did not complete scroll' flag into there - any suggestions etc. are more than appreciated! Idea behind Pageboy is to have a much nicer and better delegate than UIPageViewController so 😄

ChristianSteffens commented 6 years ago

Ok, I get the idea that we shouldn't copy the ugly interface from the original UIPageViewController - that's for sure ;-) But I do found it very useful to have an information regarding the transition's completion. Adding it to one of the existing delegate calls is indeed tricky and not even the best approach.

I would recommend a new delegate callback, so nothing changes in the current structure and anyone who's interested will get the necessary information. I mean in the end we're talking about something like cancelledScrollToPageAt index? Implementation-wise it would probably enough to add a new internal property like expectedPageIndex which will be set right before calling `willScrollToPageAt.