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

Fix navigation controller root bug #74

Merged
merged 8 commits into from
Jan 13, 2021

Conversation

CJEkman
Copy link
Contributor

@CJEkman CJEkman commented Jan 8, 2021

This PR fixes a set of subtle problems experienced when a UINavigationController is presented in a modal context. The initial presentation wasn't performed immediately, and some kind of lifecycle issue caused input cursors for UITextField instances in the first view controller to be missing from the view hierarchy.

The issue could be solved by assigning an explicit root view controller, which lead to the discovery of the problem here in Presentation - likely introduced when creating the workaround for issue #40 in iOS 13+ without considering UINavigationController. This PR also fixes a minor issue found when setting up the dismiss action for these flows.

@CJEkman CJEkman added the bug Something isn't working label Jan 8, 2021
@nataliq-pp
Copy link
Contributor

Looks like the cancel button doesn't get correctly set up in the case of embedInNavigationController option.
Will be good to add a test case to the StylesAndOptions example project that covers presenting a view controller in an explicit instance of UINavigationController which is what the bug was about.

nataliq-pp
nataliq-pp previously approved these changes Jan 12, 2021
Copy link
Contributor

@nataliq-pp nataliq-pp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. This will be a patch I suppose. Feel free to bump the version in the same PR. We need to update issue 40 to include the navigation controller too.

@CJEkman
Copy link
Contributor Author

CJEkman commented Jan 12, 2021

@nataliq The version is bumped and the changelog has been updated. I also added info to issue #40
Please leave another review 🙂

@CJEkman CJEkman merged commit 6f05074 into master Jan 13, 2021
@CJEkman CJEkman deleted the fix-navigation-controller-root-bug branch January 13, 2021 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants