Skip to content
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

Intercept Back Button Call in UINavigationController? #17

Open
Ben-G opened this issue Mar 13, 2016 · 25 comments
Open

Intercept Back Button Call in UINavigationController? #17

Ben-G opened this issue Mar 13, 2016 · 25 comments

Comments

@Ben-G
Copy link
Member

Ben-G commented Mar 13, 2016

Problem: When using ReSwiftRouter we want every view controller change to be reflected by a route update. This means we need to be notified when UIKit's container view controllers trigger VCs to be presented or hidden.

For UITabBarController we can implement delegate methods to intercept view controller changes and allow the router to change view controllers instead of the controller handling the changes directly.

For the back button of UINavigationController there's no comparable solution. Right now I'm using a workaround that I'm not very happy with: https://github.com/ReSwift/GitHubBrowserExample/blob/master/SwiftFlowGitHubBrowser/ViewControllers/RepositoryDetailViewController.swift#L37-L41

When a VC disappears, I check if the current route is still the one of the current VC. If that's the case, we know that the VC was dismissed without the route being updated; in response I update the route manually.

Any suggestions on how to intercept back button taps more directly are welcome. Ideally this would not involve replacing bar button items, or any other larger modifications of the UINavigationController.

@jschmid
Copy link
Contributor

jschmid commented Mar 14, 2016

There is also didMoveToParentViewController:, but this also means that every view controller must implement this method :/

@yammada
Copy link

yammada commented Mar 16, 2016

I've encountered this exact same issue in my own small scale experiments with unidirectional data flow architecture. I realized that the problem is not only intercepting taps from the back button, but also the standard edge swipe gestures, so any sort of button trickery doesn't seem to work.

One possibility that's similar to your solution, but at least doesn't require participation from every child VC, is using - navigationController:didShowViewController:animated:. It would still involve a manual update to the route by filtering, since it doesn't actually between a pop or a push, but it works.

Another downside with taking over the delegate though is that it's also responsible for custom transitions, so splitting that out becomes another potential headache if you want maintain standard UINC behavior.

@ivanmisuno
Copy link

I ran into the same problem, and also trying to use UINavigationControllerDelegate.
For every view controller I push into navigation stack, I set associated route:

    // In Root Routable:
    // MARK: - Routable
    func pushRouteSegment(routeElementIdentifier: RouteElementIdentifier,
        animated: Bool,
        completionHandler: RoutingCompletionHandler) -> Routable
    {
        // ...
        let viewController = self.viewControllerForPushingWithSegment(segment)
        self.navigationController.pushViewController_(viewController, animated: animated, completion: completionHandler)
    }

    func viewControllerForPushingWithSegment(segment: RoutableSegment) -> ViewControllerPresentationInterop
    {
        let viewController = self.viewControllerFactory.viewControllerForRouteSegment(segment)
        self.setViewControllerCurrentNavigationState(viewController)
        return viewController
    }
    func setViewControllerCurrentNavigationState(viewController: ViewControllerPresentationInterop)
    {
        if let navigationController = viewController as? NavigationControllerPresentationInterop {
            if let topViewController = navigationController.topViewController_ {
                self.setViewControllerCurrentNavigationState(topViewController)
            }
        }
        if var viewControllerWithRoute = viewController as? HasNavigationRoute {
            // set associated route
            viewControllerWithRoute.navigationRoute = self.navigationState.route
        }
    }
    // ...
    // Root Routable has to subscribe to NavigationState updates
    func newState(state: NavigationState) {
        self.navigationState_ = state
    }

Then, in the root NavigationController's delegate I check that the route stored in a view controller being presented matches the expected route and correct the latter if necessary:

    // UINavigationControllerDelegate
    func navigationController(navigationController: UINavigationController, didShowViewController viewController: UIViewController, animated: Bool) {
        // ...
        self.verifyRouteIsSynced(nc: navigationController, c: viewController, animated: animated)
    }
    // ...
    func verifyRouteIsSynced(nc nc: NavigationControllerPresentationInterop, c: ViewControllerPresentationInterop, animated: Bool)
    {
        if let viewControllerWithRoute = c as? HasNavigationRoute {
            if let viewControllerRoute = viewControllerWithRoute.navigationRoute {
                if self.navigationState.route != viewControllerRoute {
                    debugPrint("correcting route for \(c) from (\(self.navigationState.route)) to (\(viewControllerRoute))")
                    let store = self.getStoreBlock()
                    store.dispatch(SetRouteAction(viewControllerRoute, animated: false))
                }
            }
        }
    }

