Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix keyWindow.rootViewController nullability on iOS #1231

Merged

Conversation

Ponyets
Copy link
Contributor

@Ponyets Ponyets commented Mar 26, 2019

  • This is a small change
  • This change has been discussed in issue #<?> and the solution has been agreed upon with maintainers.

Description:

Issue: #1230

Some iOS developer use [UIApplicaiton sharedApplication].keyWindow.rootViewController as a convenient way to get a VC instance. Surely, this isn't suggested, but it's widely used anyway.
When the app is driven by Detox, this call would return null. It's because DTXTouchVisualizerWindow has a very high windowLevel and doesn't has any ViewController.

This PR fixes this issue by avoid making DTXTouchVisualizerWindow the keyWindow.

@Ponyets Ponyets requested a review from LeoNatan as a code owner March 26, 2019 07:23
@LeoNatan
Copy link
Contributor

I don't think this is the right solution. It can be often incorrect when several UIWindow instances exist (more complex apps). The PR assumes that there is only one user window, but that could be a very wrong assumption.

I don't think it's the right move to break Detox for complex apps in favor of apps that cannot be bothered to use iOS APIs correctly.

If I could suggest something else, but I don't have time to check. If you could test with that instead, I will be happy to include it. UIWindow has a private method called - (BOOL)_canBecomeKeyWindow;

So if we define this method for the touch indicator window, like so:

- (BOOL)_canBecomeKeyWindow
{
	return NO;
}

It could solve the problem people are seeing?

detox/ios/Detox/DetoxAppDelegateProxy.m Outdated Show resolved Hide resolved
@Ponyets Ponyets force-pushed the DTXTouchVisualizerWindow_rootViewController branch from 74e4bde to 54ce816 Compare March 29, 2019 02:38
@LeoNatan
Copy link
Contributor

Thanks for checking! I will accept it.

1 similar comment
@LeoNatan
Copy link
Contributor

Thanks for checking! I will accept it.

@LeoNatan LeoNatan merged commit 725df09 into wix:master Mar 29, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Apr 1, 2019
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants