-
Notifications
You must be signed in to change notification settings - Fork 476
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
Fix android error from reusing a callback #81
Conversation
+1 |
This is probably a better fix, just don't handle the |
Thanks @jspaine ... I'll go with the second patch... Thanks a lot!! |
@jspaine This solution works and thanks for the PR. But with this implementation the Fingerprint dialog on android gives no feedback when user scans invalid fingerprint and the dialog stays always open until you get five invalid fingerprints and the system shuts the fingerprint down. Do you have any idea how to handle this? On iOS the dialog flashes and updates itself with info that is was not successful. |
Good point, my phone vibrates twice quickly if it's not recognized but some visual feedback would be nice too. How about a message and maybe changing the icon? |
@jspaine Yeah, other apps usually flash icon indifferent colour or change icon and change text a little, explaining that the attempt failed. |
I've opened #92 as a fix for this as well as another issue. I'm not going to be implementing any sort of UI changes, but would welcome a PR for changing that formatting/allowing other options. |
The dialog closes on android after an incorrect attempt to authenticate, but the app keeps listening for fingerprints and breaks.
I think we just want to leave the dialog open if the error is "Authentication Failed" (fingerprint wasn't recognized). Then when the error is "Too many attempts. Try again later." the dialog closes.
This seems to work fine, but in
FingerprintHandler.java
theendAuth
method also calls theonError
callback with "Authentication Failed" if thecancellationSignal
has already been cancelled for some reason. I guess that's unlikely, but maybe that error message should be changed to avoid issues?fixes #67