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

Fade skip button for last page if fade last page is enabled #82

Closed
wants to merge 3 commits into from

Conversation

willthink
Copy link

No description provided.

willthink added 3 commits August 28, 2015 06:57
@mamaral
Copy link
Owner

mamaral commented Sep 1, 2015

I'm not a fan of this for two reasons - the first is this PR includes the changes from your other PR that was closed because the functionality already exists to customize the skip button and page control without adding additional code. The second is that fading out the skip button on the last page is a fundamentally different process than fading out the page control - and thus shouldn't be tied logically to it. Some people might want to fade the page control but not the skip button. Also, AFAIK, setting the alpha of the skip button wouldn't disable it - so it would still be selectable and thus would lead to some weirdness if someone tapped the spot where it used to be and the skip handler was called.

@willthink
Copy link
Author

I agree. Fade the button is not good idea. Do you think which way is good to disable skip button at last? How about removeFromSuperview?
BTW,
Thanks for your comment, I really learn from it

@mamaral
Copy link
Owner

mamaral commented Sep 1, 2015

You could always set the enabled property to NO, although I don't know exactly what the use-case is you have in mind for this.

@willthink
Copy link
Author

To me, I don't like the skip button shows up with the "Getting started" at the last page. So I want skip button to disappear when the page control fades.

@mamaral
Copy link
Owner

mamaral commented Sep 1, 2015

Do you want it to reappear if you scroll back from the last page to the previous pages, or be gone forever once you reach the last screen?

@willthink
Copy link
Author

Yes, it should reappear if you scroll back

@mamaral
Copy link
Owner

mamaral commented Sep 1, 2015

I'll try and whip something up tonight for you.

@higueraalfredo
Copy link

How about something like this in - scrollViewDidScroll:

_pageControl.alpha = 1.0 - percentComplete;
if (percentComplete >= 1.0) {
    _skipButton.enabled = NO;
    [UIView transitionWithView:_skipButton
                      duration:0.1
                       options:UIViewAnimationOptionTransitionCrossDissolve
                    animations:^{
                        _skipButton.hidden = YES;
                    }
                    completion:NULL];
}

@mamaral
Copy link
Owner

mamaral commented Sep 2, 2015

We definitely don't want to do a UIView animation inside that scrollViewDidScroll: as that is called a huge amount of times. I have a pretty simple solution, I just need to get the time to implement it.

@higueraalfredo
Copy link

Wouldn't checking if (percentComplete >= 1.0) make sure that the animation only fires once a page is fully loaded?
I agree with you that hiding the skip button should not be tied to fadePageControlOnLastPage but I'll use this solution in the meantime until you get a chance to implement this feature. I want to hide both on the last page so this is good enough for now.
Thanks.

@mamaral
Copy link
Owner

mamaral commented Sep 2, 2015

@higueraalfredo: No, because lets say the user was dragging the page back and forth from 98% -> 100% of the scroll percentage, it would be firing constantly and would probably break.

I should have this feature added tonight.

@mamaral
Copy link
Owner

mamaral commented Sep 2, 2015

I believe what you're looking for was been added in version 2.1.7 after merging #84. You should be able to run a pod update and pull those changes in, and simply set fadeSkipButtonOnLastPage to YES on your OnboardingViewController.

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.

3 participants