-
Notifications
You must be signed in to change notification settings - Fork 388
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
Memory Leak /Retain Cycles resolved #215
base: master
Are you sure you want to change the base?
Conversation
@@ -19,14 +19,15 @@ final class GalleryPagingDataSource: NSObject, UIPageViewControllerDataSource { | |||
fileprivate var itemCount: Int { return itemsDataSource?.itemCount() ?? 0 } | |||
fileprivate unowned var scrubber: VideoScrubber | |||
|
|||
init(itemsDataSource: GalleryItemsDataSource, displacedViewsDataSource: GalleryDisplacedViewsDataSource?, scrubber: VideoScrubber, configuration: GalleryConfiguration) { | |||
init(itemsDataSource: GalleryItemsDataSource?, displacedViewsDataSource: GalleryDisplacedViewsDataSource?, scrubber: VideoScrubber, configuration: GalleryConfiguration) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not too sure about this optional at the point of initialisation. Can you tell me in what scenario do you envision this happening?
@@ -148,7 +148,7 @@ open class GalleryViewController: UIPageViewController, ItemControllerDelegate { | |||
} | |||
} | |||
|
|||
pagingDataSource = GalleryPagingDataSource(itemsDataSource: itemsDataSource, displacedViewsDataSource: displacedViewsDataSource, scrubber: scrubber, configuration: configuration) | |||
pagingDataSource = GalleryPagingDataSource(itemsDataSource:itemsDataSource, displacedViewsDataSource: displacedViewsDataSource, scrubber: scrubber, configuration: configuration) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
itemsDataSource:itemsDataSource
⬇️
itemsDataSource: itemsDataSource
@@ -464,7 +466,8 @@ open class GalleryViewController: UIPageViewController, ItemControllerDelegate { | |||
|
|||
open func page(toIndex index: Int) { | |||
|
|||
guard currentIndex != index && index >= 0 && index < self.itemsDataSource.itemCount() else { return } | |||
guard let source = self.itemsDataSource else {return} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{return}
🔽
{ return }
@@ -490,8 +493,8 @@ open class GalleryViewController: UIPageViewController, ItemControllerDelegate { | |||
func removePage(atIndex index: Int, completion: @escaping () -> Void) { | |||
|
|||
// If removing last item, go back, otherwise, go forward | |||
|
|||
let direction: UIPageViewController.NavigationDirection = index < self.itemsDataSource.itemCount() ? .forward : .reverse | |||
guard let source = self.itemsDataSource else {return} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
@@ -502,8 +505,8 @@ open class GalleryViewController: UIPageViewController, ItemControllerDelegate { | |||
} | |||
|
|||
open func reload(atIndex index: Int) { | |||
|
|||
guard index >= 0 && index < self.itemsDataSource.itemCount() else { return } | |||
guard let source = self.itemsDataSource else {return} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
For a little while I didn't realize
pagingDataSource
is also holding a reference to itemsDataSource...Making
itemsDelegate
anditemsDataSource
weak then passing the weak value toGalleryPagingDataSource
resolved my issues.Merge it with grain of salt....