Guidance for a PR

[jmesquita]

Hi, I'm not an Objective-C developer and I'm pretty new to React-Native native modules so I need some guidance to reach a specific goal that I think should benefit the project greatly.

On all callbacks in which we receive actions such as CXAnswerCallAction (on performAnswerCallAction), while inspecting the object, I saw that we're able to fetch the UUID of the call that is being acted upon.

Adding that parameter to the event would be of much use to the application that is trying to deal with multiple calls. Despite my efforts, I wasn't able to get this to work and I'm sure I must be missing something rather simple. Here's the modification I've made:

// Answering incoming call
- (void)provider:(CXProvider *)provider performAnswerCallAction:(CXAnswerCallAction *)action
{
#ifdef DEBUG
    NSLog(@"[RNCallKit][CXProviderDelegate][provider:performAnswerCallAction]");
#endif
    if (![self lessThanIos10_2]) {
        [self configureAudioSession];
    }
    [self sendEventWithName:RNCallKitPerformAnswerCallAction body:(@{ @"callUUID" : (action.callUUID.UUIDString) })];
    [action fulfill];
}

I have no problem doing the work but I need some guidance towards what I'm missing here. Thank you in advance!

[ianlin]

Hi @jmesquita, thanks for contributing. You have done right on the Obj-C side. However you still need to modify the JS side so the event payload can be passed to the user correctly.

Add the following and everything is good:

--- node_modules/react-native-callkit/index.js
+++ node_modules/react-native-callkit/index.js
@@ -29,7 +29,7 @@
         } else if (type === 'answerCall') {
             listener = _RNCallKitEmitter.addListener(
                 RNCallKitPerformAnswerCallAction,
-                () => { handler();}
+                (data) => { handler(data);}
             );
         } else if (type === 'endCall') {
             listener = _RNCallKitEmitter.addListener(

Feel free to let me know if there's any question!

[jmesquita]

Thank you Ian! I'll do a PR later today. Have you considered using promises instead of the events you're currently using? Reason I'm asking is because incoming calls as an example could throw an error on several occasions such as night mode or blocking users but those errors are not being propagated to the app. A promise based approach could be a good way to implement that sort of error handling. Don't you think? I can come up with a second PR for that but wanted to know your thoughts first.

…

Sent from my iPhone

On Apr 25, 2017, at 12:19 AM, Ian Yu-Hsun Lin ***@***.***> wrote: Hi @jmesquita, thanks for contributing. You have done right on the Obj-C side. However you still need to modify the JS side so the event payload can be passed to the user correctly. Add the following and everything is good: --- node_modules/react-native-callkit/index.js +++ node_modules/react-native-callkit/index.js @@ -29,7 +29,7 @@ } else if (type === 'answerCall') { listener = _RNCallKitEmitter.addListener( RNCallKitPerformAnswerCallAction, - () => { handler();} + (data) => { handler(data);} ); } else if (type === 'endCall') { listener = _RNCallKitEmitter.addListener( Feel free to let me know if there's any question! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[ianlin]

Hi @jmesquita, you're right using promises is better than events in this case. Just do it!

The reason I used events is performAnswerCallAction and other CXProviderDelegate are event based as well. To use promise the only way I can think of is we need to have some global promise resolvers for each event. Have you got any thoughts on it?

[jmesquita]

Yes, it is event based indeed but I was thinking more on the lines of adding promise to this specific call:

// Display the incoming call to the user
RCT_EXPORT_METHOD(displayIncomingCall:(NSString *)uuidString
                               handle:(NSString *)handle
                               handleType:(NSString *)handleType
                             hasVideo:(BOOL)hasVideo)
{
#ifdef DEBUG
    NSLog(@"[RNCallKit][displayIncomingCall] uuidString = %@", uuidString);
#endif
    int _handleType = [self getHandleType:handleType];
    NSUUID *uuid = [[NSUUID alloc] initWithUUIDString:uuidString];
    CXCallUpdate *callUpdate = [[CXCallUpdate alloc] init];
    callUpdate.remoteHandle = [[CXHandle alloc] initWithType:_handleType value:handle];
    callUpdate.supportsDTMF = YES;
    // TODO: Holding
    callUpdate.supportsHolding = NO;
    callUpdate.supportsGrouping = NO;
    callUpdate.supportsUngrouping = NO;
    callUpdate.hasVideo = hasVideo;

    [self.callKitProvider reportNewIncomingCallWithUUID:uuid update:callUpdate completion:^(NSError * _Nullable error) {
        if (error == nil) {
            // Workaround per https://forums.developer.apple.com/message/169511
            if ([self lessThanIos10_2]) {
                [self configureAudioSession];
            }
        }
    }];
}

There are clear reasons for the reporting to fail that are valid such as Do Not Disturb so I guess that should be checked and a promise should be rejected. This could be a start and we could be moving on to other methods as they make sense to do it.

[ianlin]

Yeah it totally make sense to add a promise here, just go ahead 👍