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.

Sign up for GitHub

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

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Memory Leak /Retain Cycles resolved #215

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
07a40e5
weak var delegate
Apr 4, 2019
67008b8
weak vars delegates
Apr 4, 2019
39dbb06
Update GalleryViewController.swift
ugenlik Apr 5, 2019
aaf243a
Update GalleryViewController.swift
ugenlik Apr 5, 2019
1bf2c86
Update GalleryViewController.swift
ugenlik Apr 5, 2019
0f63b18
Update GalleryViewController.swift
ugenlik Apr 5, 2019
7424776
Update GalleryPagingDataSource.swift
ugenlik Apr 5, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions ImageViewer/Source/GalleryPagingDataSource.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Copy link
Collaborator

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?


self.itemsDataSource = itemsDataSource
self.displacedViewsDataSource = displacedViewsDataSource
self.scrubber = scrubber
self.configuration = configuration

if itemsDataSource.itemCount() > 1 { // Potential carousel mode present in configuration only makes sense for more than 1 item
guard let source = itemsDataSource else {return}
if source.itemCount() > 1 { // Potential carousel mode present in configuration only makes sense for more than 1 item

for item in configuration {

Expand Down
23 changes: 13 additions & 10 deletions ImageViewer/Source/GalleryViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ open class GalleryViewController: UIPageViewController, ItemControllerDelegate {
fileprivate var initialPresentationDone = false

// DATASOURCE/DELEGATE
fileprivate let itemsDelegate: GalleryItemsDelegate?
fileprivate let itemsDataSource: GalleryItemsDataSource
fileprivate weak var itemsDelegate: GalleryItemsDelegate?
fileprivate weak var itemsDataSource: GalleryItemsDataSource?
fileprivate let pagingDataSource: GalleryPagingDataSource

// CONFIGURATION
Expand Down Expand Up @@ -69,7 +69,7 @@ open class GalleryViewController: UIPageViewController, ItemControllerDelegate {
@available(*, unavailable)
required public init?(coder: NSCoder) { fatalError() }

public init(startIndex: Int, itemsDataSource: GalleryItemsDataSource, itemsDelegate: GalleryItemsDelegate? = nil, displacedViewsDataSource: GalleryDisplacedViewsDataSource? = nil, configuration: GalleryConfiguration = []) {
public init(startIndex: Int, itemsDataSource: GalleryItemsDataSource? = nil, itemsDelegate: GalleryItemsDelegate? = nil, displacedViewsDataSource: GalleryDisplacedViewsDataSource? = nil, configuration: GalleryConfiguration = []) {

self.currentIndex = startIndex
self.itemsDelegate = itemsDelegate
Expand Down Expand Up @@ -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)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

itemsDataSource:itemsDataSource

⬇️

itemsDataSource: itemsDataSource


super.init(transitionStyle: UIPageViewController.TransitionStyle.scroll,
navigationOrientation: UIPageViewController.NavigationOrientation.horizontal,
Expand Down Expand Up @@ -440,8 +440,10 @@ open class GalleryViewController: UIPageViewController, ItemControllerDelegate {
//ThumbnailsimageBlock

@objc fileprivate func showThumbnails() {

guard let source = self.itemsDataSource else {return}

let thumbnailsController = ThumbnailsViewController(itemsDataSource: self.itemsDataSource)
let thumbnailsController = ThumbnailsViewController(itemsDataSource: source)

if let closeButton = seeAllCloseButton {
thumbnailsController.closeButton = closeButton
Expand All @@ -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}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{return}

🔽

{ return }

guard currentIndex != index && index >= 0 && index < source.itemCount() else { return }

let imageViewController = self.pagingDataSource.createItemController(index)
let direction: UIPageViewController.NavigationDirection = index > currentIndex ? .forward : .reverse
Expand All @@ -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}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

let direction: UIPageViewController.NavigationDirection = index < source.itemCount() ? .forward : .reverse

let newIndex = direction == .forward ? index : index - 1

Expand All @@ -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}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

guard index >= 0 && index < source.itemCount() else { return }

guard let firstVC = viewControllers?.first, let itemController = firstVC as? ItemController else { return }

Expand Down