This is somewhat dirty:

  • UIViewController has to be extended to support storing associated route;
  • Root routable has to subscribe to state updates to track current (expected) NavigationState, and needs to dispatch actions to the store, which somewhat breaks initial concept.

The real problem, however, is that SetRouteAction() dispatched in verifyRouteIsSynced tries to initiate changes in the UI (via Routable callbacks), that have already been made by user pushing "Back" button. For this I need only to update store's NavigationState and Routable structure, while skipping actual changes to UI.

@Ben-G
Copy link
Member Author

Ben-G commented Jun 20, 2016

Stumbled across isMovingFromParentViewController method on UIViewController today; based on the documentation it might be another viable candidate.

@Ben-G
Copy link
Member Author

Ben-G commented Jul 16, 2016

Another relevant read that covers the same issue using the coordinator pattern: http://irace.me/navigation-coordinators

@Ben-G
Copy link
Member Author

Ben-G commented Jul 23, 2016

Another potential approach: implementing the UINavigationBarDelegate which has a method that can prevent popping view controllers. Details here: http://stackoverflow.com/a/21462109/1046430

@dodikk
Copy link

dodikk commented Dec 8, 2016

So what is the client code workaround or approach for dealing with UINavigationController ?
How about adding some hints to the README.md file?

I've faced the issue when the NavigationState.route does not get updated when the user taps back button of UINavigationController. So my navigation actions end up dispatching to a wrong Routable sub-class.

$ cat Cartfile.resolved 
github "ReSwift/ReSwift" "3.0.0"
github "ReSwift/ReSwiftRouter" "0.5.1"

Here is some supplementary code

    // Root

class RootRoutable : Routable
{
    fileprivate func routableToLoginWizardVC() -> Routable
    {
        // instantiate VC from storyboard
        let nextVC = PKStoryboardFactory.loadLoginWizardVC()

        // add navigation and present explicitly
        let navigation = UINavigationController(rootViewController: nextVC)
        self.window.rootViewController = navigation
        
        return LoginWizardRoutable(withActiveVC: nextVC)
    }

...
}
    @IBAction func onLoginEmailButtonTapped(_ sender: UIButton?)
    {
        let currentRoute = store.state.navigationState.route

        // when back button is tapped the last element of currentRoute is still there
        // seems like I have to hard code the routes
        let nextRoute = currentRoute + [loginRoute]
        
        let routeToNextScreenAction = ReSwiftRouter.SetRouteAction(nextRoute)
        store.dispatch(routeToNextScreenAction)
    }
po routeToNextScreenAction
▿ SetRouteAction
  ▿ route : 3 elements
    - 0 : "login_or_register_selector"
    - 1 : "login"
    - 2 : "login"
  - animated : true

@dodikk
Copy link

dodikk commented Dec 8, 2016

Sorry for spamming. I've found the "invoke route manually in viewWillDisappear" workaround in a github client example.

Still, isn't this approach worth mentioning in a readme?
At least until a better solution is implemented.

@rpassis
Copy link
Member

rpassis commented Dec 22, 2016

@Ben-G thanks for the reference article on #17 (comment)

I've been able to implemented something similar and it seems to be working well.

For anyone trying to find a more elegant solution to this, here's a quick overview of what I've done. Feel free to ping me if you need any other details.

  1. Create a new protocol RoutableIdentifiable that inherits from Routable. Change your routables to adopt and conform to this protocol.
protocol RoutableIdentifiable: Routable {
    var routeIdentifier: RouteElementIdentifier { get set }
}
  1. Create a NavigationController proxy class that bridges to your UINavigationController
class NavigationController: UIViewController, UINavigationControllerDelegate {

    let rootViewController: UIViewController
    private let childNavigationController: UINavigationController

    required init(rootViewController: UIViewController) {
        self.rootViewController = rootViewController
        self.childNavigationController =
            UINavigationController(rootViewController: rootViewController)
        super.init(nibName: nil, bundle: nil)
        self.childNavigationController.delegate = self
    }

    required init?(coder aDecoder: NSCoder) {
        fatalError("init(coder:) has not been implemented")
    }

    override func viewDidLoad() {
        super.viewDidLoad()
        configureChildViewController()
    }

