Skip to content
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

[iOS10] UILocalNotification is deprecated from iOS10 and up #19

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

Conversation

shkhaliq
Copy link

Upgraded our project to minimum iOS10. Fixed all the warnings

Copy link
Owner

@JaviSoto JaviSoto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for taking so long! I missed this originally.
I'm a bit confused by the duplicate #if/#elif lines: what's the logic there?

Alternatively I'm also thinking: this could use some clean up, and use NS_AVAILABLE declarations and @available instead.

@@ -107,7 +108,8 @@ NS_ASSUME_NONNULL_BEGIN
- (void)application:(UIApplication *)application didReceiveRemoteNotification:(NSDictionary *)userInfo fetchCompletionHandler:(void (^)(UIBackgroundFetchResult result))completionHandler;
#endif

#if JSIOS8SDK
#if JSIOS10SDK
#elif JSIOS8SDK
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm why these 2?

Copy link

@ghost ghost Jan 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JaviSoto So didRegisterUserNotifications and handleActionWithIdentifier were both added in iOS8 but deprecated in iOS10.
and #if JSIOS8SDK condition is true for all iOS versions 8 and above. Hence I added an if for iOS10> where those methods are not registered

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, should this be #if JSIOS8SDK && ! JSIOS10SDK perhaps to make it more clear?

I think the JSIOXSDK macros should be renamed to like JSSDKAtLeastIOS9. That way this would be a bit clearer.

Alternatively, what I would like is to stop using those macros, and instead, add:

NS_DEPRECATED_IOS(8_0, 10_0)

What do you think?

@ghost
Copy link

ghost commented Jan 19, 2018

I am not sure if you can provide a min and max iOS version for NS_AVAILABLE_IOS.

Though I have renamed the macros to something that's way more readable. Let me know how that looks.

@JaviSoto

@JaviSoto
Copy link
Owner

I am not sure if you can provide a min and max iOS version for NS_AVAILABLE_IOS.

Yeah, look at UIApplication.h for examples. The syntax is like:

NS_DEPRECATED_IOS(INTRODUCED, DEPRECATED)
// E.g.:
NS_DEPRECATED_IOS(2_0, 10_0)

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.

3 participants