FaceID TouchID and friends by tht7 · Pull Request #316 · mlemgroup/mormaer-mlem

tht7 · 2023-06-26T07:08:12Z

Checklist

I have described what this PR contains

Choose one of the following two options:

- This PR does not introduce major changes
- 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
- 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

RPReplay_Final1687620258.mov

Additional Context

Maybe mlemgroup/mlem#118

sync

…ented - doing that allowed us to push/pop anything we want to the navigation stack - added new link handling, now if a link looks even remotely "sus" I will ask Lemmy's servers if they know it, if they do know it: We push the appropriate object to the navigation stack if they don't know about it and it's an HTTP link, kept the old behavior otherwise I let the system try to open it (which is way nicer than telling the user "it's not supported" especially for mailto: links XD

…vigationLink

…in there SwiftUI should pick up on it better (that solved a bug where the upvote/downvotes weren't updating correctly sometimes - updated every place that expects APIPostView.id == APIPostView.post.id to reference the correct id (usually .post.id) - Since ExpandedPost is not more independent made the postView within it it's state

…n-app # Conflicts: # Mlem.xcodeproj/project.pbxproj

…n-app # Conflicts: # Mlem/Views/Tabs/Posts/Components/Post/Components/Community View/Community Search/Community Search View.swift

- saved a new security on it - included touchID / FaceID

Had to commit some more crims to keep the SwiftUI state all synced up nicely Made sure to noty double up on the NavigationStack in the user page since it really crashs everything for some reason

# Conflicts: # Mlem.xcodeproj/project.pbxproj # Mlem/Extensions/View - Handle Lemmy Links.swift # Mlem/Views/Tabs/Posts/Components/Post/Components/Instance and Community List/Accounts Page.swift

ShadowJonathan

The blur is clear enough that someone can still somewhat make out the things behind it, maybe make it even more blurred?

Otherwise, looks good, other than another code pass

tht7 · 2023-06-26T08:54:03Z

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

ShadowJonathan

Thanks, no further nits, i'll leave @JakeShirley to review this further :)

mormaer · 2023-06-26T09:51:12Z

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 · 2023-06-26T09:54:08Z

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?

tht7 · 2023-06-26T10:21:05Z

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)

JakeShirley · 2023-06-26T20:05:51Z

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

JakeShirley

Minor nit but "Account Settings" makes it sound like it will only apply to a single account but these seem to apply cross-account.

JakeShirley · 2023-06-26T20:08:21Z

Mlem/Views/Tabs/Settings/Components/Views/General.swift

+                                }
+                            } catch {}
+                            // if all went well we shouldn't get here
+                            //                               await MainActor.run {


Looks like some left over debug code

EricBAndrews

One nit, everything else looks good

EricBAndrews · 2023-06-26T20:03:40Z

Mlem/Extensions/View - BioLock.swift

