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

Send didDisplayIncomingCall event on Android #672

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Send didDisplayIncomingCall event on Android #672

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6ab4d13
Send didDisplayIncomingCall event in correct method
RamsayRomero Feb 27, 2023
1d7583b
Update docs for didDisplayIncomingCall
RamsayRomero Feb 27, 2023
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
5 changes: 4 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -798,7 +798,8 @@ Callback for `RNCallKeep.displayIncomingCall`
```js
RNCallKeep.addEventListener('didDisplayIncomingCall', ({ error, callUUID, handle, localizedCallerName, hasVideo, fromPushKit, payload }) => {
// you might want to do following things when receiving this event:
// - Start playing ringback if it is an outgoing call
// - Navigate to your incoming call screen
// - Handle errors on iOS
});
```

Expand All @@ -816,9 +817,11 @@ RNCallKeep.addEventListener('didDisplayIncomingCall', ({ error, callUUID, handle
- `1` (video enabled)
- `0` (video not enabled)
- `fromPushKit` (string)
- iOS only.
- `1` (call triggered from PushKit)
- `0` (call not triggered from PushKit)
- `payload` (object)
- iOS only.
- VOIP push payload.

### didPerformSetMutedCallAction
Expand Down
20 changes: 9 additions & 11 deletions android/src/main/java/io/wazo/callkeep/RNCallKeepModule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,17 +181,6 @@ public void reportNewIncomingCall(String uuid, String number, String callerName,
Log.d(TAG, "[RNCallKeepModule] reportNewIncomingCall, uuid: " + uuid + ", number: " + number + ", callerName: " + callerName);

this.displayIncomingCall(uuid, number, callerName, hasVideo);

// Send event to JS
WritableMap args = Arguments.createMap();
args.putString("handle", number);
args.putString("callUUID", uuid);
args.putString("name", callerName);
args.putString("hasVideo", String.valueOf(hasVideo));
if (payload != null) {
args.putString("payload", payload);
}
sendEventToJS("RNCallKeepDidDisplayIncomingCall", args);
}

public void startObserving() {
Expand Down Expand Up @@ -329,6 +318,15 @@ public void displayIncomingCall(String uuid, String number, String callerName, b
extras.putString(EXTRA_HAS_VIDEO, String.valueOf(hasVideo));

telecomManager.addNewIncomingCall(handle, extras);

// Send event to JS
WritableMap args = Arguments.createMap();
args.putString("handle", number);
args.putString("callUUID", uuid);
args.putString("localizedCallerName", callerName);
args.putString("hasVideo", hasVideo ? "1" : "0");
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please add the payload attribute ? so there is no breaking change.

Copy link
Author

Choose a reason for hiding this comment

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

Sorry, this change was made under the assumption that the reportNewIncomingCall method was not used, since it is undocumented and uncalled in the native module, so the arguments were changed to match the types that didDisplayIncomingCall expects in the documentation and type definitions. If one was using reportNewIncomingCall and listening for the event in didDisplayIncomingCall, they would be expecting the arguments to also have a name property and expect the hasVideo property to be a value of 'true' | 'false' rather than a value of '0 | 1'. Neither of these arguments are accurately reflected in the documentation or type definitions.

For this change to not be breaking, I would also have to change the localizedCallerName argument back to name, and change the value of hasVideo back to 'true' | 'false' rather than '0 | 1', and update the docs and type definitions to reflect that these properties have different names and types on iOS and Android.

I would also have to add an optional parameter payload to the displayIncomingCall method so that the reportNewIncomingCall method can pass this parameter to displayIncomingCall where sendEventToJS can be called with the payload. I would also have to update the documentation and type definitions to reflect that displayIncomingCall now accepts this additional parameter.

Let me know if these changes sound okay or if you have any other recommendations to make this event fire in a non breaking way.


sendEventToJS("RNCallKeepDidDisplayIncomingCall", args);
}

@ReactMethod
Expand Down