standardnotes / iOS-classic

iOS client - (Deprecated) - New version: https://github.com/standardnotes/mobile
167 stars 21 forks source link

Issue 8 Touch ID support #13

Closed jz709u closed 7 years ago

jz709u commented 7 years ago

-added touchID toggle button -added Touch ID authentication to AppDelegate and lockout popup -added user defaults for touchdIDEnabled on the UserManager

moughxyz commented 7 years ago

Hey, thank you for the PR. I've tested it and it works well, but a few changes are needed.

  1. The "Enable TouchID" button is grayed out and looks unselectable. I think you should use the same style as the other buttons. That is, it should always be red text, but the text should be either enable/disable.
  2. When the app is locked, it should not show notes list, it should show account tab instead.
  3. Even with TouchID enabled, I was able to select "Export Data" in between the alerts. This is a major issue, since anyone can export my data even when I've enabled TouchID.
  4. For code style, I think we should try to copy existing style as much as possible, to make it more uniform. This means a lot less space in between lines of code.

Otherwise, looks great! Thank you for your help.

jz709u commented 7 years ago

Done

moughxyz commented 7 years ago

Are you able to change the button style for Enable TouchID to match the others? I think instead of

self.touchIDButton.isSelected = UserManager.sharedInstance.touchIDEnabled

it should be:

self.touchIDButton.isEnabled = true
if UserManager.sharedInstance.touchIDEnabled {
 self.touchIDButton.text = "Disable TouchID"
} else {
 self.touchIDButton.text = "Enable TouchID"
}

Something like that..

jz709u commented 7 years ago

updated

moughxyz commented 7 years ago

Thanks @jz709u! Will review this later today.

moughxyz commented 7 years ago

This functionality is really cool, thanks @jz709u