Mauin / RxFingerprint

Android Fingerprint authentication and encryption with RxJava
Apache License 2.0
379 stars 81 forks source link

Fix for launchFingerprintEnrollment #98

Closed Flo354 closed 6 years ago

Flo354 commented 6 years ago

This PR fixes the issue #97

Flo354 commented 6 years ago

I hesitated with this one. But then I saw that somewhere in the code you use the class FingerprintDialog, which is also only available in Android P.

I think we can use directly the constant since this method should be called only for P+.

But if you want I can use the static string and set a @todo to remember that we have to modify this when Android P will be available in its final release.

Le sam. 7 avr. 2018 à 9:25 AM, Marvin Ramin notifications@github.com a écrit :

@Mauin commented on this pull request.

In rxfingerprint/src/main/java/com/mtramin/rxfingerprint/RxFingerprint.java https://github.com/Mauin/RxFingerprint/pull/98#discussion_r179910815:

@@ -413,7 +415,7 @@ public boolean hasEnrolledFingerprints() { }

 public void launchFingerprintEnrollment() {
  • context.startActivity(new Intent("ACTION_FINGERPRINT_ENROLL"));
  • context.startActivity(new Intent(ACTION_FINGERPRINT_ENROLL));

As the constant is only available in the Android P dev preview, should we use the string constant directly instead? "android.settings.FINGERPRINT_ENROLL"

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Mauin/RxFingerprint/pull/98#pullrequestreview-110249137, or mute the thread https://github.com/notifications/unsubscribe-auth/AEDW36oZuwFh32CWwfJnJn3o3mI4u4bbks5tmGnsgaJpZM4TKZmJ .

Mauin commented 6 years ago

You're right. I probably missed some more Android P-only classes that might be exposed. In any case the launchFingerprintEnrollment method should have a @RequiresApi(P) annotation to warn users to only call this on Android P as it won't be available before. Would be great if you can add it as well as part of this PR 👍

Flo354 commented 6 years ago

I will do that.

It's another topic but isn't it possible to get a return from launchFingerprintEnrollment? So we can know when the user has added a fingerprint.