Bugfix FXIOS-8364 [v123] Add confirmation dialog #18581
Conversation
| accessibilityIdentifier: AccessibilityIdentifiers.TabTray.deleteCancelButton) | ||
| controller.popoverPresentationController?.barButtonItem = deleteButton | ||
| present(controller, animated: true, completion: nil) | ||
| } |
There was a problem hiding this comment.
Just curious, we don't have any existing Alert Controller designs for destructive actions? The UX of this to me doesn't emphasize the fact that this is to delete a bunch of data (for example, using red in the text of the confirmation)
There was a problem hiding this comment.
Improving the UX is mostly out of scope for this project so I think it's fine to keep it the same for now but this is a very good point, would you mind creating a bug for it and we can tackle it through our regular triage process since it' needs UX input.
There was a problem hiding this comment.
Created a bug here https://mozilla-hub.atlassian.net/browse/FXIOS-8399
|
Works well, but seems like the undo toast for undo-ing the delete all does not update the tab count until you enter the tab tray again and add a tab there. Probably fine to make a ticket for it in the epic and address it after? Simulator.Screen.Recording.-.iPhone.15.Plus.-.2024-02-06.at.15.38.53.mp4 |
| @@ -134,14 +138,25 @@ struct TabTrayState: ScreenState, Equatable { | |||
| // Only update the nomal tab count if the tabs being refreshed are not private | |||
| let tabModel = context.tabDisplayModel | |||
| let isPrivate = tabModel.tabs.first?.isPrivate ?? false | |||
| let tabCount = tabModel.normalTabsCount | |||
| let tabCount = isPrivate ? state.normalTabsCount : tabModel.normalTabsCount | |||
There was a problem hiding this comment.
what's the difference between state.normalTabsCount and tabModel.normalTabsCount?
There was a problem hiding this comment.
It's possible tabModel.normalTabsCount might not be calculated in certain cases if it's updating the private window so just safer to keep the value the same as it's current value since the private screen cannot impact this number for the normal screen.
|
Added a ticket for the undo tab count issue #18612 |
OrlaM
force-pushed
the
om/FXIOS-8364-confirmation
branch
from
84be214 to
ee27544
Compare
Client.app: Coverage: 31.85
Generated by 🚫 Danger Swift against ee27544 |
|
@Mergifyio backport release/v123 |
✅ Backports have been created
|
* Add confirmation dialog * Fix swiftlint (cherry picked from commit e7dae22)
* Add confirmation dialog * Fix swiftlint (cherry picked from commit e7dae22) Co-authored-by: OrlaM <[email protected]>
📜 Tickets
Jira ticket
Github issue
💡 Description
Adds the confirmation dialog to the close all tabs action
📝 Checklist
You have to check all boxes before merging