PM-12565: Add settings badges for account setup steps #972
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #972 +/- ##
==========================================
+ Coverage 88.77% 88.80% +0.02%
==========================================
Files 638 638
Lines 40129 40180 +51
==========================================
+ Hits 35625 35680 +55
+ Misses 4504 4500 -4 ☔ View full report in Codecov by Sentry. |
|
No New Or Fixed Issues Found |
|
|
||
| // Kick off this task in init so that the tab bar badge will be updated immediately when | ||
| // the tab bar is shown vs once the user navigates to the settings tab. | ||
| badgeUpdateTask = Task { @MainActor [weak self] in |
There was a problem hiding this comment.
just for my understanding, whats the benefit of storing this task in a variable vs just kicking off the Task in the init?
There was a problem hiding this comment.
Storing a reference to the task allows us to cancel it when the processor goes away (e.g. when you switch accounts or logout, etc).
There was a problem hiding this comment.
makes sense, thanks!
| try await updateSettingsBadgePublisher(userId: userId) | ||
| } | ||
|
|
||
| func setAccountSetupVaultUnlock(_ vaultUnlockSetup: AccountSetupProgress?, userId: String?) async throws { | ||
| let userId = try userId ?? getActiveAccountUserId() | ||
| appSettingsStore.setAccountSetupVaultUnlock(vaultUnlockSetup, userId: userId) | ||
| try await updateSettingsBadgePublisher(userId: userId) |
There was a problem hiding this comment.
🤔 @matt-livefront these methods are currently called in loginWithMasterPassword so potentially if updateSettingsBadgePublisher throws then loginWithMasterPassword would fail which is not correct IMO. The badge update is a secondary effect but that shouldn't block the flow of logging in.
I think the updateSettingsBadgePublisher should not throw and just wrap the code in a do...catch and log any error to the errorReporter so it doesn't affect any caller flows.
There was a problem hiding this comment.
Thanks, great point! I have a fix here: #988.
🎟️ Tracking
PM-12563 - Add badge indicator to bottom navigation for missing autofill setup
PM-12564 - Add badge indicator to bottom nav for missing unlock method setup
PM-12565 - Implement badge count logic
📔 Objective
Adds a badge to the settings tab indicating how many account setup steps remain. These will be added if the user chooses to set up autofill or vault unlock at a later point from the onboarding flow. Once they enable autofill or a vault unlock method, the corresponding badge will be removed.
📸 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