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

Changing SizesWindow's rootViewController after initial setup doesn't remove the old child view #19

Closed
frankdilo opened this issue Oct 19, 2018 · 4 comments
Labels
bug Something isn't working
Projects

Comments

@frankdilo
Copy link
Collaborator

frankdilo commented Oct 19, 2018

Expected Behavior

Only ViewB should be on screen.

Actual Behavior

ViewA is on screen on top of ViewB.

Steps to Reproduce the Problem

  1. Set SizesWindow as app window in AppDelegate
  2. Set the root view controller of the window to ViewControllerA
  3. Later on in the app, change the root view controller of the window to ViewControllerB
  4. Observe that both ViewA and ViewB are on screen
@frankdilo
Copy link
Collaborator Author

I already spotted the problem and can provide a PR with a fix:

open class SizesWindow: UIWindow {
    
    public let sizesViewController = SizesViewController()
    
    override open var rootViewController: UIViewController? {
        get {
            return sizesViewController.containedController
        }
        set {
            guard let root = newValue, !root.isKind(of: SizesViewController.self) else {
                return
            }
            super.rootViewController = sizesViewController
            sizesViewController.contain(viewController: root)
        }
    }
}

In the setter, before adding another child view controller we should remove the existing one if it's there. I can submit a PR.

@marcosgriselli
Copy link
Owner

marcosgriselli commented Oct 19, 2018

Exactly. I think we need to modify the contain(viewController: _) method to handle this. I'd really appreciate a PR :).

@marcosgriselli marcosgriselli added the bug Something isn't working label Oct 19, 2018
@marcosgriselli marcosgriselli added this to To do in Sizes via automation Oct 19, 2018
frankdilo pushed a commit to frankdilo/Sizes that referenced this issue Oct 19, 2018
@frankdilo
Copy link
Collaborator Author

Done! 🙌

@marcosgriselli
Copy link
Owner

Closed in #20

Sizes automation moved this from To do to Done Oct 19, 2018
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
Sizes
  
Done
Development

No branches or pull requests

2 participants