[PM-15634] Add intermediate Export settings view #1227
Conversation
# Conflicts: # BitwardenShared/Core/Platform/Services/ServiceContainer.swift # BitwardenShared/Core/Platform/Services/TestHelpers/ServiceContainer+Mocks.swift
…approach and fixed some warnings.
…when Credential Exchange flag is enabled so the user can decide whether the export to a file or using the Credential Exchange flow.
|
No New Or Fixed Issues Found |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1227 +/- ##
==========================================
+ Coverage 89.47% 89.60% +0.12%
==========================================
Files 700 711 +11
Lines 44733 45137 +404
==========================================
+ Hits 40026 40445 +419
+ Misses 4707 4692 -15 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
🎨 What do you think about renaming the containing folder from Export to ExportSettings since these files are all using the ExportSettings prefix?
| SettingsListItem(Localizations.exportVaultToAFile) { | ||
| store.send(.exportToFileTapped) | ||
| } trailingContent: { | ||
| chevron | ||
| } | ||
| .accessibilityIdentifier("ExportVaultToAFileLabel") | ||
| .cornerRadius(10) | ||
|
|
||
| SettingsListItem(Localizations.exportVaultToAnotherApp) { | ||
| store.send(.exportToAppTapped) | ||
| } trailingContent: { | ||
| chevron | ||
| } | ||
| .accessibilityIdentifier("ExportVaultToAnotherAppLabel") | ||
| .cornerRadius(10) |
There was a problem hiding this comment.
⛏️ It looks like both of these items are displaying the bottom divider. Should that be removed? Alternatively, should both of these options be grouped together, similar to the main settings view?
There was a problem hiding this comment.
Nice catch the divider, I'll fix that! As to group them, in Figma they appear separated so I based the UI on that.
| /// A route to the export vault settings view. | ||
| case exportVault |
There was a problem hiding this comment.
⛏️ Slight preference to renaming this to exportSettings to match the prefix for the files associated with this route.
There was a problem hiding this comment.
I'll clarify the comment, as this route could go to the export settings view as well as to the export to file view depending on whether the CXP Export feature flag is enabled. That's why I left it as exportVault as it has a more generic interpretation.
|
Closing this to be replaced by a new PR #1245 to ease merging from |
🎟️ Tracking
PM-15634
📔 Objective
Add intermediate Export settings view so that the user can choose whether to export to a file or using the Credential Exchange flow.
Notes
The old "Export vault" view files were moved to another folder, so now we have this structure:
📸 Screenshots
Export.intermediate.view.mov
⏰ 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