Skip to content

Allow user to handle phone account manually #221

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

Closed
wants to merge 15 commits into from

Conversation

jpudysz
Copy link

@jpudysz jpudysz commented Jun 18, 2020

This small PR allows user to handle phone account flow manually.
Currently, the library is too obtrusive and it can be an issue for a well-designed app which doesn't want to display ugly Android alert.

What I'm proposing is another way for the developer to handle Android's phone account. Changes are backwards compatible so it can be released as a minor version.

How react-native-callkeep works now:

  1. User can handle microphone, camera and phone state permissions manually. If user accepts it before RNCallKeep.setup, library won't display any prompts
  2. After initial setup JS part of the lib will prompt the user to enable phone account and we can only inject alert's title and message
  3. As a developer, I cannot change this behaviour

How library will work with this PR merged to master:

  1. Same as in the master branch, we can request every permission beforehand
  2. Developer can check if account permission is already enabled (eg. for visual representation) with new method checkPhoneAccountEnabled
  3. Developer can register phone account manually (before setup) with new method registerPhoneAccount - with such option we can prompt the user and navigate to account settings manually, we can use our UI eg.:

example

  1. At this point, we can setup library and enjoy CallKeep without any additional alerts or prompts ;)

jpudysz added 10 commits June 18, 2020 15:10

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Open app in headless mode, pass additional callUUID
@manuquentin
Copy link
Contributor

Thanks @jpudysz for the PR. I'll take a look.

Meanwhile, can you rebase it on master ?

@manuquentin
Copy link
Contributor

manuquentin commented Jul 16, 2020

Moved in #242
@jpudysz does this PR covers your need ?
The backToForeground method has been improved in #223

@jpudysz
Copy link
Author

jpudysz commented Jul 17, 2020

@manuquentin yup, looks good. It will improve UX a lot.
When it comes to the backToForeground it can be improved more.
I successfully launched my app last week. I created additional improvements to a headless mode like passing callUUID and consuming it after a cold boot. I used it to redirect to my custom WebRTC call UI so I'm not sure it's a common case for the community.

Edit:
You can check it here:
jpudysz@20213af

@jdegger
Copy link

jdegger commented Jul 23, 2020

This PR is great, when is this due to be merged?

@sboily
Copy link
Member

sboily commented Jul 29, 2020

@jdegger we need to have a rebase from @jpudysz and review to merge it.

@jpudysz jpudysz closed this Aug 8, 2020
@jpudysz
Copy link
Author

jpudysz commented Aug 10, 2020

Hello everyone, I'm super busy and I can't contribute here, what's more, we are far behind each other. You can check my repo with a few additional improvements for Android (https://github.com/jpudysz/react-native-callkeep) - tested on production as I'm developing for my client's VoIP app.

I tried to create PR here, but I changed backToForeground logic and it differs from original repo. New functionality such as headless mode, locked screen are strictly connected with this function and I can't merge it. Ideally, we can take my ideas and merge it with the master (I don't mind and you can copy-paste my code under your PR). Ping me if I needed.

PS. I added changelog inside my repo, so you can check new functionalities.
@manuquentin if you want to discuss how to merge it or you need some explanation ping/PM me.

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.

None yet

4 participants