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

[TIMOB-23527] iOS10: Support the UserNotifications framework #8078

Merged
merged 56 commits into from
May 23, 2018

Conversation

hansemannn
Copy link
Collaborator

@hansemannn hansemannn commented Jun 21, 2016

@hansemannn hansemannn modified the milestones: 6.1.0, 6.0.0 Jul 28, 2016
@vijaysingh-axway
Copy link
Contributor

@hansemannn, It's good if we can create module for same. Because -

  1. As we are trying to minimise Core, so creating module will fulfil this requirement.
  2. Maintenance will be easy.
  3. Any unseen issue which may arise due to these changes, will not happen in sdk.
  4. And your PR of ticket TIMOB-24266 is already merged, so it will be easy to use sdk delegates in module.

@hansemannn
Copy link
Collaborator Author

I would agree, but then we:

  • miss a core feature in the core sdk
  • will have issues maining parity across iOS versions (which is highly critical)
  • May experience duplicate events and handlers, since complex use cases like silent pushes also require other core features like background-services that only can be completed once.

What I would suggest is that we create a Singleton NotificationManager class in the sdk that is clearly separated into both frameworks, but shares a common api endpoint.

Let me still think about the module approach, but sooner or later, it will need to come to the SDK anyway. If it will be a customer wanting to have parity or the move to a higher minimum SDK version.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@sgtcoolguy sgtcoolguy modified the milestones: 7.2.0, 7.3.0 May 16, 2018
Copy link
Contributor

@vijaysingh-axway vijaysingh-axway left a comment

Choose a reason for hiding this comment

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

TiAppiOSNotificationCategoryProxy.m, TiAppiOSNotificationActionProxy.m, TiAppiOSNotificationActionProxy.h files are deleted. Are these files getting used internally only? What if developer is using?

extends: Titanium.Module
platforms: [iphone, ipad]
osver: {ios: {min: "8.0"}}
since: "7.2.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

As we are targeting this PR for 7.3.0, can you please updated this and similar stuffs in doc to 7.3.0.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated!


- (void)cancel:(id)args;
- (void)cancel:(id)used;
Copy link
Contributor

Choose a reason for hiding this comment

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

It should be 'unused' .

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated!

;
if ([TiUtils isIOS10OrGreater]) {
DebugLog(@"[ERROR] Please use Ti.App.iOS.NotificationCenter.requestUserNotificationSettings in iOS 10 and later to request user notification settings asynchronously.");
return;
Copy link
Contributor

Choose a reason for hiding this comment

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

If this function is previously getting used, now it will not return relevant data in iOS >= 10.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated!

Copy link
Contributor

@vijaysingh-axway vijaysingh-axway left a comment

Choose a reason for hiding this comment

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

CR passed.

@vijaysingh-axway
Copy link
Contributor

@hansemannn Can you resolve conflict please. Thanks!

… TIMOB-23527

# Conflicts:
#	iphone/Classes/UtilsModule.m
@hansemannn hansemannn merged commit 6094584 into tidev:master May 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants