PM-10267: Generate master password view #831
Conversation
8f03e44 to
cbfcd8e
Compare
|
No New Or Fixed Issues Found |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #831 +/- ##
==========================================
+ Coverage 88.82% 88.86% +0.03%
==========================================
Files 641 644 +3
Lines 40219 40342 +123
==========================================
+ Hits 35725 35848 +123
Misses 4494 4494 ☔ View full report in Codecov by Sentry. |
.../UI/Auth/CompleteRegistration/MasterPasswordGenerator/MasterPasswordGeneratorProcessor.swift
Show resolved
Hide resolved
|
|
||
| /// `receive(_:)` with `.preventAccountLock` shows the prevent account lock screen. | ||
| @MainActor | ||
| func test_receive_save() { |
There was a problem hiding this comment.
⛏️ This test should be renamed.
| func test_receive_save() { | |
| func test_receive_preventAccountLock() { |
| BitwardenTextField(text: store.binding( | ||
| get: \.generatedPassword, | ||
| send: MasterPasswordGeneratorAction.masterPasswordChanged | ||
| )) | ||
| .submitLabel(.done) |
There was a problem hiding this comment.
🤔 Should this be an actual text field that you can edit? I guess I assumed it would be read-only and use PasswordText.
There was a problem hiding this comment.
yeah good catch! figma shows it with the PasswordText look as well. Making the update
| } label: { | ||
| Text(Localizations.learnAboutOtherWaysToPreventAccountLockout) | ||
| .styleGuide(.footnote, weight: .semibold) | ||
| .foregroundStyle(Asset.Colors.textCodeBlue.swiftUIColor) |
There was a problem hiding this comment.
⛏️ I think the intent behind the semantic colors would be to only use textCodeBlue for the cases where we're displaying a password and highlighting the different character cases. textInteraction would be a better fit for this button. They are both the same blue currently, but that could potentially change in the future.
| .foregroundStyle(Asset.Colors.textCodeBlue.swiftUIColor) | |
| .foregroundStyle(Asset.Colors.textInteraction.swiftUIColor) |
| func generateMasterPassword() async throws -> String { | ||
| try await clientService.generators().passphrase( | ||
| settings: PassphraseGeneratorRequest( | ||
| numWords: 3, |
There was a problem hiding this comment.
❓ Is product ok with this being fixed at 3? I'd use 4 (or customizable by the user) for a stronger master password but well 3 is easier to remember...
There was a problem hiding this comment.
Per the figma design it was 3. I can confirm but I believe when Shannon did this work she was going off the figma doc that was provided.
There was a problem hiding this comment.
Confirmed that Android is using 3 words for their implementation.
🎟️ Tracking
PM-10267
PM-10677
📔 Objective
PreventAccountLockViewfrom the "Generate master password" screen📸 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