TabsController - Missing logic

[markst]

As outlined in #935 and provided fix in #936.

Here is a PR again which provides the appropriate logic for adding the destination view controller to the child view controller stack & calls the appropriate appearance transitions in the correct order.

[markst]

@seubseub please can you test this PR as you mentioned you also had the issue where by the child view controllers do not belong to parent.

[daniel-jonathan]

I must be missing something here. All seems to work and I updated Motion. Thanks for double checking this.

[markst]

@DanielDahan my advice is add all this logic to your tab view controllers:

override open func viewWillAppear(_ animated: Bool) {
    super.viewWillAppear(animated)
    print("\(self) viewWillAppear - Parent: \(self.parent)")
}

override open func viewDidAppear(_ animated: Bool) {
    super.viewDidAppear(animated)
    print("\(self) viewDidAppear - Parent: \(self.parent)")
}

override open func viewWillDisappear(_ animated: Bool) {
    super.viewWillDisappear(animated)
    print("\(self) viewWillDisappear - Parent: \(self.parent)")
}

override open func viewDidDisappear(_ animated: Bool) {
    super.viewDidDisappear(animated)
    print("\(self) viewDidDisappear - Parent: \(self.parent)")
}

[daniel-jonathan]

I’ll double check :)

[markst]

without this PR notice how the view lifecycle calls are performed way too many times & has no parent:

<TabsController.GreenViewController: 0x7feb16507230> viewWillAppear - Parent: nil
<TabsController.GreenViewController: 0x7feb16507230> viewDidAppear - Parent: nil
<TabsController.GreenViewController: 0x7feb16507230> viewWillDisappear - Parent: nil
<TabsController.CyanViewController: 0x7feb165069d0> viewWillDisappear - Parent: nil
<TabsController.GreenViewController: 0x7feb16507230> viewWillAppear - Parent: nil
<TabsController.GreenViewController: 0x7feb16507230> viewDidAppear - Parent: nil
<TabsController.CyanViewController: 0x7feb165069d0> viewDidDisappear - Parent: nil

vs. with this PR:

<TabsController.CyanViewController: 0x7f91eac04dd0> viewWillDisappear - Parent: Optional(<TabsController.AppTabsController: 0x7f91eac079b0>)
<TabsController.GreenViewController: 0x7f91eac05630> viewWillAppear - Parent: nil
<TabsController.CyanViewController: 0x7f91eac04dd0> viewDidDisappear - Parent: nil
<TabsController.GreenViewController: 0x7f91eac05630> viewDidAppear - Parent: Optional(<TabsController.AppTabsController: 0x7f91eac079b0>)

[markst]

Without the tvc.beginAppearanceTransition() the view controller lifecycle viewWillAppear() gets called upon adding subview to container: container.addSubview(v.view)

To overcome this perhaps the container.addSubview(v.view) could be removed from prepare(viewController: UIViewController?, in container: UIView) and leave Motion responsible for performing the appearance transition calls as you've implemented here: CosmicMind/Motion@b9a2576

[daniel-jonathan]

What version are you using?

[markst]

43237ca

[markst]

@DanielDahan or perhaps rather TransitionController should be responsible for adding the child view controller to it's parent & performing the balanced calls to beginAppearanceTransition().

[daniel-jonathan]

That's your commit. What version are you testing that Material fails on?

[daniel-jonathan]

Well, I put the logic in Motion, which is the actual transition library. So I need to see the issue, I am testing your lifecycle functions in the sample.

[markst]

@DanielDahan which is good. but as the screenshot demonstrated container.addSubview(v.view) is triggering viewWillAppear() before appropriate.

Perhaps consider using:

override open var shouldAutomaticallyForwardAppearanceMethods: Bool {
    return false
}

As this prevents this said behaviour & viewWillAppear() is triggered by Motion

https://developer.apple.com/library/content/featuredarticles/ViewControllerPGforiPhoneOS/ImplementingaContainerViewController.html

[markst]

☝🏾 yup that last commit combined with latest version of Motion (CosmicMind/Motion@b9a2576) produces the desired behaviour

[daniel-jonathan]

Please try this commit 68308ad

[daniel-jonathan]

I found an issue with that last commit, but please still do try it. The issue is in other TransitionControllers.

[daniel-jonathan]

I will need to look this through some more, but your suggestion is on the right track. I have to confirm this doesn't break other controllers and that all cases are handled. Thank you! I will get back to this tonight or tomorrow morning :)

[markst]

@DanielDahan yeah nice. that's commit is spot on. works exactly as expected. Thanks!

Important to note a release update to Motion is required, to include the appearance transition updates ( CosmicMind/Motion@b9a2576 )

[markst]

Only thing I can spot with those changes is the appearance transitions are not being called on the initial root view controller.

Perhaps the following needs to take place when setting the initial root view controller (without transition)

self.rootViewController.beginAppearanceTransition(true, animated: false)
self.rootViewController.endAppearanceTransition()

or perhaps shouldAutomaticallyForwardAppearanceMethods should be toggled prior to transitioning.

[daniel-jonathan]

Please find a comprehensive update in Material 2.12.7. Thank you for working on this with me :)