jcoleman / JCNotificationBannerPresenter

A library for generic presentation of "banners" (e.g. to present a push notification) from anywhere inside an iOS app.
MIT License
198 stars 46 forks source link

Banner doesn't show up correctly on iOS7 #18

Closed yimingcheung closed 11 years ago

yimingcheung commented 11 years ago

As the following screenshot shows, banner only shows up partially.

banner_half_shown

jcoleman commented 11 years ago

Likely this is an issue with the coordinate calculation due to the new iOS 7 view insets for having content under navigation bars etc.

It's unlikely I'll be able to get to this until late next week unfortunately. Any other debugging information or patches would be appreciated.

jcoleman commented 11 years ago

@yimingcheung Can you test and confirm this fix before I release a new CocoaPods version?

yimingcheung commented 11 years ago

@jcoleman : Thanks for the fixing!

This did fix the bug for iOS7. However, it broke iOS6 since the compile-time check using IPHONE_7_0 and IPHONE_OS_VERSION_MAX_ALLOWED can't actually determine if the current iOS is iOS7 or not, and thus the following code is never executed for iOS6: y -= (MIN(statusBarSize.width, statusBarSize.height));

which creates a gap between the banner and iOS device screen top on iOS6, see the screenshot below.

Instead, you can check the iOS version in runtime as suggested in this post: http://stackoverflow.com/questions/17846544/how-to-define-based-on-ios-version

Thus the fix can be:

define IS_IOS7 [[UIDevice currentDevice].systemVersion hasPrefix:@"7"]

CGFloat y = -self.bannerHeight; if (!IS_IOS7) { y -= (MIN(statusBarSize.width, statusBarSize.height)); }

As far as I test, this works fine on both iOS7 and iOS6.

Here are the screenshots: iOS6 is broken: ios6_messedup

iOS7 looks good: ios7_looksgood

jcoleman commented 11 years ago

So here's the problem I've encountered (and why I used the compile time check instead of the runtime check): if you're running on iOS 7 but the build has been compiled with the Base SDK set to iOS6, then the view sizing happens as if it were on iOS6 rather than 7. Therefore a strict runtime check is not correct, I believe.

What are the base SDK settings you're using when taking those two screenshots?

jcoleman commented 11 years ago

@yimingcheung So I've updated the fix to handle both Base SDK < 7 (in which case we can assume we need to adjust for the status bar height) and for Base SDK >= 7 (in which case we need to adjust for the status bar height if we're running on iOS 6 or lower.)

Basically that's the combination of your fix (which handled apps compiled with SDK7) and my original fix (which handled apps compiled with SDK < 7.)

Let me know if this fixes it for you.

yimingcheung commented 11 years ago

@jcoleman : Thanks very much for the latest fixing!

For both the two screenshots, I'm using iOS7.0 as the base SDK setting. And I'm not able to set the base SDK to be lower than iOS7.0 since I'm using Xcode 5.0 now.

The latest fix works well on both iOS7 and iOS6 devices with base SDK set to iOS7.0. I was not able to test on iOS7 device with base SDK set to iOS6. But good to know about this!

Thanks for fixing this bug so quickly, and your library is very useful to our project!

jcoleman commented 11 years ago

Awesome. I'll release a CocoaPods update this afternoon. Incidentally, there are a few other bug fixes in this release as well for issues that you may encounter with positioning of the alert view.

BTW, if you need to use a lower base SDK in Xcode 5, you can still do it by manually "installing" the SDK into the Xcode app bundle. See http://stackoverflow.com/questions/18423896/is-it-possible-to-install-ios-6-sdk-on-xcode-5 for directions if you ever need it.

yimingcheung commented 11 years ago

Nice! Thanks for the instruction, and good to know you're ready to release it!

jcoleman commented 11 years ago

The new release should be in CocoaPods (version 1.0.3).