+        if let account = account {
+            if accountsTracker.accountPreferences[account.id]?.requiresSecurity == true {


else condition is idempotent, condense these into a single check

EricBAndrews · 2023-06-26T20:16:39Z

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

Agreed, that's a good idea

…pp, it takes a url and uses our cache to load it, and blurs it according to the users wishes if you marked it as NSFW introduced a new CachedImageProvider that can give you the uniform cached image view that's loaded from a URL and respects the nsfw setting Had to break the Markdown view since it wasn't flexible enough, so now the MarkdownView handles images before passing on the torch of actual markdown to the library's Markdown View

…faceId # Conflicts: # Mlem.xcodeproj/project.pbxproj

…mage-handling # Conflicts: # Mlem/Views/Shared/Markdown View.swift # Mlem/Views/Shared/Website Icon Complex.swift # Mlem/Views/Tabs/Posts/Components/Comment/Components/Comment Item Old.swift # Mlem/Views/Tabs/Posts/Components/Comment/Components/Comment Item.swift # Mlem/Views/Tabs/Posts/Components/Post/Components/Community View/Post Item/Post Types/Large Post.swift

…faceId # Conflicts: # Mlem/App Constants.swift # Mlem/Logic/File Management/Decode Data from File.swift # Mlem/MlemApp.swift # Mlem/Views/Tabs/Posts/Components/Post/Components/Community List View/Community List View.swift # Mlem/Views/Tabs/Settings/Components/Views/General.swift

- made faceID try automaticlly once the device *wakes up from sleep* or when the app returns from the background - also merged the better-ish-image-handling... don't

tht7 · 2023-06-28T17:23:30Z

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

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

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?

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)

mormaer · 2023-06-28T21:18:37Z

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 🙃

EricBAndrews · 2023-06-28T21:29:06Z

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

tht7 · 2023-06-28T21:53:03Z

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

mormaer · 2023-06-28T22:12:31Z

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 😂

# Conflicts: # Mlem.xcodeproj/project.pbxproj # Mlem/Views/Shared/Markdown View.swift # Mlem/Views/Tabs/Posts/Components/Comment/Components/Comment Item Old.swift # Mlem/Views/Tabs/Posts/Components/Comment/Components/Comment Item.swift # Mlem/Views/Tabs/Posts/Components/Post/Components/Community View/Components/Sidebar.swift # Mlem/Views/Tabs/Posts/Components/Post/Components/Community View/Post Item/Post Types/Large Post.swift # Mlem/Views/Tabs/Posts/Components/User/Components/User View.swift

ShadowJonathan · 2023-07-03T10:00:45Z

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 ^^

tht7 and others added 15 commits June 23, 2023 16:02

Merge pull request #2 from mormaer/master

ad6085b

sync

- found some more places that need updating to the new value based Na…

b991e4e

…vigationLink

Merge branch 'mormaer:master' into master-master

26dd81c

Merge remote-tracking branch 'origin/master-master' into open-links-i…

79fd314

…n-app # Conflicts: # Mlem.xcodeproj/project.pbxproj

Merge branch 'mormaer:master' into master-master

1686eaa

Merge remote-tracking branch 'origin/master-master' into open-links-i…

24125ad

…n-app # Conflicts: # Mlem/Views/Tabs/Posts/Components/Post/Components/Community View/Community Search/Community Search View.swift

- missed a file during merge

d11bdbe

- added a little toast message while resolving the link

12e68c6

- added a new place to save changing account preference

8b6895e

- saved a new security on it - included touchID / FaceID

Some bugfixes

57d68e3

Had to commit some more crims to keep the SwiftUI state all synced up nicely Made sure to noty double up on the NavigationStack in the user page since it really crashs everything for some reason

Merge branch 'open-links-in-app' into faceId

b120413

Merge branch 'mormaer:master' into master-master

01717dc

Merge remote-tracking branch 'origin/master-master' into faceId

1fa1a5a

# Conflicts: # Mlem.xcodeproj/project.pbxproj # Mlem/Extensions/View - Handle Lemmy Links.swift # Mlem/Views/Tabs/Posts/Components/Post/Components/Instance and Community List/Accounts Page.swift

tht7 requested a review from JakeShirley June 26, 2023 07:08

ShadowJonathan requested changes Jun 26, 2023

View reviewed changes

The Bio lock screen now uses iOS's thiccMaterial

df0efac

ShadowJonathan previously approved these changes Jun 26, 2023

View reviewed changes

JakeShirley previously approved these changes Jun 26, 2023

View reviewed changes

EricBAndrews requested changes Jun 26, 2023

View reviewed changes

tht7 and others added 3 commits June 27, 2023 15:28

Merge branch 'mormaer:master' into master-master

bf8784e

Merge remote-tracking branch 'origin/better-ish-image-handling' into …

a79d696

…faceId # Conflicts: # Mlem.xcodeproj/project.pbxproj

Itay sharzer and others added 6 commits June 27, 2023 17:25

- removed commented test code

425a2d4

Merge branch 'mormaer:master' into master-master

1a46ebe

- merged the linter

b71e976

- merged the linter

c2adc8e

- made faceID try automaticlly once the device *wakes up from sleep* or when the app returns from the background - also merged the better-ish-image-handling... don't

tht7 dismissed stale reviews from JakeShirley and ShadowJonathan via c2adc8e June 27, 2023 19:14

Merge branch 'mormaer:master' into master-master

45ed7d4

Merge branch 'mormaer:master' into master-master

9d5bd2d

tht7 and others added 2 commits June 29, 2023 09:11

Merge branch 'mormaer:master' into master-master

be2d0a7

ShadowJonathan added the needs migration Issues or PRs that still need to be migrated to the new repo label Jul 3, 2023

		if let account = account {
		if accountsTracker.accountPreferences[account.id]?.requiresSecurity == true {

Conversation

tht7 commented Jun 26, 2023

Checklist

Pull Request Information

Screenshots and Videos

Additional Context

Uh oh!

ShadowJonathan left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tht7 commented Jun 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ShadowJonathan left a comment

Choose a reason for hiding this comment

Uh oh!

mormaer commented Jun 26, 2023

Uh oh!

mormaer commented Jun 26, 2023

Uh oh!

tht7 commented Jun 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JakeShirley commented Jun 26, 2023

Uh oh!

JakeShirley left a comment

Choose a reason for hiding this comment

Uh oh!

JakeShirley Jun 26, 2023

Choose a reason for hiding this comment

Uh oh!

EricBAndrews left a comment

Choose a reason for hiding this comment

Uh oh!

EricBAndrews Jun 26, 2023

Choose a reason for hiding this comment

Uh oh!

EricBAndrews commented Jun 26, 2023

Uh oh!

tht7 commented Jun 28, 2023

Uh oh!

mormaer commented Jun 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

EricBAndrews commented Jun 28, 2023

Uh oh!

tht7 commented Jun 28, 2023

Uh oh!

mormaer commented Jun 28, 2023

Uh oh!

ShadowJonathan commented Jul 3, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ShadowJonathan left a comment •

edited

Loading

tht7 commented Jun 26, 2023 •

edited

Loading

tht7 commented Jun 26, 2023 •

edited

Loading

mormaer commented Jun 28, 2023 •

edited

Loading