    private func configureChildViewController() {
        addChildViewController(childNavigationController)
        view.addSubview(childNavigationController.view)
        childNavigationController.didMove(toParentViewController: self)
        // Setup your constraints here if required
    }

    // Expose any UINavigationController methods as required (i.e. push, pop, etc)
    ...

    // MARK: NavController delegate
    // We use the navigationController delegate method for animation controllers
    // as it gives us a reference to the source and destination view controllers. 
    // This method is called after the navigation controller state has been updated
    // so the viewControllers array will reflect what the navigation stack is 
    // supposed to be. From here, we can simply check if the viewControllers array contains 
    // the source view controller and then ensure that our navigation state is correct

    func navigationController(_ navigationController: UINavigationController, animationControllerFor
        operation: UINavigationControllerOperation, from fromVC: UIViewController, to toVC:
        UIViewController) -> UIViewControllerAnimatedTransitioning? {
        if navigationController.viewControllers.contains(fromVC) == false {
            // The source view controller was popped, let's check if the state needs updating
            guard let vc = fromVC as? RoutableIdentifiable else { return nil }
            var newRoute = mainStore.state.navigationState.route
            let lastRouteElementIdentifier = newRoute.last
            if vc.routeIdentifier.rawValue == lastRouteElementIdentifier {
                _ = newRoute.popLast()
                mainStore.dispatch(ReSwiftRouter.SetRouteAction(newRoute))
            }
            return nil
        }
        return nil
    }

@Ben-G
Copy link
Member Author

Ben-G commented Feb 21, 2017

@rpassis Sorry for the super late response on this. I've been on a long vacation and pretty swamped ever since I'm back. But your solution is actually really good compared to everything we have so far (actually we've implemented something similar for the router we're using at PlanGrid).

Once I've freed up a little bit of time it would nice to document this solution or maybe even incorporate it into the library somehow.

@dagio
Copy link
Member

dagio commented Mar 8, 2017

@rpassis can you conform that with your solution, if you do not dispatch SetRouteAction the view controller will be poped anyway ? I mean in that case it seems the state is lagging behind the UI.

@rpassis
Copy link
Member

rpassis commented Mar 8, 2017

@dagio the view controller will be popped irrespective of setting the route. The idea is that we are ensuring the route state is in sync with the UI when triggering actions that are hard to capture (ie. back button press or swipe right).

@dagio
Copy link
Member

dagio commented May 31, 2017

@rpassis did you manage to keep the native swipe gesture of the navigation view controller in order to pop the current view controller ?
When implementing func navigationController(_ navigationController: UINavigationController, animationControllerFor operation: UINavigationControllerOperation, from fromVC: UIViewController, to toVC: UIViewController) -> UIViewControllerAnimatedTransitioning? in our delegate, then the gesture is not available anymore

At the moment we are using func navigationController(_ navigationController: UINavigationController, didShow viewController: UIViewController, animated: Bool). It does the job. The native swipe gesture work. The back button too. The NavigationState get updated.

import UIKit
import ReSwift
import ReSwiftRouter

/**
 This delegate disable the back button in UIKit and dispatch a routing action instead
 */

public class RoutingNavigationControllerDelegate<S: StateType>: NSObject, UINavigationControllerDelegate {

    private let store: Store<S>
    private let navigationStateSelector: (S) -> NavigationState
    private var currentViewController: UIViewController?

    public init(store: Store<S>, navigationStateSelector: @escaping (S) -> NavigationState) {
        self.store = store
        self.navigationStateSelector = navigationStateSelector
    }

