From 02e24acae5f747e44dc3855967b087f524b381cf Mon Sep 17 00:00:00 2001 From: Ed Rutter Date: Fri, 9 Nov 2018 11:16:20 +0000 Subject: [PATCH 1/6] Fix issue #20 where view was not removed when removing view controller --- Example/Tests/ScrollingStackViewTests.swift | 22 +++++++++++++++++++ .../ScrollingStackViewController.swift | 1 + 2 files changed, 23 insertions(+) diff --git a/Example/Tests/ScrollingStackViewTests.swift b/Example/Tests/ScrollingStackViewTests.swift index 1ed70f5..428d289 100644 --- a/Example/Tests/ScrollingStackViewTests.swift +++ b/Example/Tests/ScrollingStackViewTests.swift @@ -59,6 +59,28 @@ class ScrollingStackViewTests: XCTestCase { vc.remove(viewController: child2) XCTAssert(vc.stackView.arrangedSubviews.count == 1) } + + func testRemoveViewControllers() { + let vc = Factory.createScrollingStackViewController(window: window) + let height = vc.view.frame.height + + let child1 = Factory.createStubViewController(height: height) + let child2 = Factory.createStubViewController(height: height) + let edgeInset = UIEdgeInsets(top: 10, left: 10, bottom: 10, right: 10) + + vc.add(viewController: child1) + vc.show(viewController: child1) + vc.show(viewController: child2, insertIfNeeded: (position: .end, insets: edgeInset)) + XCTAssert(vc.stackView.arrangedSubviews.count == 2) + + vc.remove(viewController: child1) + XCTAssert(vc.stackView.arrangedSubviews.count == 1) + XCTAssert(child1.view.superview == nil) + + vc.remove(viewController: child2) + XCTAssert(vc.stackView.arrangedSubviews.count == 0) + XCTAssert(child2.view.superview == nil) + } func testComplexScrolling() { diff --git a/ScrollingStackViewController/Classes/ScrollingStackViewController.swift b/ScrollingStackViewController/Classes/ScrollingStackViewController.swift index 9f341ae..b664052 100644 --- a/ScrollingStackViewController/Classes/ScrollingStackViewController.swift +++ b/ScrollingStackViewController/Classes/ScrollingStackViewController.swift @@ -208,6 +208,7 @@ open class ScrollingStackViewController: UIViewController { open func remove(viewController: UIViewController) { guard let arrangedView = arrangedView(for: viewController) else { return } stackView.removeArrangedSubview(arrangedView) + viewController.view.removeFromSuperview() viewController.willMove(toParentViewController: nil) viewController.removeFromParentViewController() From eed903f8c0520194480fd8043e9047cd154a6b33 Mon Sep 17 00:00:00 2001 From: Ed Rutter Date: Fri, 9 Nov 2018 11:16:43 +0000 Subject: [PATCH 2/6] Increment patch version [4.0.1] --- Example/Podfile.lock | 4 ++-- .../Local Podspecs/ScrollingStackViewController.podspec.json | 4 ++-- Example/Pods/Manifest.lock | 4 ++-- .../ScrollingStackViewController/Info.plist | 2 +- ScrollingStackViewController.podspec | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Example/Podfile.lock b/Example/Podfile.lock index ee5b243..f5313a6 100644 --- a/Example/Podfile.lock +++ b/Example/Podfile.lock @@ -1,5 +1,5 @@ PODS: - - ScrollingStackViewController (4.0.0) + - ScrollingStackViewController (4.0.1) DEPENDENCIES: - ScrollingStackViewController (from `../`) @@ -9,7 +9,7 @@ EXTERNAL SOURCES: :path: "../" SPEC CHECKSUMS: - ScrollingStackViewController: c87977311c6e8b0c4d32fcf9530dd60250d9c470 + ScrollingStackViewController: ae954d22a57508749a58c5ffb01231a8dba2817d PODFILE CHECKSUM: 1107023c97aaa6562f2eb38595e37b1267bba2b1 diff --git a/Example/Pods/Local Podspecs/ScrollingStackViewController.podspec.json b/Example/Pods/Local Podspecs/ScrollingStackViewController.podspec.json index ce5f175..e632305 100644 --- a/Example/Pods/Local Podspecs/ScrollingStackViewController.podspec.json +++ b/Example/Pods/Local Podspecs/ScrollingStackViewController.podspec.json @@ -1,6 +1,6 @@ { "name": "ScrollingStackViewController", - "version": "4.0.0", + "version": "4.0.1", "summary": "A view controller that uses root views of child view controllers as views in a UIStackView.", "description": "This view controller is more suitable than an UITableViewController when creating a list of segments that are dynamically behaving, but are well defined and bound in number. The delegation pattern that the data source of an UITableViewController is best suited for situation when there is an unbounded number of cells, but in many cases is an overkill and becomes a burden. Also, UITableViewCells are not controllers, but sometimes it makes sense to properly partition the responsibility of the segments, not just over the view layer. Using ScrollingStackViewController you can have a bunch of view controllers, each of them encapsulating their own responsibilities.", "homepage": "https://github.com/justeat/ScrollingStackViewController", @@ -15,7 +15,7 @@ }, "source": { "git": "https://github.com/justeat/ScrollingStackViewController.git", - "tag": "4.0.0" + "tag": "4.0.1" }, "platforms": { "ios": "9.0" diff --git a/Example/Pods/Manifest.lock b/Example/Pods/Manifest.lock index ee5b243..f5313a6 100644 --- a/Example/Pods/Manifest.lock +++ b/Example/Pods/Manifest.lock @@ -1,5 +1,5 @@ PODS: - - ScrollingStackViewController (4.0.0) + - ScrollingStackViewController (4.0.1) DEPENDENCIES: - ScrollingStackViewController (from `../`) @@ -9,7 +9,7 @@ EXTERNAL SOURCES: :path: "../" SPEC CHECKSUMS: - ScrollingStackViewController: c87977311c6e8b0c4d32fcf9530dd60250d9c470 + ScrollingStackViewController: ae954d22a57508749a58c5ffb01231a8dba2817d PODFILE CHECKSUM: 1107023c97aaa6562f2eb38595e37b1267bba2b1 diff --git a/Example/Pods/Target Support Files/ScrollingStackViewController/Info.plist b/Example/Pods/Target Support Files/ScrollingStackViewController/Info.plist index 3424ca6..b672cd7 100644 --- a/Example/Pods/Target Support Files/ScrollingStackViewController/Info.plist +++ b/Example/Pods/Target Support Files/ScrollingStackViewController/Info.plist @@ -15,7 +15,7 @@ CFBundlePackageType FMWK CFBundleShortVersionString - 4.0.0 + 4.0.1 CFBundleSignature ???? CFBundleVersion diff --git a/ScrollingStackViewController.podspec b/ScrollingStackViewController.podspec index b098102..f47fd54 100644 --- a/ScrollingStackViewController.podspec +++ b/ScrollingStackViewController.podspec @@ -8,7 +8,7 @@ Pod::Spec.new do |s| s.name = 'ScrollingStackViewController' - s.version = '4.0.0' + s.version = '4.0.1' s.summary = 'A view controller that uses root views of child view controllers as views in a UIStackView.' s.description = <<-DESC From b5d4fc03272fb57331ff6b412680b3c18ce3ecae Mon Sep 17 00:00:00 2001 From: Steven Warren Date: Fri, 9 Nov 2018 12:28:51 +0000 Subject: [PATCH 3/6] padding view now also removed from superview if present. removeFromSuperview also removes the view from the arrangedSubviews array, so that call is no longer needed. --- .../Classes/ScrollingStackViewController.swift | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ScrollingStackViewController/Classes/ScrollingStackViewController.swift b/ScrollingStackViewController/Classes/ScrollingStackViewController.swift index b664052..922ee43 100644 --- a/ScrollingStackViewController/Classes/ScrollingStackViewController.swift +++ b/ScrollingStackViewController/Classes/ScrollingStackViewController.swift @@ -207,8 +207,7 @@ open class ScrollingStackViewController: UIViewController { open func remove(viewController: UIViewController) { guard let arrangedView = arrangedView(for: viewController) else { return } - stackView.removeArrangedSubview(arrangedView) - viewController.view.removeFromSuperview() + arrangedView.removeFromSuperview() viewController.willMove(toParentViewController: nil) viewController.removeFromParentViewController() From 4e365de1e9e3433d928058f43507133ec6d3a123 Mon Sep 17 00:00:00 2001 From: Steven Warren Date: Fri, 9 Nov 2018 14:21:45 +0000 Subject: [PATCH 4/6] fixed unit test. --- Example/Tests/ScrollingStackViewTests.swift | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/Example/Tests/ScrollingStackViewTests.swift b/Example/Tests/ScrollingStackViewTests.swift index 428d289..b5f9973 100644 --- a/Example/Tests/ScrollingStackViewTests.swift +++ b/Example/Tests/ScrollingStackViewTests.swift @@ -64,22 +64,25 @@ class ScrollingStackViewTests: XCTestCase { let vc = Factory.createScrollingStackViewController(window: window) let height = vc.view.frame.height - let child1 = Factory.createStubViewController(height: height) - let child2 = Factory.createStubViewController(height: height) + let arrangedVC = Factory.createStubViewController(height: height) + let containedVC = Factory.createStubViewController(height: height) let edgeInset = UIEdgeInsets(top: 10, left: 10, bottom: 10, right: 10) - vc.add(viewController: child1) - vc.show(viewController: child1) - vc.show(viewController: child2, insertIfNeeded: (position: .end, insets: edgeInset)) + vc.add(viewController: arrangedVC) + vc.show(viewController: arrangedVC) + vc.show(viewController: containedVC, insertIfNeeded: (position: .end, insets: edgeInset)) XCTAssert(vc.stackView.arrangedSubviews.count == 2) - vc.remove(viewController: child1) + let firstArrangedView = arrangedVC.view! + let secondArrangedView = containedVC.view.superview! + + vc.remove(viewController: arrangedVC) XCTAssert(vc.stackView.arrangedSubviews.count == 1) - XCTAssert(child1.view.superview == nil) + XCTAssert(firstArrangedView.superview == nil) - vc.remove(viewController: child2) + vc.remove(viewController: containedVC) XCTAssert(vc.stackView.arrangedSubviews.count == 0) - XCTAssert(child2.view.superview == nil) + XCTAssert(secondArrangedView.superview == nil) } func testComplexScrolling() { From 96616eca4609772e976c546dbb4803e643e75aa8 Mon Sep 17 00:00:00 2001 From: Steven Warren Date: Fri, 9 Nov 2018 15:24:15 +0000 Subject: [PATCH 5/6] Handle the case where there is still a strong reference to the view controller --- .../Classes/ScrollingStackViewController.swift | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ScrollingStackViewController/Classes/ScrollingStackViewController.swift b/ScrollingStackViewController/Classes/ScrollingStackViewController.swift index 922ee43..5eff52d 100644 --- a/ScrollingStackViewController/Classes/ScrollingStackViewController.swift +++ b/ScrollingStackViewController/Classes/ScrollingStackViewController.swift @@ -208,6 +208,9 @@ open class ScrollingStackViewController: UIViewController { open func remove(viewController: UIViewController) { guard let arrangedView = arrangedView(for: viewController) else { return } arrangedView.removeFromSuperview() + if arrangedView != viewController.view { + viewController.view.removeFromSuperview() + } viewController.willMove(toParentViewController: nil) viewController.removeFromParentViewController() From 769f12360b5a6aa35da6b8a9a82a3691c7f39354 Mon Sep 17 00:00:00 2001 From: Steven Warren Date: Mon, 19 Nov 2018 10:46:46 +0000 Subject: [PATCH 6/6] Updated Unit Test to demonstrate both VC superviews are nil. --- Example/Tests/ScrollingStackViewTests.swift | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/Example/Tests/ScrollingStackViewTests.swift b/Example/Tests/ScrollingStackViewTests.swift index b5f9973..a8feded 100644 --- a/Example/Tests/ScrollingStackViewTests.swift +++ b/Example/Tests/ScrollingStackViewTests.swift @@ -73,16 +73,13 @@ class ScrollingStackViewTests: XCTestCase { vc.show(viewController: containedVC, insertIfNeeded: (position: .end, insets: edgeInset)) XCTAssert(vc.stackView.arrangedSubviews.count == 2) - let firstArrangedView = arrangedVC.view! - let secondArrangedView = containedVC.view.superview! - vc.remove(viewController: arrangedVC) XCTAssert(vc.stackView.arrangedSubviews.count == 1) - XCTAssert(firstArrangedView.superview == nil) + XCTAssert(arrangedVC.view.superview == nil) vc.remove(viewController: containedVC) XCTAssert(vc.stackView.arrangedSubviews.count == 0) - XCTAssert(secondArrangedView.superview == nil) + XCTAssert(containedVC.view.superview == nil) } func testComplexScrolling() {