Titanium should add view controllers as children

akashivskyy commented 4 years ago

When adding windows to windows (e.g. using Titanium.UI.createNavigationWindow), Titanium ignores the rules of view controller containment on iOS and doesn't add view controllers as children.

The code responsible for calling the required methods has been commented out with a TODO: Revisit for almost 5 years (as of writing this issue):

https://github.com/appcelerator/titanium_mobile/blob/1afbbfc3f7fbc3f9f69580043d61ba4e75bf7127/iphone/TitaniumKit/TitaniumKit/Sources/API/TiWindowProxy.m#L150-L174

This causes issues in view-controller-backed modules (such as PSPDFKit), especially during view controller presentation. UIKit even logs a warning message about this:

Presenting view controllers on detached view controllers is discouraged ...

It's time for the revisit. 😉

Steps to reproduce

Create a plain window and a navigation window. Run the app and inspect the view hierarchy.

var window = Titanium.UI.createWindow({
    backgroundColor: "white",
})

var navigationWindow = Titanium.UI.createNavigationWindow({
    window: window,
})

navigationWindow.open()

Expected behavior

Every view controller in the hierarchy (except for TiRootViewController) has a correctly set parent view controller.

Actual behavior

UINavigationController managed by Titanium.UI.NavigationWindow is detached and has no parent view controller:

Environment

Titanium SDK: 9.0.1.GA Titanium CLI: 5.2.2 Appcelerator CLI: 8.0.0

hansemannn commented 4 years ago

Hi @akashivskyy ! @vijaysingh-axway added a pull request to fix this issue via #12233, would you mind reviewing it as well? Thank you! :)

vijaysingh-axway commented 4 years ago

@akashivskyy As Hans has said, I have created PR #12233 to fix the mentioned issue. It would be great if you can try to use it in your module and give feedback. Thanks!

akashivskyy commented 4 years ago

Hi @hansemannn and @vijaysingh-axway! I'm sorry for my late reply. I don't feel I'm proficient enough with Titanium SDK to leave a meaningful review there. Also, I'm not sure how to test this locally with my minimal setup. I'm no authority on Titanium SDK at my company, I'm just making sure our plugin builds every time we or you release a new version. From my point of view, as long as the expected behavior described in this issue (that every view controller has a parent) is there, it's good to go.

I'm very sorry I can't be of more help here.

tidev / titanium-sdk