Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for Issue #174 - pageAppeared Delegate Callback #177

Merged
merged 1 commit into from
Jul 19, 2016

Conversation

JakeSc
Copy link
Contributor

@JakeSc JakeSc commented Jul 18, 2016

#174

There is a bug in which tapping a page to advance does not call the delegate callback pageAppeared. This is because the tap IBAction sets the new _currentPageIndex before the notifyDelegateWithPreviousPage gets called. This latter method has a check which fails when the currentPageIndex is updated before calling it.

This PR resolves this bug by just making the tap IBAction scroll to the page index and not update the currentPageIndex. Then in the scrollDidEnd callback, we update new currentPageIndex already.

Note that in #174 , this bug still exists when one calls the setCurrentPageIndex: method. My recommendation is to not expose the setCurrentPageIndex: setter, and instead expose the scrollToPageForIndex: method. Then, we only ever update the new currentPageIndex property once, after the scrollView delegate callback is called.

…fter tapping a page

This resolves [Issue ealeksandrov#174: Delegate method pageAppeared doesn't being called]
@JakeSc JakeSc force-pushed the Issue174_PageAppeared branch from ec4451e to 52f7cdd Compare July 18, 2016 07:13
@JakeSc
Copy link
Contributor Author

JakeSc commented Jul 18, 2016

I exposed the new scrollToPageForIndex: method in the header file. Calling this is essentially the same as calling setCurrentPageIndex:animated:, except for the fact that it doesn't actually set the _currentPageIndex. This happens after the scroll has occurred.

@JakeSc
Copy link
Contributor Author

JakeSc commented Jul 18, 2016

I did this so that I can add an NSTimer that automatically advances pages, but still have that pageAppeared delegate method called.

@ealeksandrov
Copy link
Owner

Thanks a lot for your contributions, looks awesome!

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.

2 participants