-
Notifications
You must be signed in to change notification settings - Fork 145
fixed an issue with custom container view controllers #57
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
base: master
Are you sure you want to change the base?
fixed an issue with custom container view controllers #57
Conversation
…troller origin.y is not 0)
@@ -383,6 +383,10 @@ - (void)dismissWithStyle:(CSNotificationViewStyle)style message:(NSString *)mess | |||
- (CGFloat)topLayoutGuideLengthCalculation | |||
{ | |||
CGFloat top = MIN([UIApplication sharedApplication].statusBarFrame.size.height, [UIApplication sharedApplication].statusBarFrame.size.width); | |||
|
|||
UIView *view = self.parentNavigationController.view ?: self.parentViewController.view; | |||
CGPoint origin = [view.superview convertPoint:view.frame.origin toView:[UIApplication sharedApplication].keyWindow.rootViewController.view]; |
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.
Why are you deriving the origin from the application's root view controller? This may fit your special usecase but it is not a general-purpose solution.
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.
convertPoint:
gets the origin of the parent(Navigation)ViewController
inside of the keyWindow
.
Using the rootViewController
instead of the keyWindow
directly, solves some problems with rotations.
The notificationView.origin.y
is the top of the parentViewController
not the top of the keyWindow
. So you should subtract the height of the status bar only if the origin of the parentViewController
is actually the top of the keyWindow
. Otherwise you have to calculate the height of the notificationView
somehow differently. Perhaps there is a better way to achieve that.
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.
OK, now I understand what you are trying to achieve.
However: You are mixing visual impression ("being at the top of the screen") with the logical representation in Apple's frameworks. This is a little too hacky for me and it looks like it can easily break.
topLayoutGuideLengthCalculation
tries to calculate the vertical indentation from 0 to y in the parentViewController
or parentNavigationController
because topLayoutGuide
is not always reliable / useful (e.g. when presenting in a UINavigationController
). Hence, the name of the method.
In you case (modal popover on iPad), using the parent's layout guide is enough.
return [self.parentViewController.topLayoutGuide length];
However, this does not work as a general solution (try yourself ;) ).
ATM, I don't have a satisfying answer to this. In principle, we need a way to express the condition of being in a modal popover via API calls (on iOS 7 and iOS 8).
Do you have any idea?
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.
Actually in my case it's not a modal popover on iPad but a custom content view controller.
My logic was that the status bar height should only be added to the ´topLayoutGuide´ when the view controller frame actually has an intersection with the status bar frame. I haven't got an idea how to achieve that without using ´convertRect:fromView:´ or something similar.
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.
when the view controller frame actually has an intersection with the status bar frame
This may be a correlation with your usecase but isn't what we want to express.
I am unhappy with the situation, too, but I don't like the approach of checking the view's location on screen.
I just spent an hour looking for a solution. Three ways:
- Using the layout guide may work if we hooked into the
parentViewController
's viewDidLayoutSubviews method. However, this is very invasive behavior. But this way would allow binding the topLayoutGuide's length to the CSNotificationView's height. It remains to be seen whether this is backwards compatible to iOS 7. - Use the
topLayoutGuide
and deal with the following problem: when transitioning between view controllers, CSNotificationView recalculates its height since a presented view controller may have a navigation bar hidden / present. We do this innavigationControllerWillShowViewControllerNotification
. If we use topLayoutGuide here, it is not set yet because AutoLayout has not done its work at this point. One could postpone the resize tonavigationControllerDidShowViewControllerNotification
(resulting in a delayed resize animation). Since users already rely on a smooth animation, I don't want to break this. - Making the height (or
addStatusBarHeight
) a property of CSNotificationView would allow app developers to implement those bindings externally. This may be the easiest solution.
I currently do not have the time to do this myself. If you provide a clean implementation of the addStatusBarHeight
property that does not break the current behavior, I am happy to merge :)
when the origin.y of the child view controller is not 0 the notification view is to big