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.

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

Conversation

RamsayRomero
Copy link

Sends the didDisplayIncomingCall event when RNCallKeep.displayIncomingCall is called on Android. The documentation is also updated to reflect the which arguments are available on iOS only.

Fixes #671.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

didDisplayIncomingCall event is never sent on Android
2 participants