Call presented UIViewController lifecycle methods from UINavigationController

[ephemer]

Fixes #341

Type of change: Bug fix

Motivation (current vs expected behavior)

As reported in #341, UINavigationController does not work as expected. This is because viewWillAppear (etc.) are never called there. I have added a call to viewWillAppear and viewDidAppear, but there are probably other lifecycle methods that are currently not being called.

Please check if the PR fulfills these requirements

• [x] Self-review: I am confident this is the simplest and clearest way to achieve the expected behaviour
• [x] There are no dependencies on other PRs or I have linked dependencies through Zenhub
• [x] The commit messages are clean and understandable
• [ ] Tests for the changes have been added (for bug fixes / features)

[rikner]

Hey @ephemer ! I reproduced the issue this way (AppDelegate.swift):

let viewController = ViewController()
let navigationController = UINavigationController(rootViewController: viewController)
window?.rootViewController = navigationController

The PR seems to fix the issue, but I realized that the BackButton does not seem to work. Tapping it has no effect. Using the hardware button (or actually the screen gesture in my case) works.

https://user-images.githubusercontent.com/2410252/141165039-abe0a397-7ffe-4aea-afa6-3855764a983f.mp4

[ephemer]

Hey @rikner, thanks a lot for testing this!

I disabled the back button specifically in this PR because it should not do anything when the UINavigationController is at root position. If we dismiss the rootViewController the app will be "dead" forever. If the hardware back button still "works" we need to fix that too :)

[rikner]

Hey @rikner, thanks a lot for testing this!

I disabled the back button specifically in this PR because it should not do anything when the UINavigationController is at root position. If we dismiss the rootViewController the app will be "dead" forever. If the hardware back button still "works" we need to fix that too :)

Ah ok I see. Apparently I have no idea what the expected behaviour is. Are you looking into disabling the Hardware Button?

[ephemer]

@rikner the hardware back button should work in general, but the "back button" on a UINavigationController should not do anything if there is only one view on the navigation stack and that UINavigationController is the application's rootViewController

[rikner]

@rikner the hardware back button should work in general, but the "back button" on a UINavigationController should not do anything if there is only one view on the navigation stack and that UINavigationController is the application's rootViewController

Ok, yeah I get that. But then shouldn't we make sure that in this special case the hardware back button does not send the App into the void like seen in the video? Or would that be an issue for a seperate PR?

[ephemer]

Yep:

If the hardware back button still "works" we need to fix that too :)

[ephemer]

@rikner ~@michaelknoch~ would appreciate a(nother) review on this

[michaelknoch]

oops, CI doesn't like the change @ephemer 😄 https://app.circleci.com/pipelines/github/flowkey/UIKit-cross-platform/151/workflows/202a524e-10dc-4ce7-9862-0da1201f167d/jobs/1453

[ephemer]

@michaelknoch UIViewControllerTests.testMemoryLeakInNavigationController seems very suspicious ;) I will check it out thanks

[ephemer]

ah, it's because ~navigationController!.dismiss(animated: false) will now only work if it's not the root view controller, which is desired behaviour.~ UIApplication.shared does not exist in the test context. I will fix it