-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Refactor FXIOS-7596 [v121] remove legacy store #17224
Changes from 10 commits
be99cc1
85819ca
0ac40e5
15b3d61
1b7bfd1
a9dd8ae
e89fbec
d8a63a4
c6d0356
33d9686
cea3eb1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -375,8 +375,10 @@ class LegacyGridTabViewController: UIViewController, | |
} | ||
|
||
func dismissTabTray() { | ||
self.navigationController?.dismiss(animated: true, completion: nil) | ||
TelemetryWrapper.recordEvent(category: .action, method: .close, object: .tabTray) | ||
DispatchQueue.main.async { | ||
self.navigationController?.dismiss(animated: true, completion: nil) | ||
TelemetryWrapper.recordEvent(category: .action, method: .close, object: .tabTray) | ||
} | ||
Comment on lines
+378
to
+381
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not an ideal solution, but since the select tab that happens prior to this is now an async event it is causing crashes on the home page when the view is dismissed since the tab data is not fully resolved after deleting tabs. I'm comfortable putting this solution here because the legacy tab tray is going away soon anyway. |
||
} | ||
|
||
/// Handles close tab by clicking on close button or swipe gesture | ||
|
@@ -652,6 +654,7 @@ extension LegacyGridTabViewController { | |
case .addTab: | ||
didTapToolbarAddTab() | ||
case .deleteTab: | ||
print("") | ||
didTapToolbarDelete(sender) | ||
} | ||
notificationCenter.post(name: .TabDataUpdated) | ||
|
This file was deleted.
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.
I'm trying to understand why we need to add a call to the
overlayManager
in this particular PR to remove the legacy store? I guess this relates to the tests that were failing right?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.
Yup, for some reason it no longer triggers overlay mode at the right time. There is likely a superfluous call at the wrong time somewhere else but I was afraid to start messing with it because it's spagetified into other code paths.