Mlem icon indicating copy to clipboard operation
Mlem copied to clipboard

FaceID TouchID and friends

Open tht7 opened this issue 1 year ago • 12 comments

Checklist

  • [x] I have described what this PR contains

Choose one of the following two options:

    • [ ] This PR does not introduce major changes
    • [x] This PR introduces major changes, and I have consulted the Mlem Development Matrix room

Choose one of the following two options:

    • [ ] This PR does not change the UI in any way
    • [x] This PR adds new UI elements / changes the UI, and I have attached pictures or videos of the new / changed UI elements

Pull Request Information

Wop wop made a new place to place all changing account preferences that's way more reactive with SwfitUI added a new option to lock an account with FaceID

Screenshots and Videos

https://github.com/mormaer/Mlem/assets/6067331/9c9c665f-f7a6-48d9-a6a2-7b76a7827bfe

Additional Context

Maybe mlemgroup/mlem#118

tht7 avatar Jun 26 '23 07:06 tht7

I found your comment kinda funny buttt I loved it so, there you go it’s the same screen (of the Mlem community) but now with iOS’s thiccMaterial image

tht7 avatar Jun 26 '23 08:06 tht7

should a blocking screen like this not be presented as a .fullScreenCover?

in the video we're only covering the central content, the navigation and tab bars remain visible which show potentially sensitive information like the users name and community names etc?

Is interaction blocked on those visible elements?

mormaer avatar Jun 26 '23 09:06 mormaer

Another small thought, generally biometric locks like this on iOS will attempt authentication automatically when you open, as opposed to requiring the user to interact by tapping?

mormaer avatar Jun 26 '23 09:06 mormaer

I was still working on this, but in order to achieve this, we need to introduce some artificial delay since swiftUI doesn’t like it for some reason (and also a lot of times FaceID is so fast you don’t even notice the locking screen is there which is confusing and jarring especially if the faceID sensor is still warm from unlocking your phone)

tht7 avatar Jun 26 '23 10:06 tht7

Visually could we align this with the darkened blur + white text?

JakeShirley avatar Jun 26 '23 20:06 JakeShirley

Visually could we align this with the darkened blur + white text?

Agreed, that's a good idea

EricBAndrews avatar Jun 26 '23 20:06 EricBAndrews

Whoa guys I didn’t notice all those Nits so let me address them all

  1. This is not displayed using a full cover, I wanted the user to be able to go back, especially to the accounts, or the Settings menu if he’s ever in trouble, but be sure that the content itself has interactions disabled

1.2 Once Face ID has been enabled for a certain account, you need to scan Face ID in order to remove it so you can’t remove it without consequence

1.3 As an escape mechanism, if you delete an account and relogin, Face ID goes away, I didn’t want to create a death trap and if they know the password, they are pretty much the owner of the account XD

  1. I’ve made a change to the biometrics such that when the app becomes foreground or the device wakes up from sleep the biometrics will be attempted automatically, but I left in the tapping action just in case something goes wrong.

3 and 5. You guys need to make up your mind I got told that the shield is too transparent, making the content underneath semi-visual and so I made it specifically iOS’s thiccMaterial So do you want it less opaque?

  1. It says account settings because it is genuinely account specific settings.

The Face ID protects per account So, if you go back to the account list and choose a different account, Face ID won’t protect it if it’s not specifically ticked off (my use case here was the obvious regular and alt accounts where the regular is a meme stream you can share with friends and the alt account needs to be protected, so it would be less annoying if you could leave the biometrics off for the main account, but enable them on the secondary account)

tht7 avatar Jun 28 '23 17:06 tht7

This is not displayed using a full cover, I wanted the user to be able to go back, especially to the accounts, or the Settings menu if he’s ever in trouble, but be sure that the content itself has interactions disabled

I must say I disagree with this approach, any and all biometric lock I've seen implemented in iOS blocks all interaction with the application. FaceID/TouchID is a reliable system feature, and as long as we allow fallback to the users passcode they're not at risk of getting stuck unless they've lost their finger / face and forgot their code 🙃

What do other people think, should only part of the interface be obscured when locked?

I can see some value in allowing it to be set by account, but to me again we'd block the entire interface, if someone needed out they could always swap the app away and when it re-opens it will be on the account screen, tapping on a protected account could prompt for biometrics before pushing into the user feed?

--- EDIT

Also, just to be clear I'm saying I disagree, I'm not the boss, it's just my opinion 🙃

mormaer avatar Jun 28 '23 21:06 mormaer

We've been erring on the side of "do it the Apple way" pretty much every time it's come up, for consistency I'm going to agree with morm here

EricBAndrews avatar Jun 28 '23 21:06 EricBAndrews

I honestly don’t disagree. I think both approaches have a value to them, and I wanted to air on the side of caution (to me, that meant a lock that you can always escape from)

So I'll gladly change it

(I believe we should strive to be system, consistent and looking at a few examples I can confirm that the usual behavior is Face ID stop the interaction all together (so full cover), and when it comes to critical stuff when you can’t scan Face ID, or tap your passcode, it goes back to the previous interaction, which matches your description, so I’m also happy to change)

I will change the behavior for this feature in my next update

tht7 avatar Jun 28 '23 21:06 tht7

Screenshot 2023-06-28 at 23 10 20

here's a disgusting draw.io fever-dream of how I would envision it working if we offer it per account 🙈

I know, I know, I should be a designer 😂

mormaer avatar Jun 28 '23 22:06 mormaer

Heya @tht7, we have a new repo at https://github.com/mlemgroup/mlem, due to some shuffling around we couldn't get it migrated properly, but if you could reopen this PR there, and link it back here, then it can continue to be considered ^^

ShadowJonathan avatar Jul 03 '23 10:07 ShadowJonathan