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

feat: add phone lock (pin, biometrics, etc) #120

Merged
merged 8 commits into from
Sep 19, 2024
Merged

Conversation

im-adithya
Copy link
Member

@im-adithya im-adithya commented Sep 13, 2024

Fixes #35

Description

Adds basic auth using expo-local-authentication, also adds this lock if the inactivity time is more than 5s (when kept in background) (I think we can increase this a bit)

And do we wish to add a screen to block the display when it's in recent apps, let me know if I should add that as well

Screenshots

@im-adithya im-adithya requested review from rolznz and reneaaron and removed request for rolznz September 13, 2024 07:26
lib/constants.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@reneaaron reneaaron left a comment

Choose a reason for hiding this comment

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

Great PR! Just tested it and it worked like a charm 👍

If you haven't set up biometrics on your phone you won't see this setting now. I wondered if that can lead to some confusion for users (who might see a screenshot in the docs, etc). Would it be better to always display the setting, but disable / explain you first have to set it up the /security page?

@im-adithya
Copy link
Member Author

Yeah I had the same question, maybe we keep the setting and gray it out if biometrics aren't added? (with some text below suggesting to add biometrics to enable?)

Also we somehow have to explain this feature as well, maybe right after the onboarding & wallet setup we show a popup saying "Add biometrics" / Setup Later?

@reneaaron
Copy link
Contributor

with some text below suggesting to add biometrics to enable?

That probably breaks the rhythm of the settings page here, that's why i suggested to always display the setting but explain you have to enable it on the detail page.

Also we somehow have to explain this feature as well, maybe right after the onboarding & wallet setup we show a popup saying "Add biometrics" / Setup Later?

I don't think we need to include it an the onboarding at this point. Let's keep it simple, users can enable it after they familiarized themselves with the app.

@reneaaron reneaaron added this to the 1.5 milestone Sep 16, 2024
@bumi
Copy link

bumi commented Sep 16, 2024

does this only work with biometrics or also with some "normal" unlock screen? I don't use Face ID and fingerprints just to make the wrench attack a little harder :D

@im-adithya
Copy link
Member Author

Yes it works with any pattern/pin/password you set for unlock screen not just face id/fingerprint

@reneaaron
Copy link
Contributor

image

Tried to improve the copy a bit and replaced the custom alert with a card.

@reneaaron reneaaron changed the title feat: add biometrics feat: add phone lock (pin, biometrics, etc) Sep 17, 2024
Copy link
Contributor

@reneaaron reneaaron left a comment

Choose a reason for hiding this comment

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

Some minor things to sort out, otherwise

tACK

🙌

lib/constants.ts Outdated Show resolved Hide resolved
context/UserInactivity.tsx Show resolved Hide resolved
lib/state/appStore.ts Outdated Show resolved Hide resolved
lib/state/appStore.ts Outdated Show resolved Hide resolved
@reneaaron reneaaron merged commit 0d03586 into master Sep 19, 2024
1 check passed
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.

Add biometrics
4 participants