PM-13066 - Add FAB on various screens #1028
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1028 +/- ##
==========================================
+ Coverage 89.22% 89.25% +0.03%
==========================================
Files 665 667 +2
Lines 41788 41924 +136
==========================================
+ Hits 37285 37421 +136
Misses 4503 4503 ☔ View full report in Codecov by Sentry. |
|
No New Or Fixed Issues Found |
There was a problem hiding this comment.
If there's a bunch of items in the vault list, it seems like this overlaps the last item. Should we add additional padding to the bottom of the list to account for the FAB?
Simulator.Screen.Recording.-.iPhone.15.Pro.-.2024-10-09.at.13.34.45.mp4
The FAB looks like it's used in a few different places. Do you know if there are other tickets for that, or should we do that as part of this? I think this includes VaultGroupView, VaultAutofillListView, SendListView, VaultItemSelectionView, and FoldersView (anywhere addToolbarItem is used).
There was a problem hiding this comment.
🤔 There are a couple of these screenshots that have the FAB that I'm wondering if they should, such as loading, or the empty state where there's an "Add Item" button front and center. Is this what we want, or will more conditional logic for this be in a future PR?
| @@ -68,6 +68,14 @@ class VaultListViewTests: BitwardenTestCase { | |||
| XCTAssertEqual(processor.dispatchedActions.last, .addItemPressed) | |||
| } | |||
|
|
|||
| /// Tapping the new login floating acrtion button dispatches the `.addItemPressed` action.` | |||
There was a problem hiding this comment.
| /// Tapping the new login floating acrtion button dispatches the `.addItemPressed` action.` | |
| /// Tapping the new login floating action button dispatches the `.addItemPressed` action.` |
Im about to make a commit to fix a lot of what Matt brought up and add the FAB to the other screens as well. I did confirm with design that the FAB should be on the loading screen and on the figma designs they show the FAB being on the empty state as well. |
🎟️ Tracking
PM-13066
📔 Objective
VaultListView,VaultGroupView,VaultAutofillListView,SendListView,VaultItemSelectionView, andFoldersView.CircleButtonStyleand aFloatingActionButton.Viewto create anaddandeditfloating action buttons which were used in the views above to prevent duplication.plus.pdfis actually a little off centered. I am going to be replacing it in the icons update ticket so I left it alone in this PR but you may notice the "+" looks a bit weird on the FAB and thats why.** Going to confirm with design about the empty states... After seeing some snapshots with large font, it just doesn't make sense to have two buttons overlapping that do the same thing. Keeping as is for now since figma matches but might update in this Pr or a future one.
Update from design: Currently working on a design update and approved keeping the FAB on the empty state as-is until coming to a decision.
📸 Screenshots
see snapshots for all of the screenshot changes but heres an example.
⏰ 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