Skip to content

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jjochen
Copy link

@jjochen jjochen commented Apr 13, 2015

when the origin.y of the child view controller is not 0 the notification view is to big

@@ -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];
Copy link
Owner

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.

Copy link
Author

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.

Copy link
Owner

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?

Copy link
Author

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.

Copy link
Owner

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 in navigationControllerWillShowViewControllerNotification. 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 to navigationControllerDidShowViewControllerNotification (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 :)

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