    public func navigationController(_ navigationController: UINavigationController, didShow viewController: UIViewController, animated: Bool) {

        guard let fromVC = currentViewController else {
            self.currentViewController = viewController

            return
        }

        if navigationController.viewControllers.contains(fromVC) == false {
            // The source view controller was popped, let's check if the state needs updating

            // If the source view controller does not conform to RoutableIdentifiable
            // then do nothing
            guard let sourceVC = fromVC as? RoutableIdentifiable else {
                NSLog("The source view controller does not conform to RoutableIdentifiable. Did you forget to conform your view controller to the protocol ?")

                return
            }

            // Here we mean by current route the one that is in the navigation state.
            // At this moment the user has taped on the back button, however the navigation
            // state has not changed. Therefore the current route should still be the same
            let currentRoute = navigationStateSelector(store.state).route
            let currentRouteElementIdentifier = currentRoute.last

            // If the source view controller is the one of the current route
            if sourceVC.routeIdentifier == currentRouteElementIdentifier {
                // Create destinationRoute as copy of currentRoute so we can make some change on it
                var destinationRoute = currentRoute
                _ = destinationRoute.popLast()
                store.dispatch(ReSwiftRouter.SetRouteAction(destinationRoute))

                store.dispatch(DeleteRouteSpecificData(route: currentRoute))
            }
        }

        self.currentViewController = viewController
    }
}

@ambientlight
Copy link

@dagio, @Ben-G: I have created #74 for it. Take a look and let me know your thoughts.

@ghost
Copy link

ghost commented Aug 21, 2017

Have a look at RoutableUIKit, it contains a navigation controller that supports routing. Please contribute with more routable UIKit components!

@richy486
Copy link
Member

richy486 commented Aug 29, 2017

I'm trying using to counter to check if the route should be updated. Checking lastViewControllersCount against the current view controllers count minus 1 to see if we are popping and checking that the lastRouteCount is the same as the current route count to see if the Routers have already updated the route.

class NavigationController: UINavigationController {
    var lastViewControllersCount: Int = 0
    var lastRouteCount: Int = 0
    //...
}
extension NavigationController: UINavigationControllerDelegate {
    func navigationController(_ navigationController: UINavigationController, 
                              willShow viewController: UIViewController, animated: Bool) {
        
        if lastViewControllersCount == navigationController.viewControllers.count + 1 &&
            lastRouteCount == mainStore.observable.value.navigationState.route.count {
            var mutableRoute = mainStore.observable.value.navigationState.route
            _ = mutableRoute.popLast()
            mainStore.dispatch(ReactiveReSwiftRouter.SetRouteAction(mutableRoute))
        }
        lastViewControllersCount = navigationController.viewControllers.count
        lastRouteCount = mainStore.observable.value.navigationState.route.count
    }
}

This doesn't store a RouteElementIdentifier on the View Controllers which I don't really want them to know about. The counters might to be simplistic but appears to work with edge swiping and the standard back button in my simple tests. Testing using the standard nav bar back button as well as an action to popRouteSegment.

Note I'm using my ReactiveReSwiftRouter fork of ReSwiftRouter hence the mainStore.observable.value.

@ghost
Copy link

ghost commented Aug 30, 2017

@richy486 is this working when swiping? navigationController(navigationController:willShow viewController:animated:) doesn't mean the view was popped (or pushed) if being called as a result of a swipe.

@richy486
Copy link
Member

richy486 commented Aug 31, 2017

@hlineholm-flir you are right, when you cancel an edge swipe it still pops off the router.

@richy486
Copy link
Member

richy486 commented Sep 4, 2017

Updated our NavigationController subclass to handle edge swiping with a custom back button and capturing edge swipes via the UIViewControllerTransitionCoordinator

class NavigationController: UINavigationController {

    override func viewDidLoad() {
        super.viewDidLoad()
        
        // Do any additional setup after loading the view.
        self.delegate = self
    }
    
    override func pushViewController(_ viewController: UIViewController, animated: Bool) {
        
        if viewControllers.count > 0 {
            let backButton = UIBarButtonItem(title: "<", style: .plain, target: self, action: #selector(tapBack))
            viewController.navigationItem.leftBarButtonItem = backButton
        }
        
        super.pushViewController(viewController, animated: animated)
        
        // This is to enable the edge swipe with a custom back button on the `leftBarButtonItem`, it must be called after `pushViewController`
        viewController.navigationController?.interactivePopGestureRecognizer?.delegate = self
        viewController.navigationController?.interactivePopGestureRecognizer?.isEnabled = true
    }
    
    // Must be public to expose to Objective-c
    func tapBack() {
        forcePopRoute()
    }
    
    fileprivate func forcePopRoute() {
        var mutableRoute = mainStore.observable.value.navigationState.route
        _ = mutableRoute.popLast()
        mainStore.dispatch(ReactiveReSwiftRouter.SetRouteAction(mutableRoute))
    }
}

extension NavigationController: UIGestureRecognizerDelegate {
    func gestureRecognizer(_ gestureRecognizer: UIGestureRecognizer, shouldBeRequiredToFailBy otherGestureRecognizer: UIGestureRecognizer) -> Bool {
        return true
    }
}

extension NavigationController: UINavigationControllerDelegate {
    
