-
Notifications
You must be signed in to change notification settings - Fork 24.4k
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
[PushNotifications] Register for Remote Notifications and Extend AppDelegate #1019
Conversation
Added optional callback parameter to requestPermissions Method and Extend AppDelegate so you don’t need to call methods on AppDelegate.h
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at [email protected]. Thanks! |
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
@lazaronixon - I definitely like the improvement in the API here, are there any drawbacks to this approach? ObjC isn't my strongest language - is this automatically imported into |
BTW I'm using this fork in my app, works great. |
This is pretty fragile... What if the person renames the file? or the folder? or move the library? This would also break any example app... When I wrote it I tried swizzling as well, but it doesn't work because we don't know the AppDelegate's class at load time, we thought about some other options, but none of them seemed to be good. Something that I'm not sure (cc @nicklockwood in case you don't know as well) is whether |
You're right @tadeuzagallo it is a price to pay, but too high to me, a solution i found was |
With a bit of runtime logic we could just detect if the RCTPushNotificationManager exists, and include the necessary code to call it in the AppDelegate of the standard project template. Maybe that's the best option. |
If I move AppDelegate+notification to SampleApp Folder and with |
Thank you for reporting this issue and appreciate your patience. We've notified the core team for an update on this issue. We're looking for a response within the next 30 days or the issue may be closed. |
@nicklockwood should we add this code to the SampleApp's AppDelegate, or should I close it for now? |
@tadeuzagallo @nicklockwood This PR consists of 3 discussions which are continued here: Extend the appdelegate
Device token PushNotificationIOS.addEventListener('register', function(token){
console.log('You are registered and the device token is: ',token)
}); Error handling I think that this PR can be closed. |
@lazaronixon Any updates on this? |
@lazaronixon updated the pull request. |
Closing since no activity on the PR. let's re-open if you wanna work on it again. |
Update touchablehighlight.md
Added optional callback to requestPermissions Method.
If registration is successful, APNs returns a device token.
If there is a problem in obtaining the token, a error message is returned.
Extend AppDelegate so you don’t need to call methods on AppDelegate.h