Use split controller's top navigation controller to present toasts #5767
Conversation
Anderas
requested review from
a team and
pixlwave
and removed request for
a team
|
📱 Scan the QR code below to install the build for this PR. If you can't scan the QR code you can install the build via this link: https://i.diawi.com/EGi6WG |
| // Setup detail user indicator presenter | ||
| let presentingViewController = splitViewController.isCollapsed ? tabBarCoordinator.toPresentable() : detailNavigationController | ||
| detailUserIndicatorPresenter = UserIndicatorTypePresenter(presentingViewController: presentingViewController) |
There was a problem hiding this comment.
I wonder if this should be pulled out into its own method and also called from the 2 UISplitViewControllerDelegate methods too, to handle iPad layout changes?
SplitViewCoordinator.splitViewController(_:separateSecondaryFrom:)
SplitViewCoordinator.splitViewController(_:collapseSecondary:onto:)There was a problem hiding this comment.
This makes sense, although the fix I just pushed is not sufficient yet either, because currently each time the layout is changed, the presenter will be recreated, incl creating a new indicator queue, which is bad. Because this is a relative edge case (ie you need iPad + detail shown + spinner active + change of layout) I'd go with this for now and in separete PR I want to hide the presentingViewController behind some kind of factory interface that gets passed into the presenter. That way only the view controller is changed on demand rather than the entire indicator presenter
| @@ -15,6 +15,7 @@ | |||
| */ | |||
|
|
|||
| import Foundation | |||
| import MatrixSDK | |||
There was a problem hiding this comment.
Probably not needed, but Xcode 13 seems to like adding this 🤷♂️
Fixes #5752
The issue of missing spinner is actually a layout issue caused by split view controller allowing multiple nested navigation controllers. Previously when a toast was presented on a navigation controller, it was added on top of the navigation bar and anchored to the title, which could happen before the navigation controller itself was added to the view controller hierarchy. Once the navigation controller was properly added to the hierarchy, the parent navigation took control over the navigation bar and placed it on top of the spinner as well as invalidating any previously set constraints.
To solve for this issue the app needs to get hold of the ever-present top navigation controller, owned by the split view controller. On iPhones, this is the navigation controller that holds onto the detail navigation controller, whereas on iPads the detail navigation controller is the top navigation controller.
The whole point of this change is to expose navigation controller that is always in the view hierarchy, as opposed to a temporary one that may not have stable navigation bar.