search

react-native-webrtc / react-native-callkit

#deprecated iOS 10 new CallKit framework for React Native
ISC License
122 stars 67 forks source link

ContactIdentifier added to startCall action. #34

Closed aarkalyk closed 6 years ago

sujameslin commented 6 years ago

@aarkalyk Would you please review the feedback that I left?

aarkalyk commented 6 years ago

@sujameslin Sorry, mate, I can't see any feedback. Could you, please, send me the link?

sujameslin commented 6 years ago

@aarkalyk Weird. Not sure why you can't see my feedback in this PR. Anyway I attached screenshot which will help you. screen shot 2018-03-02 at 9 23 09 pm

Also would you please elaborate README.md to include your localizedCallerName params? Thanks

aarkalyk commented 6 years ago

@sujameslin It's indeed weird that I can't see your comments. Thanks for the screenshot! I'll updated the PR soon.

aarkalyk commented 6 years ago

here's how contactIdentifier is described in the docs

The identifier is displayed in the native call UI, and is typically the name of the call recipient. If a caller corresponds to a CNContact object, set this to the value of the identifier property of the contact.

Although, it can be used as a name as well, it's not the same thing as localizedCallerName. As we can see it can be used as a contact identifier too.

A value that uniquely identifies a contact on the device.

I don't feel comfortable renaming it as localizedCallerName, I think description in README should be enough.

sujameslin commented 6 years ago

@ianlin Can we merge this PR? For me, it looks good.

ianlin commented 6 years ago

Thanks guys!