BUG: 'setCurrentPageIndex:animated:' no longer triggers 'onPageDidAppear'

[funnel20]

We're updating EAIntroView from version 2.7.4 to latest 2.11.0.
We found a serious bug in setCurrentPageIndex:animated:.

Use case
On page 1, we have placed a button NEXT.
The button calls setCurrentPageIndex:animated: with animation enabled to scroll to page 2.
On Page 2, we have implemented onPageDidAppear to start some animations.
However, in version 2.11.0 the animations are not started.

Analysis
While debugging we found that onPageDidAppear is never fired.
It should be called in notifyDelegateWithPreviousPage:andCurrentPage:, which is called in checkIndexForScrollView: when the scrollView ends scrolling.

The error can be visualised in checkIndexForScrollView::

[self notifyDelegateWithPreviousPage:self.currentPageIndex andCurrentPage:newPageIndex];

In the debugger it appears that for this use case self.currentPageIndex already equals newPageIndex.
Since both have the same value, the IF condition in notifyDelegateWithPreviousPage:andCurrentPage: fails, and any required (delegate) methods, such as onPageDidAppear, are not called.

Why is self.currentPageIndex already set to the new index?
Because that's already done in setCurrentPageIndex:animated: before scrolling is even started:

- (void)setCurrentPageIndex:(NSUInteger)currentPageIndex animated:(BOOL)animated {
    if(![self pageForIndex:currentPageIndex]) {
        NSLog(@"Wrong currentPageIndex received: %ld",(long)currentPageIndex);
        return;
    }
    
    _currentPageIndex = currentPageIndex;
    
    [self scrollToPageForIndex:currentPageIndex animated:animated];
}

Solution
Remove the following line from setCurrentPageIndex:animated::
_currentPageIndex = currentPageIndex;

Verification
Animated = YES
We tested the modified code while calling setCurrentPageIndex:animated: with animation enabled. Now onPageDidAppear is called properly.

Animated = NO
We repeated the test. The first time onPageDidAppear was not called.
A second run it was called.
Huh?

More debugging showed that when animation is disabled, sometimes scrollViewDidEndScrollingAnimation: is called, but not always.
Hence we enforced to always call checkIndexForScrollView: in order to call the delegate methods in scrollToPageForIndex:animated::

- (void)scrollToPageForIndex:(NSUInteger)newPageIndex animated:(BOOL)animated
{
    if(![self pageForIndex:newPageIndex]) {
        NSLog(@"Wrong newPageIndex received: %ld",(long)newPageIndex);
        return;
    }

    CGFloat offset = newPageIndex * self.scrollView.frame.size.width;
    CGRect pageRect = { .origin.x = offset, .origin.y = 0.0, .size.width = self.scrollView.frame.size.width, .size.height = self.scrollView.frame.size.height };
    [self.scrollView scrollRectToVisible:pageRect animated:animated];
    
    if(!animated) {
        [self scrollViewDidScroll:self.scrollView];
        
        // When animated, the scrollView delegates will call 'checkIndexForScrollView' that will first call the delegate methods and after it update _currentPageIndex.
        // When animation is disabled, sometimes 'scrollViewDidEndScrollingAnimation:' is called, but not always. Hence always call 'checkIndexForScrollView:' in order to call the delegate methods:
        [self checkIndexForScrollView:self.scrollView];
    }
}

Feedback
I agree that the latter for animation disabled is more a work-around than a solution, as it's not clear why scrollViewDidEndScrollingAnimation: is sometimes called.
Please advise if you have any suggestions.

[ealeksandrov]

Thanks for detailed investigation, I'll play with it later on this week.

[ealeksandrov]

Hmmm.
It was a long dance around one line.

4f68750
f1fdfd4
a5ce2c3

#67
#144
#174
#177

Remove the following line from setCurrentPageIndex:animated::
_currentPageIndex = currentPageIndex;

This is bad for a reason mentioned in #144:

This would be a rather unusual behavior for an UIKit component. Normally the initial state is effected immediately not after the animation has finished (UITabBarController.selectedIndex, UINavigationController.topViewController, ...).

The main issue is initial exposing of currentPageIndex setter while there are some actions triggered by pages scrolling. So currentPageIndex should be only observed without any intervention from outside.
To change pages programmatically - scrollToPageForIndex:animated: should be used.

[ealeksandrov]

Decided to go ahead with this suggestions: #174 (comment) & #177

Fixing scrollToPageForIndex:animated: for animated:NO - fa98a37
Removing setCurrentPageIndex:animated: - 08dc45c

Will push 2.12.0 release shortly.