    func navigationController(_ navigationController: UINavigationController,
                              willShow viewController: UIViewController, animated: Bool) {
        
        // Handle edge swipe back, need to check that we haven't canceled the gesture.
        // We setup the coordinator when showing the view controller because 
        // the `transitionCoordinator` is not setup when pushing it
        if let coordinator = topViewController?.transitionCoordinator {
            
            coordinator.notifyWhenInteractionChanges({ [unowned self] (context) in
                if !context.isCancelled {
                    self.forcePopRoute()
                }
            })
        }
    }
}

@ghost
Copy link

ghost commented Sep 5, 2017

@richy486 I've done something similar in RoutableUIKit but instead of using

navigationController(navigationController:willShow viewController:animated:)

I use

navigationController(navigationController:didShow viewController:animated:)

as it doesn't fire if the swipe is cancelled. The RoutableNavigationController takes care of deciding if it is performing a pop or not and then calls routableNavigationController(_ rnvc:, didPop vc:animated:) on the routingDelegate.

@Ben-G
Copy link
Member Author

Ben-G commented Sep 16, 2017

I should have thought about this earlier, but it's probably best to do something similar to what React Native does to intercept back button calls. Unfortunately the solution is rather involved: https://github.com/facebook/react-native/blob/1e8f3b11027fe0a7514b4fc97d0798d3c64bc895/React/Views/RCTNavigator.m#L67-L148

But would be great if someone would port this at some point!

@marcocpt
Copy link

marcocpt commented Oct 14, 2017

@Ben-G use this

extension UINavigationController: UINavigationBarDelegate  {

  public func navigationBar(_ navigationBar: UINavigationBar, didPop item: UINavigationItem) {
      let newRoute = Array(store.state.navigationState.route.dropLast())
      let routeAction = ReSwiftRouter.SetRouteAction(newRoute)
      store.dispatch(routeAction)
  }
}

@MojtabaHs
Copy link

@marcocpt nice and simple solution. but what about 'ReSwiftRouterStuck' ?

@hlineholm
Copy link
Contributor

hlineholm commented Oct 26, 2017

@Ben-G I've released 1.2.0 of RoutableUIKit and it is also quite involved but I cannot see how we would solve this in a simpler way. And I don't think this is an issue that should be solved here as it's domain isn't specific to ReSwift-Router.

@richy486 @marcocpt do you handle popping of the navigation controller from any of your routables? If so how, do you avoid popping your route again in your delegate methods? For an example:

A (working)

  1. Back button / Swipe gesture occurs
  2. Your delegate picks it up and dispatches a SetRouteAction with a popped route
  3. Your routable handles the pop

B (not working)

  1. Someone dispatches a SetRouteAction which causes ReSwift-Router to fire popRouteSegment
  2. Your routables handles the pop by popping the navigation controller
  3. Your delegate picks it up and dispatches a SetRouteAction with a popped route
  4. Step 2 and 3 are repeated until there is only one view controller left in the stack

@MojtabaHs ReSwiftRouterStuck will occur if you don't call the completion handler in your routables. It isn't handled in any methods of the navigation controller, navigation controller delegate or the navigation bar.

@MojtabaHs
Copy link

@hlineholm I solved completion issue with this little extension:

extension UINavigationController {
    
    func pushViewController(
        _ viewController: UIViewController,
        animated: Bool,
        completion: (()->())? = nil)
    {
        pushViewController(viewController, animated: animated)
        
        guard animated, let coordinator = transitionCoordinator else {
            completion?()
            return
        }
        
        coordinator.animate(alongsideTransition: nil) { _ in completion?() }
    }
    
    func popViewController(animated: Bool, completion: (() -> ())? = nil) {
        popViewController(animated: animated)
        
        guard animated, let coordinator = transitionCoordinator else {
            completion?()
            return
        }
        
        coordinator.animate(alongsideTransition: nil) { _ in completion?() }
    }
    
}

@marcocpt I think calling "setRouterAction" when UI is already presented "newRoute" is not well enough. Maybe using "shouldPop" method instead of "didPop" is better solution when your return false and dispatch "setRouter" yourself. But in this case, navigation controller behave oddly. For example it shows back button in root view controller 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests