-
Notifications
You must be signed in to change notification settings - Fork 53
PM-17854: Add vault item type selection menu to FAB #1351
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1351 +/- ##
==========================================
+ Coverage 89.64% 89.66% +0.02%
==========================================
Files 754 755 +1
Lines 47459 47519 +60
==========================================
+ Hits 42544 42608 +64
+ Misses 4915 4911 -4 ☔ View full report in Codecov by Sentry. |
Great job, no security vulnerabilities found in this Pull Request |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just one small ⛏️
/// - Parameters: | ||
/// - addItem: The action to perform when a new cipher item type is tapped in the menu. | ||
/// - addFolder: The action to perform when the new folder button is tapped in the menu. | ||
/// - Returns: A `FloatingActionMenu` configured for adding a vault item for folder. | ||
/// | ||
func addVaultItemFloatingActionMenu( | ||
hidden: Bool = false, | ||
addItem: @escaping (CipherType) -> Void, | ||
addFolder: (() -> Void)? = nil | ||
) -> some View { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
⛏️ Missing hidden
parameter docs.
var floatingActionButtonType: FloatingActionButtonType? { | ||
switch group { | ||
case .card, .identity, .login, .secureNote: | ||
return .button | ||
case .collection, .folder, .noFolder: | ||
return .menu | ||
case .sshKey, .totp, .trash: | ||
return nil | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎨 Could we add a unit test for floatingActionButtonType
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, added.
🎟️ Tracking
PM-17854
📔 Objective
This moves the vault item type selection menu from the vault add/edit screen and integrates it with the FAB in the vault list.
📸 Screenshots
Simulator.Screen.Recording.-.iPhone.16.Pro.-.2025-02-13.at.13.47.27.mp4
⏰ 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