search

mrackwitz / MRProgress

Collection of iOS drop-in components to visualize progress
MIT License
2.55k stars 306 forks source link

fix rotation with system version check #69

Closed ulechka closed 9 years ago

ulechka commented 9 years ago

Added system version check to fix rotation issue on ios 8.

ulechka commented 9 years ago

https://github.com/mrackwitz/MRProgress/issues/65

marcelofabri commented 9 years ago

This is not the recommended way to do it. Either NSFoundationVersionNumber or NSProcessInfo (iOS 8+) are preferred.

This SO answer explain more: http://stackoverflow.com/a/18987193/1777634

mrackwitz commented 9 years ago

@ulechka: Thanks for your updated PR. :+1:

@marcelofabri: Thanks for pointing out. Generally you're right, even if it feels wrong to me to compare with a constant, which gives info about an other framework then UIKit. Because the behavior here is specific to pre iOS 8, we can't rely on NSProcessInfo, so we'd need to use NSFoundationVersionNumber. But there is another issue here: there is no such definition for iOS 8. NSFoundationVersionNumber_iOS_7_1 is the last defined macro, though you could argue we can compare NSFoundationVersionNumber > NSFoundationVersionNumber_iOS_7_1. Theoretically Apple could release a patch with an higher foundation version number for iOS in the major version 7, which is still the latest officaly available version for iPhone 4.

So from my side this PR is semantically good to go. What do you think?

marcelofabri commented 9 years ago

We could use [[NSProcessInfo processInfo] respondsToSelector:@selector(isOperatingSystemAtLeastVersion:)] to check availability.

Also, Apple even tough there's no iOS 8 constant, that's what Apple recommends (https://developer.apple.com/library/ios/documentation/UserExperience/Conceptual/TransitionGuide/SupportingEarlieriOS.html#//apple_ref/doc/uid/TP40013174-CH14-SW3).

mrackwitz commented 9 years ago

@marcelofabri: Regarding your first paragraph: This would be possible and slightly more efficient. :+1:

That's what Apple recommends under the section Supporting iOS 6. They don't recommend it in a general manner. Thankfully they provide now an appropriate API to check for the OS version.

mrackwitz commented 9 years ago

Thanks @ulechka. I rebased your branch and merged it to master.