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

chore: improve biometrics permission prompting. #1301

Conversation

tom11-nguyen
Copy link
Contributor

@tom11-nguyen tom11-nguyen commented Oct 31, 2024

Summary of Changes

  • Move biometric system prompt to onboarding steps, when the user toggles use-biometrics switch (iOS).
  • Improve UX when biometrics is not setup, unavailable, or previously denied.
  • Fix typo in AndroidManifest file.

Related Issues

bcgov/bc-wallet-mobile#2111
#999

Pull Request Checklist

Tick all boxes below to demonstrate that you have completed the respective task. If the item does not apply to your this PR check it anyway to make it apparent that there's nothing to do.

  • All commits contain a DCO Signed-off-by line (we use the DCO GitHub app to enforce this);
  • Updated LICENSE-3RD-PARTY.md for any added dependencies or vendored components;
  • Updated documentation as needed for changed code and new or modified features;
  • Added sufficient tests so that overall code coverage is not reduced.

If you have any questions to any of the points above, just submit and ask! This checklist is here to help you, not to deter you from contributing!

Pro Tip 🤓

  • Read our contribution guide at least once; it will save you a few review cycles!
  • Your PR will likely not be reviewed until all the above boxes are checked and all automated tests have passed.

PR template adapted from the Python attrs project.

@tom11-nguyen tom11-nguyen force-pushed the chore/prompt-biometrics-permission-onboarding branch from 09b373d to 650edc3 Compare November 3, 2024 03:04
@tom11-nguyen tom11-nguyen marked this pull request as ready for review November 5, 2024 04:05
@@ -153,9 +238,9 @@ const UseBiometry: React.FC = () => {
<ToggleButton
testID={testIdWithKey("ToggleBiometrics")}
isEnabled={biometryEnabled}
isAvailable={biometryAvailable}
isAvailable={true}
Copy link
Contributor

Choose a reason for hiding this comment

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

is it ok that these are hard coded values? the diff shows biometryAvailable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it was changed so that the biometric switch is now always enabled for interaction, it will shows a popup with messages when biometryAvailable == false; instead of being disabled from interacting entirely before this change.

Copy link
Contributor

@bryce-mcmath bryce-mcmath left a comment

Choose a reason for hiding this comment

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

Have you also tested these changes when altering biometry setting from the settings menu (after user has logged in)? ie. the case where usage is UseBiometryUsage.ToggleOnOff?

@tom11-nguyen
Copy link
Contributor Author

Have you also tested these changes when altering biometry setting from the settings menu (after user has logged in)? ie. the case where usage is UseBiometryUsage.ToggleOnOff?

Yes, I did test that screen, too

Copy link
Contributor

@bryce-mcmath bryce-mcmath left a comment

Choose a reason for hiding this comment

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

Just a note for next time, for larger UI work like this, it's helpful to add a gif or video to the PR showing what the changes look like. You can use quicktime on your mac and/or ezgif for this

Nguyen, Tom CITZ:EX added 9 commits November 6, 2024 15:30
…cs switch during onboarding, add new strings.

Signed-off-by: Nguyen, Tom CITZ:EX <[email protected]>
…iometric is unavailable.

Signed-off-by: Nguyen, Tom CITZ:EX <[email protected]>
Signed-off-by: Nguyen, Tom CITZ:EX <[email protected]>
Signed-off-by: Nguyen, Tom CITZ:EX <[email protected]>
@bryce-mcmath bryce-mcmath force-pushed the chore/prompt-biometrics-permission-onboarding branch from 2f9ef84 to f4eb26c Compare November 6, 2024 23:30
@bryce-mcmath bryce-mcmath merged commit 4766aa2 into openwallet-foundation:main Nov 6, 2024
6 checks passed
Copy link

sonarqubecloud bot commented Nov 6, 2024

@tom11-nguyen tom11-nguyen deleted the chore/prompt-biometrics-permission-onboarding branch November 7, 2024 18:03
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