Fix height not correctly resized when remove child

[ThomasLeblond]

Issue

Hello,

First, thanks for your framework we have complex ViewController in our app and this is really useful for us!

I have an issue when I remove a view from the parent view controller. purgeRemovedViews() & scrollView.purgeWrapperViews() are not called so obviously the height is not correctly resize and a blank space is still visible.

If you want to test the issue, please find this sample.

Test no resize height.zip

Fix

The fix is pretty simple because all methods already exists. I just create a removeChild function (as we have for the addChild) to call this private methods purgeRemovedViews() & scrollView.purgeWrapperViews().

[codecov-commenter]

Codecov Report

Merging #193 into master will increase coverage by 1.33%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #193      +/-   ##
==========================================
+ Coverage   80.11%   81.45%   +1.33%     
==========================================
  Files          11       11              
  Lines         835      836       +1     
==========================================
+ Hits          669      681      +12     
+ Misses        166      155      -11     

Flag	Coverage Δ
#iOS	84.00% <ø> (+41.00%)	⬆️
#macOS	80.14% <100.00%> (+0.02%)	⬆️
#tvOS	43.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
Sources/macOS/Classes/FamilyViewController.swift	78.81% <100.00%> (+0.10%)	⬆️
Sources/Shared/BinarySearch.swift	79.41% <0.00%> (+5.88%)	⬆️
Sources/Shared/FamilySpaceManager.swift	83.87% <0.00%> (+29.03%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e98ff55...7b88f70. Read the comment docs.

[zenangst]

Hey @ThomasLeblond, thank you very much for the PR… I'll verify and merge this later this evening (hopefully). Keep rocking!

[zenangst]

Also, I need to update the tests to make them pass, it seems that there is some humbug happening on Travis.

[zenangst]

@ThomasLeblond would it be possible to observe changes to children and respond with trying to purge children when the hierarchy changes?

[zenangst]

We could achieve this by calling scrollView.purgeWrapperViews() in FamilyViewController.purgeRemovedViews()

public func purgeRemovedViews() -> Self {
  for (controller, container) in registry where controller.parent == nil {
    _removeChild(controller)
    if container.view.superview is FamilyWrapperView {
      container.view.superview?.removeFromSuperview()
    }

    container.view.removeFromSuperview()
    container.observer.invalidate()
    registry.removeValue(forKey: controller)
  }

  scrollView.purgeWrapperViews()

  return self
}

That would do proper cleanup and leave the public API unchanged, what do you think about that @ThomasLeblond.

I tried it with the sample code and it works as intended, at least as far as I can see.
Also, cheers to the sample project, that helped a lot to both understand the problem and come up with some suggestions.

Cheers!

[zenangst]

@ThomasLeblond If you rebase of master then I think Travis will show green on this PR :)

[ThomasLeblond]

@zenangst good point, if we can do it smoothly this is better option.

Updated & rebase done!

[zenangst]

@ThomasLeblond fantastic, LGTM!