Task/103/android phone auth change #369
Conversation
…atic authentication fails to trigger
|
**this is for issue 104 |
|
Hey guys, when will this be released? |
| @@ -123,6 +124,7 @@ actual interface PhoneVerificationProvider { | |||
| val activity: Activity | |||
| val timeout: Long | |||
| val unit: TimeUnit | |||
| fun onRecievedVerificationID(verificationID: String) | |||
| fun codeSent(triggerResend: (Unit) -> Unit) | |||
There was a problem hiding this comment.
why separate method, instead of the codeSent's parameter?
There was a problem hiding this comment.
I don't want to change a method signature. Also, less convoluted code is easier to read and maintain. Separation of concern. Separation of responsibility.
There was a problem hiding this comment.
Why does it (changing method signature) matter? It doesn't seem like anyone is using gitlive's "sign in with phone" stuff.... because as soon as you try to use it - you run into this very problem (#104), which is a UX show-stopper.
For the separation of concerns, I think this is artificial. The Firebase documentation (https://firebase.google.com/docs/auth/android/phone-auth#oncodesentstring-verificationid,-phoneauthprovider.forceresendingtoken) is describing the verification process, where verificationId becomes known at the time of codeSent callback, and you'd be better off sticking to same semantics, to reduce the need to search for how gitlive does it.
Also, why is there Unit parameter in triggerResend: (Unit) -> Unit) -- should be just triggerResend: () -> Unit) - a normal signature of a method that doesn't take any parameters 🤷 You may as well add verificationId parameter, and fix triggerResend in one shot.
There was a problem hiding this comment.
I am not sure about the last part but I have worked with many sdks and the smaller the pieces the better. Less siginture changes the better.
Either way this update is needed and had been needed for a very long time. If it doesn't happen soon I will have to shift into building and maintaining my own SDK for this which is what I am trying to avoid.
There was a problem hiding this comment.
api should follow android sdk apart from the kotlin-first design principles defined in the readme, is this the case for this?
If you paste in links to the relevant firebase sdk documentation I would be happy to confirm it for you
There was a problem hiding this comment.
override fun onCodeSent(
verificationId: String,
token: PhoneAuthProvider.ForceResendingToken
) {
Line 68
There was a problem hiding this comment.
Android applications need that ID or they cant complete the phone code auth.
There was a problem hiding this comment.
Someone else may need this so I'll leave it open but I am pivoting away from this.
|
What do we need to get this reviewed? |
This allows android to have the verificationID when auto verify fails. This lets android have the verificationID to manually continue