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

Titanium should add view controllers as children #11651

Closed
akashivskyy opened this issue Apr 21, 2020 · 3 comments
Closed

Titanium should add view controllers as children #11651

akashivskyy opened this issue Apr 21, 2020 · 3 comments

Comments

@akashivskyy
Copy link

akashivskyy commented Apr 21, 2020

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
Copy link
Collaborator

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
Copy link
Contributor

@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
Copy link
Author

akashivskyy commented Nov 3, 2020

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.

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

No branches or pull requests

3 participants