Skip to content
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

1. fix error.code; 2. Fix the android cancel button; 3. Add the method of authenticateDevice to enable device password #133

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

orzhtml
Copy link

@orzhtml orzhtml commented Apr 24, 2020

  1. fix error.code;

  2. Fix the android cancel button;

  3. Add the method of authenticateDevice to enable device password

orzhtml added 4 commits April 24, 2020 16:33
2、安卓修复 lockout 错误的状态
3、其他优化
2、ios 新增 authenticateDevice 方法,唤起设备密码解锁
3、ios 修复 biometric 返回的 code 不正确的问题
4、新增错误消息文案
Comment on lines 292 to 295
if( availableTimes <= 0 ){
mReactContext.getJSModule(RCTDeviceEventEmitter.class)
.emit("FINGERPRINT_SCANNER_AUTHENTICATION", "AuthenticationLockout");
}else{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if( availableTimes <= 0 ){
mReactContext.getJSModule(RCTDeviceEventEmitter.class)
.emit("FINGERPRINT_SCANNER_AUTHENTICATION", "AuthenticationLockout");
}else{
if (availableTimes <= 0) {
mReactContext.getJSModule(RCTDeviceEventEmitter.class)
.emit("FINGERPRINT_SCANNER_AUTHENTICATION", "AuthenticationLockout");
} else {

I think general whitespace style (the majority at least) is like this in the file

README.md Outdated
.authenticate({ description: 'Log in with Biometrics' })
.authenticate({
description: 'Scan your fingerprint on the device scanner to continue',
cancelButton: 'cancel',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was the android implementation using a different signature than iOS before? It is inconvenient to have separate APIs for the platforms, but maybe that is unavoidable

@mikehardy
Copy link
Collaborator

I'm just an interested user, not a maintainer or anything, so these are just things I saw, but it may be that no one cares :-), that is fine

Here is what I saw:

This would be a breaking change because API signatures changed and the change does have default or optional values, but there is no note of that in the README or a CHANGELOG

The default return value for iOS was changed but I didn't see that mentioned in the README?

The example was not updated to show the changes, it would prove that it worked, this is a large change how have you tested it / how do you show it works without the example altered to match?

@orzhtml
Copy link
Author

orzhtml commented Apr 26, 2020

I'm just an interested user, not a maintainer or anything, so these are just things I saw, but it may be that no one cares :-), that is fine

Here is what I saw:

This would be a breaking change because API signatures changed and the change does have default or optional values, but there is no note of that in the README or a CHANGELOG

The default return value for iOS was changed but I didn't see that mentioned in the README?

The example was not updated to show the changes, it would prove that it worked, this is a large change how have you tested it / how do you show it works without the example altered to match?

I don't understand what you are saying. Does readme.md have a new API

@mikehardy
Copy link
Collaborator

@orzhtml
Copy link
Author

orzhtml commented Apr 29, 2020

@phillbaker
Copy link
Collaborator

It seems like (2) is a dupe of #120, which was just merged. Please rebase this PR for any differences in functionality for the cancel button.

Please separate the functionality for (1) and (3) into individual PRs.

orzhtml added 2 commits April 30, 2020 10:50
…rprint-scanner into hieuvp-master

# Conflicts:
#	README.md
#	android/src/main/java/com/hieuvp/fingerprint/ReactNativeFingerprintScannerModule.java
#	src/authenticate.android.js
…rprint-scanner into hieuvp-master

# Conflicts:
#	README.md
# android/src/main/java/com/hieuvp/fingerprint/ReactNativeFingerprintScannerModule.java
#	src/authenticate.android.js
@orzhtml
Copy link
Author

orzhtml commented Apr 30, 2020

It seems like (2) is a dupe of #120, which was just merged. Please rebase this PR for any differences in functionality for the cancel button.

Please separate the functionality for (1) and (3) into individual PRs.

Conflict resolved

orzhtml added 2 commits May 7, 2020 10:40
…rprint-scanner into hieuvp-master

# Conflicts:
#	android/src/main/java/com/hieuvp/fingerprint/ReactNativeFingerprintScannerModule.java
#	ios/ReactNativeFingerprintScanner.m
@orzhtml
Copy link
Author

orzhtml commented May 7, 2020

Conflict resolved

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.

3 participants