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

added lock feature #4

Closed
wants to merge 6 commits into from
Closed

Conversation

danger-ahead
Copy link
Contributor

@danger-ahead danger-ahead commented Nov 14, 2021

  • tested on Android
  • tested on iOS
  • added all the necessary code changes for both Android and iOS for the feature to work

Lets the user to use biometrics as well as pin or pattern.
Also adds the ability to lock or unlock the app from the Profile section.

Uses shared_preferences to check if the user has turned on locking.
Uses local_auth for the actual authorisation feature to work.

Addresses #2

@danger-ahead
Copy link
Contributor Author

Do not merge this now. It has a bug. Will fix this tomorrow.

@31Carlton7
Copy link
Owner

So far so good! However, is it possible that you could use hive instead of shared_preferences for local storage, as hive's performance is much better and I just recently migrated the app from using shared_preferences to hive.

@danger-ahead
Copy link
Contributor Author

danger-ahead commented Nov 15, 2021

is it possible that you could use hive

Okay will be changing that too.

Changed Android app package name to `com.notes_app`
Changed iOS Face ID reason to a proper reason
@danger-ahead
Copy link
Contributor Author

@31Carlton7 Don't merge this PR yet. This needs further testing and polishing.

@danger-ahead
Copy link
Contributor Author

@31Carlton7 Don't merge this PR yet. This needs further testing and polishing.

Hi @31Carlton7
Would you please test this PR on your emulator (or device)?

Actually I think it's much consistent to work with local_auth if we're working with a stateful widget. However, in our case, the root widget of our app is a stateless widget.
Though I think it's working fine. Let me know if you encounter any error or bug.
Also one possible fix could be migrating the root widget to a stateful widget (I might be wrong here, just learning Flutter).

- Replaced a deprecated API in AndroidManifest.xml
- Fixed a bug in `authorisation.dart` that prevented the user from changing
  lock status
- Moved authenticate() under home_view.dart
@danger-ahead
Copy link
Contributor Author

@31Carlton7 I tried to fix most of the bugs.

One still remains:

Here, the contents of the top most note remains visible prior to authorization. I'm not much well accustomed to state management. Perhaps this bug can be solved by delaying the render of home_view till authenticate() has returned true. This portion resides in home_view.dart.

Also you can now MERGE this PR.

- This is a bug noticed in Android.
For more reference: flutter/flutter#83773
@31Carlton7
Copy link
Owner

I'll take a look at it either tonight or tomorrow!

@danger-ahead
Copy link
Contributor Author

I'll take a look at it either tonight or tomorrow!

I didn't change back to hive because only one pair of key-value is being used for the locking algorithm. And shared_preferences performs better when it comes to reading data. I found out about this from here.

@31Carlton7
Copy link
Owner

I just went through the code and tested it, and the feature doesn't work. None of the notes are locked, and it didn't exactly do anything. Please review your code and update the app to lock the notes and require password auth when trying to view a note.

@danger-ahead
Copy link
Contributor Author

VID-20211117-WA0038.mp4

I tested this on multiple devices.
Which device are you using? And what's the error log?
Kindly upload the console log.

@danger-ahead
Copy link
Contributor Author

danger-ahead commented Nov 19, 2021

@31Carlton7 awaiting your reply.

@31Carlton7 31Carlton7 closed this Apr 11, 2022
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.

None yet

2 participants