Add support for repeatInterval to PushNotificationIOS.scheduleLocalNotification (#2)#7219
Add support for repeatInterval to PushNotificationIOS.scheduleLocalNotification (#2)#7219adamterlson wants to merge 1 commit intofacebook:masterfrom
Conversation
|
By analyzing the blame information on this pull request, we identified @slycoder and @allofthenorthwood to be potential reviewers. |
| day: 16, | ||
| hour: 32, | ||
| minute: 64 | ||
| }; |
There was a problem hiding this comment.
What if we just remove that mapping, pass the string value directly to Objc (no need to check the details on JS side) and define something like this (note - you have to check proper enum names):
@implementation RCTConvert (NSCalendarUnit)
RCT_ENUM_CONVERTER(NSCalendarUnit, (@{
@"era": @(NSCalendarUnitEra),
@"week": @(NSCalendarUnitWeekday)
}), 0, integerValue)
@endThat means we can just:
notification.repeatInterval = [RCTConvert NSCalendarUnit:details[@"repeatInterval"]];which defaults to 0 when given null, undefined, and non-matching strings.
Here's an example where to put it -> https://github.com/grabbou/react-native/blob/feature/tab-bar-position/React/Views/RCTTabBarManager.m#L17-L21
|
Thanks for the review @grabbou! |
|
Any update on this PR. I am also looking for this feature for my mobile app. |
|
@adamterlson hey, do you have some time to apply our comments and rebase on top of master? If not - no worries, just let us now so that I can e.g. go ahead and apply neccessary tweaks. It would be really cool to get this shipped asap! |
|
@grabbou Sorry, I have been disposed recently and would like to find the time for sure. My apologies for the delay, I hope to make the fix this weekend when I'm back from travel. |
|
@grabbou Annnddd I didn't have time this weekend. Sorry, you may need to make these changes on your end if you want them in a timely fashion. :( |
|
Changed the part in RCTPushNotificationManager.m to this: Works great on my device, it would be really great if we could make the tests pass. |
|
any updates on this? would love to see this! |
|
@grabbou Could this be considered for the next release? |
|
this can be closed via #10337 |
|
@cmcewen So will this be in the next release? |
Summary: This just cleans up the work done in facebook#7219 by adamterlson based on grabbou's feedback Closes facebook#10337 Differential Revision: D4001540 Pulled By: javache fbshipit-source-id: c73b5fef912fa71d1d988dae41094c9c46dc610b
Less broken PR. Sorry about the hassle. Original PR.
Included update as per @grabbou's and @mkonicek's feedback.