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.

Sign up for GitHub

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

PushNotificationIOS requestPermission promisified #7900

Closed
wants to merge 8 commits into from
Closed

PushNotificationIOS requestPermission promisified #7900

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
b8180c9
PushNotificationIOS requestPermission promisified
JAStanton Jun 2, 2016
1e2f592
Merge branch 'master' into master
JAStanton Jun 2, 2016
6365175
short hand to create notification types
JAStanton Jun 3, 2016
ef8c066
dont throw, instead use RCTLogError
JAStanton Jun 3, 2016
8344774
short hand bool
JAStanton Jun 3, 2016
afed56b
bitwise and then compare then cast to number
JAStanton Jun 3, 2016
dbabb78
Merge branch 'master' into master
JAStanton Jun 3, 2016
45c9e15
userInfo is a dictionary, dont abuse the system
JAStanton Jun 3, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions Libraries/PushNotificationIOS/PushNotificationIOS.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,12 +216,20 @@ class PushNotificationIOS {
*
* If a map is provided to the method, only the permissions with truthy values
* will be requested.

* This method returns a promise that will resolve when the user accepts,
* rejects, or if the permissions were previously rejected. The promise
* resolves to the current state of the permission.
*/
static requestPermissions(permissions?: {
alert?: boolean,
badge?: boolean,
sound?: boolean
}) {
}): Promise<{
alert: boolean,
badge: boolean,
sound: boolean
}> {
var requestedPermissions = {};
if (permissions) {
requestedPermissions = {
Expand All @@ -236,7 +244,7 @@ class PushNotificationIOS {
sound: true
};
}
RCTPushNotificationManager.requestPermissions(requestedPermissions);
return RCTPushNotificationManager.requestPermissions(requestedPermissions);
}

/**
Expand Down
42 changes: 41 additions & 1 deletion Libraries/PushNotificationIOS/RCTPushNotificationManager.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,13 @@
NSString *const RCTLocalNotificationReceived = @"LocalNotificationReceived";
NSString *const RCTRemoteNotificationReceived = @"RemoteNotificationReceived";
NSString *const RCTRemoteNotificationsRegistered = @"RemoteNotificationsRegistered";
NSString *const RCTRegisterUserNotificationSettings = @"RegisterUserNotificationSettings";

NSString *const RCTErrorUnableToRequestPermissions = @"E_UNABLE_TO_REQUEST_PERMISSIONS";

@interface RCTPushNotificationManager ()
@property (nonatomic, copy) RCTPromiseResolveBlock requestPermissionsResolveBlock;
@end

@implementation RCTConvert (UILocalNotification)

Expand Down Expand Up @@ -66,6 +73,10 @@ - (void)startObserving
selector:@selector(handleRemoteNotificationsRegistered:)
name:RCTRemoteNotificationsRegistered
object:nil];
[[NSNotificationCenter defaultCenter] addObserver:self
selector:@selector(handleRegisterUserNotificationSettings:)
name:RCTRegisterUserNotificationSettings
object:nil];
}

- (void)stopObserving
Expand Down Expand Up @@ -93,6 +104,9 @@ + (void)didRegisterUserNotificationSettings:(__unused UIUserNotificationSettings
{
if ([UIApplication instancesRespondToSelector:@selector(registerForRemoteNotifications)]) {
[[UIApplication sharedApplication] registerForRemoteNotifications];
[[NSNotificationCenter defaultCenter] postNotificationName:RCTRegisterUserNotificationSettings
object:self
userInfo:notificationSettings];
}
}

Expand Down Expand Up @@ -145,6 +159,22 @@ - (void)handleRemoteNotificationsRegistered:(NSNotification *)notification
[self sendEventWithName:@"remoteNotificationsRegistered" body:notification.userInfo];
}

- (void)handleRegisterUserNotificationSettings:(NSNotification *)notification
{
if (self.requestPermissionsResolveBlock == nil) {
return;
}
UIUserNotificationSettings *notificationSettings = notification.userInfo;
Copy link
Contributor

@nicklockwood nicklockwood Jun 3, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

@JAStanton This failed to land due to a type error detected by our build system. You're assigning notification.userInfo (which is defined as being an NSDictionary) to a property of type UIUserNotificationSettings. I'm not sure why this worked for you locally, but it seems unlikely that it's correct.

Copy link
Contributor Author

@JAStanton JAStanton Jun 3, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Oh nice, so it works because I was passing in UIUserNotificationSettings to the userInfo parameter of my NSNotification message instead of setting it a dictionary like how it's supposed to be used. Bummer. Updated, take a look! Thanks.

Copy link
Contributor

Choose a reason for hiding this comment

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

@JAStanton ah, I totally missed that. Should just be a simple case of wrapping notificationSettings in a dictionary and then unwrapping it then:

@{ @"settings": notificationSettings }

NSDictionary *notificationTypes = @{
@"alert": @((BOOL)notificationSettings.types & UIUserNotificationTypeAlert),
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this won't actually work, for the reason I mentioned - casting to BOOL is basically just casting to char/int8, it won't actually coerce the value to be true or false

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this needs to be written as

 @"alert": @(notificationSettings.types & UIUserNotificationTypeAlert > 0)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gotcha, alright I played with it on my device and the only change I made was wrapping the bitwise-and in parenthesis before comparing it. Otherwise it works fine 👍

@"sound": @((BOOL)notificationSettings.types & UIUserNotificationTypeSound),
@"badge": @((BOOL)notificationSettings.types & UIUserNotificationTypeBadge),
};

self.requestPermissionsResolveBlock(notificationTypes);
self.requestPermissionsResolveBlock = nil;
}

/**
* Update the application icon badge number on the home screen
*/
Expand All @@ -161,12 +191,22 @@ - (void)handleRemoteNotificationsRegistered:(NSNotification *)notification
callback(@[@(RCTSharedApplication().applicationIconBadgeNumber)]);
}

RCT_EXPORT_METHOD(requestPermissions:(NSDictionary *)permissions)
RCT_EXPORT_METHOD(requestPermissions:(NSDictionary *)permissions
resolver:(RCTPromiseResolveBlock)resolve
rejecter:(RCTPromiseRejectBlock)reject)
{
if (RCTRunningInAppExtension()) {
reject(RCTErrorUnableToRequestPermissions, nil, RCTErrorWithMessage(@"Requesting push notifications is currently unavailable in an app extension"));
return;
}

if (self.requestPermissionsResolveBlock != nil) {
RCTLogError(@"Cannot call requestPermissions twice before the first has returned.");
return;
}

self.requestPermissionsResolveBlock = resolve;

UIUserNotificationType types = UIUserNotificationTypeNone;
if (permissions) {
if ([RCTConvert BOOL:permissions[@"alert"]]) {
Expand Down