-
Notifications
You must be signed in to change notification settings - Fork 985
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
Replace unmaintained biometrics package #18531
Conversation
Jenkins BuildsClick to see older builds (24)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice one @clauxx : just left a few minor comments
e332d22
to
2b878eb
Compare
1c05e16
to
108d67f
Compare
73% of end-end tests have passed
Failed tests (10)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestCommunityOneDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestDeepLinksOneDevice:
Expected to fail tests (3)Click to expandClass TestCommunityMultipleDeviceMergedTwo:
Class TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Passed tests (35)Click to expandClass TestActivityMultipleDevicePRTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestCommunityOneDeviceMerged:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestActivityMultipleDevicePR:
|
Hey @clauxx thank you for PR. The e2e fails are not PR related. Here are a found issues after performing manual tests: ISSUE 1: [Android] Inability to create additional account via biometricSteps:
Actual result:The additional account is not created successfully. enable_bio.mp4Expected result:The additional account should be successfully created via biometric verification Device:Pixel 7a, Android 14 Logs: |
Probably this issue is also related to #18531 (comment) ISSUE 2: [Android] Inability to send assets via biometricSteps:
Actual result:The user is not navigated to the 'sending process' next page. The asset is not sent slider_ios.mp4Expected result:The user is navigated to the 'sending process' next page. The asset is sent Device:Pixel 7a, Android 14 Logs: |
@VolodLytvynenko Thanks! Fixed both issues in the commit above. |
71% of end-end tests have passed
Failed tests (10)Click to expandClass TestDeepLinksOneDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Expected to fail tests (4)Click to expandClass TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Passed tests (34)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestActivityMultipleDevicePR:
Class TestActivityMultipleDevicePRTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityOneDeviceMerged:
|
80% of end-end tests have passed
Failed tests (2)Click to expandClass TestDeepLinksOneDevice:
Passed tests (8)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
|
Unassigned myself from this PR via broken face id on IOS. @status-im/mobile-qa would be better if someone else could take it into work |
I will take it @VolodLytvynenko |
@clauxx thank you for the fixes. No issues from my side. One thing that would be nice to improve is the error on Android which is displayed in case of too many failed scan attempts We need to replace the 'unknown' part with the actual reason - too many attempts. Try again later. @clauxx this is not a blocker for current PR so it's up to you to decide whether to fix it here or separately. Let me WDYT? |
@pavloburykh Thanks! i'd say let's leave it for another separate issue |
Cool. Then PR is ready for merge. |
fixes #17683
Summary
Replaced
react-native-touch-id
(which hasn't been maintained for >5 years) withreact-native-biometrics
.biometrics/get-available
, which is called before trying to log in with biometrics. It checks if biometrics is enabled on the device, which gets rid of the:biometric/supress-not-enrolled-error
messbiometrics/authorize
to make sure the arguments are passed correctly (there was a bug a while back where the fail handler key was misnamed)Platforms
Areas that maybe impacted
Functional
Steps to test
status: ready