52inc / Pulley

A library to imitate the iOS 10 Maps UI.
https://cocoapods.org/pods/Pulley
MIT License
2.02k stars 265 forks source link

Nullable Drawer #90

Closed mergesort closed 7 years ago

mergesort commented 7 years ago

I have a UI which is utilizing Pulley for displaying content from the bottom, but in a transient manner, similar to how you would use master/detail or modal view controller pattern. This is a bit different than the canonical Maps example where the drawer is on the screen forever, but still a pattern that's emerging in iOS apps. (As seen below)

Pocket Casts

I'm running into an issue where I change the drawerViewController by calling

self.pulleyViewController.setDrawerContentViewController(controller: myNewViewController, animated: false)

But the previous ViewController that was there stays resident in memory forever.

I think either one of two changes would be appropriate to address the problem, potentially both.

Option 1:

Option 2:

So far I'm really enjoying using your library, and I'd love to hear your feedback, and why this may or may not work.

Thanks a lot!

amyleecodes commented 7 years ago

But the previous ViewController that was there stays resident in memory forever.

Once you've replaced the drawer VC, Pulley no longer has any strong references to the previous drawer VC. This means that Pulley probably isn't what's causing your VC to not be deallocated. You may have a retain cycle from a block, or a non-weak delegate variable causing an issue. I'd recommend using Xcode's Memory Graph tool to see what is retaining a strong reference on your old drawer VC.

nil-ing out a variable isn't necessary for an object to be deallocated, if the variable is now pointing to a different object. For the 'current' drawer view controller, it is expected that it will remain in memory. Previous drawer controllers are fully released by Pulley, so you may have a retain cycle elsewhere in your code. Make sure all 'delegate' variables are weak and all blocks capture self as weak or unowned. If you need a lighter-weight drawer VC while 'closed', you can assign an empty UIViewController that implements the drawer delegate and only allows the 'closed' position.

The above behaviors that you've proposed have 3 issues from my point of view:

  1. It's a significantly breaking change for existing users of Pulley.
  2. I don't believe that's a good behavior, as it requires significantly more effort by the developer to use Pulley in it's 'intended' use case.
  3. It isn't necessary to solve your issue of the previous view controller not being deallocated.

If the memory graph shows that Pulley is responsible for maintaining a strong reference to your previous drawer view controller, please post a screenshot of the memory graph with 'link' selected and the right side bar opened so I can see the detail on what is holding the reference. However, I do consider this possibility very unlikely (when compared to retain cycles external to Pulley).