-
Notifications
You must be signed in to change notification settings - Fork 281
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
Usage improvements #198
base: master
Are you sure you want to change the base?
Usage improvements #198
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@igorkulman Thanks for the contribution. I find these changes useful. Though I have concerns regarding calling a delegate method inside an animation block. Please see my comment for a possible solution.
@@ -334,6 +343,7 @@ open class ENSideMenu : NSObject, UIGestureRecognizerDelegate { | |||
withDuration: animationDuration, | |||
animations: { [weak self] () -> Void in | |||
self?.sideMenuContainerView.frame = destFrame | |||
self?.delegate?.sideMenuIsAnimating() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I anticipate some unexpected side effects on the use site if the delegate method would be called inside an animation block. Instead I would suggest introducing another two delegate callbacks with an isAnimated
argument and call them before scheduling the animation:
sideMenuWillOpen:isAnimated
sideMenuWillClose:isAnimated
I see this as a more universal approach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a good point, but my specific case requries the delegate call inside the animate method so the statubar changes gradually not at once wzen animation startes or finishes.
This PR adds a few improvements I came up using the side menu in my project