PM-10269: Add initial UI for set up unlock screen #818
Conversation
|
No New Or Fixed Issues Found |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #818 +/- ##
==========================================
+ Coverage 88.42% 88.46% +0.03%
==========================================
Files 596 600 +4
Lines 30093 30224 +131
==========================================
+ Hits 26611 26738 +127
- Misses 3482 3486 +4 ☔ View full report in Codecov by Sentry. |
| } | ||
|
|
||
| /// A key path for getting/setting whether the unlock method is turned on in the state. | ||
| var keyPath: WritableKeyPath<VaultUnlockSetupState, Bool> { |
There was a problem hiding this comment.
I'm curious what the advantage is of using key paths here?
There was a problem hiding this comment.
I'd say it avoids if/switch logic to get/set the unlock method so it's more direct to use.
There was a problem hiding this comment.
Yeah, exactly. You can implement both a get (state[keyPath: unlockMethod.keyPath]) and set (state[keyPath: unlockMethod.keyPath] = newValue) without having to implement the switch twice.
|
|
||
| // MARK: - VaultUnlockSetupView | ||
|
|
||
| /// A view that allows the user to swipe through the intro carousel and then proceed to creating an |
There was a problem hiding this comment.
Update docs here?
There was a problem hiding this comment.
Good catch, thanks!
| /// The available unlock methods to show in the UI. | ||
| var unlockMethods: [UnlockMethod] { | ||
| let biometricsMethod: UnlockMethod? = if case let .available(type, _, _) = biometricsStatus { | ||
| switch type { | ||
| case .faceID: .faceID | ||
| case .touchID: .touchID | ||
| } | ||
| } else { | ||
| nil | ||
| } | ||
|
|
||
| return [biometricsMethod, .pin].compactMap { $0 } | ||
| } |
There was a problem hiding this comment.
🎨 IMO this is easier as:
| /// The available unlock methods to show in the UI. | |
| var unlockMethods: [UnlockMethod] { | |
| let biometricsMethod: UnlockMethod? = if case let .available(type, _, _) = biometricsStatus { | |
| switch type { | |
| case .faceID: .faceID | |
| case .touchID: .touchID | |
| } | |
| } else { | |
| nil | |
| } | |
| return [biometricsMethod, .pin].compactMap { $0 } | |
| } | |
| /// The available unlock methods to show in the UI. | |
| var unlockMethods: [UnlockMethod] { | |
| guard case let .available(type, _, _) = biometricsStatus else { | |
| return [.pin] | |
| } | |
| switch type { | |
| case .faceID: | |
| return [.faceID, .pin] | |
| case .touchID: | |
| return [.touchID, .pin] | |
| } | |
| } |
There was a problem hiding this comment.
I ended up changing this slightly in my next PR, if you're ok with waiting on this until then.
🎟️ Tracking
PM-10269
📔 Objective
Adds the initial UI for the set up unlock method screen as part of the create account flow. This isn't accessible in the app yet, but can be accessed by setting
authCompletionRouteto.auth(.vaultUnlockSetup)in AppCoordinator. The functionality for enabling biometrics and pin unlock will come in a future PR.📸 Screenshots
⏰ Reminders before review
🦮 Reviewer guidelines
:+1:) or similar for great changes:memo:) or ℹ️ (:information_source:) for notes or general info:question:) for questions:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion:art:) for suggestions / improvements:x:) or:warning:) for more significant problems or concerns needing attention:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt:pick:) for minor or nitpick changes