diff --git a/LayoutKit.xcodeproj/project.pbxproj b/LayoutKit.xcodeproj/project.pbxproj index 443b862d..ece45649 100644 --- a/LayoutKit.xcodeproj/project.pbxproj +++ b/LayoutKit.xcodeproj/project.pbxproj @@ -201,6 +201,12 @@ 0BD5F82C1DB43F9B00108688 /* ButtonLayoutTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0BD5F82B1DB43F9B00108688 /* ButtonLayoutTests.swift */; }; 0BDDF95B1E25ACCE008B0A6F /* ReloadableViewTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0BDDF95A1E25ACCE008B0A6F /* ReloadableViewTests.swift */; }; 0BDDF95C1E25ACCE008B0A6F /* ReloadableViewTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0BDDF95A1E25ACCE008B0A6F /* ReloadableViewTests.swift */; }; + 3F387DFE1FD6D9210045475D /* ViewRecyclerViewStorage.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3F387DFD1FD6D9210045475D /* ViewRecyclerViewStorage.swift */; }; + 3F387DFF1FD6D9210045475D /* ViewRecyclerViewStorage.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3F387DFD1FD6D9210045475D /* ViewRecyclerViewStorage.swift */; }; + 3F387E001FD6D9210045475D /* ViewRecyclerViewStorage.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3F387DFD1FD6D9210045475D /* ViewRecyclerViewStorage.swift */; }; + 3F387E021FD6D9D20045475D /* ViewRecyclerViewStorageTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3F387E011FD6D9D20045475D /* ViewRecyclerViewStorageTests.swift */; }; + 3F387E031FD6D9D20045475D /* ViewRecyclerViewStorageTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3F387E011FD6D9D20045475D /* ViewRecyclerViewStorageTests.swift */; }; + 3F387E041FD6D9D20045475D /* ViewRecyclerViewStorageTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3F387E011FD6D9D20045475D /* ViewRecyclerViewStorageTests.swift */; }; 4468A31D1E46460B00341D07 /* NSAttributedStringExtension.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4468A31C1E46460B00341D07 /* NSAttributedStringExtension.swift */; }; 4468A31E1E464A3900341D07 /* NSAttributedStringExtension.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4468A31C1E46460B00341D07 /* NSAttributedStringExtension.swift */; }; 448CEC0F1E4E0CB500F8AD9E /* TextViewDefaultFont.swift in Sources */ = {isa = PBXBuildFile; fileRef = 448CEC0E1E4E0CB500F8AD9E /* TextViewDefaultFont.swift */; }; @@ -391,6 +397,8 @@ 0BD5F8281DB43B4500108688 /* ButtonLayout.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ButtonLayout.swift; sourceTree = ""; }; 0BD5F82B1DB43F9B00108688 /* ButtonLayoutTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ButtonLayoutTests.swift; sourceTree = ""; }; 0BDDF95A1E25ACCE008B0A6F /* ReloadableViewTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ReloadableViewTests.swift; sourceTree = ""; }; + 3F387DFD1FD6D9210045475D /* ViewRecyclerViewStorage.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ViewRecyclerViewStorage.swift; sourceTree = ""; }; + 3F387E011FD6D9D20045475D /* ViewRecyclerViewStorageTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ViewRecyclerViewStorageTests.swift; sourceTree = ""; }; 4468A31C1E46460B00341D07 /* NSAttributedStringExtension.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = NSAttributedStringExtension.swift; sourceTree = ""; }; 448CEC0E1E4E0CB500F8AD9E /* TextViewDefaultFont.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = TextViewDefaultFont.swift; sourceTree = ""; }; 44F968141E425F5D00392763 /* TextViewLayout.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = TextViewLayout.swift; sourceTree = ""; }; @@ -559,6 +567,7 @@ 0BD42BDB1DB5EAAD00E04AA3 /* Text.swift */, 0BCB75EA1D8724800065E02A /* UIKitSupport.swift */, 0BCB75EB1D8724800065E02A /* ViewRecycler.swift */, + 3F387DFD1FD6D9210045475D /* ViewRecyclerViewStorage.swift */, 0BCB75EC1D8724800065E02A /* Views */, ); name = LayoutKit; @@ -598,6 +607,7 @@ 44F968161E42639500392763 /* TextViewLayoutTests.swift */, 0BCB76671D8725310065E02A /* UIFontExtension.swift */, 0BCB76681D8725310065E02A /* ViewRecyclerTests.swift */, + 3F387E011FD6D9D20045475D /* ViewRecyclerViewStorageTests.swift */, ); path = LayoutKitTests; sourceTree = ""; @@ -1039,6 +1049,7 @@ 0BCB760C1D8724800065E02A /* LayoutAdapterCollectionView.swift in Sources */, 0BD5F8291DB43B4500108688 /* ButtonLayout.swift in Sources */, 0BCB76001D8724800065E02A /* LayoutMeasurement.swift in Sources */, + 3F387DFE1FD6D9210045475D /* ViewRecyclerViewStorage.swift in Sources */, 0BCB76021D8724800065E02A /* InsetLayout.swift in Sources */, 0BCB76061D8724800065E02A /* AxisFlexibility.swift in Sources */, 448CEC0F1E4E0CB500F8AD9E /* TextViewDefaultFont.swift in Sources */, @@ -1086,6 +1097,7 @@ 0B2D09381D872F75007E487C /* ViewRecyclerTests.swift in Sources */, 0B2D09341D872F75007E487C /* StackViewTests.swift in Sources */, 0B2D09281D872F75007E487C /* LabelLayoutTests.swift in Sources */, + 3F387E021FD6D9D20045475D /* ViewRecyclerViewStorageTests.swift in Sources */, 0B2D09231D872F75007E487C /* AlignmentTests.swift in Sources */, 0B2D09261D872F75007E487C /* DensityAssertions.swift in Sources */, 0BD5F82C1DB43F9B00108688 /* ButtonLayoutTests.swift in Sources */, @@ -1118,6 +1130,7 @@ 0BCB76301D8724C70065E02A /* LayoutMeasurement.swift in Sources */, 0B193BBA1D888B6300FCA22D /* IndexSetExtension.swift in Sources */, 0BCB76371D8724CA0065E02A /* InsetLayout.swift in Sources */, + 3F387DFF1FD6D9210045475D /* ViewRecyclerViewStorage.swift in Sources */, 0BCB76401D8724CF0065E02A /* AxisFlexibility.swift in Sources */, 0BCB762C1D8724C70065E02A /* CGSizeExtension.swift in Sources */, 0BCB76361D8724CA0065E02A /* BaseLayout.swift in Sources */, @@ -1153,6 +1166,7 @@ isa = PBXSourcesBuildPhase; buildActionMask = 2147483647; files = ( + 3F387E031FD6D9D20045475D /* ViewRecyclerViewStorageTests.swift in Sources */, 0B765F311DC135B8000BF1FD /* CGFloatExtensionTests.swift in Sources */, 0B2D09421D872F75007E487C /* ReloadableViewLayoutAdapterCollectionViewTests.swift in Sources */, 0B2D09441D872F75007E487C /* ReloadableViewLayoutAdapterTestCase.swift in Sources */, @@ -1191,6 +1205,7 @@ 0BCB761B1D8724C10065E02A /* Animation.swift in Sources */, 0BCB76471D8724D00065E02A /* AxisSize.swift in Sources */, 0BCB76351D8724C70065E02A /* LayoutMeasurement.swift in Sources */, + 3F387E001FD6D9210045475D /* ViewRecyclerViewStorage.swift in Sources */, 0BCB763C1D8724CB0065E02A /* InsetLayout.swift in Sources */, 0B193BBB1D888B6400FCA22D /* IndexSetExtension.swift in Sources */, 0B193BBD1D888B6D00FCA22D /* CollectionExtension.swift in Sources */, @@ -1214,6 +1229,7 @@ isa = PBXSourcesBuildPhase; buildActionMask = 2147483647; files = ( + 3F387E041FD6D9D20045475D /* ViewRecyclerViewStorageTests.swift in Sources */, 0B2D095E1D872F76007E487C /* StackLayoutSpacingTests.swift in Sources */, 0B2D09641D872F76007E487C /* ViewRecyclerTests.swift in Sources */, 0B2D094F1D872F76007E487C /* AlignmentTests.swift in Sources */, diff --git a/LayoutKitTests/LayoutArrangementTests.swift b/LayoutKitTests/LayoutArrangementTests.swift index 66b42aee..24944aed 100644 --- a/LayoutKitTests/LayoutArrangementTests.swift +++ b/LayoutKitTests/LayoutArrangementTests.swift @@ -53,6 +53,50 @@ class LayoutArrangementTests: XCTestCase { XCTAssertEqual(container.subviews[2].frame, CGRect(x: 0, y: 30, width: 5, height: 30)) } + func testViewReuseCircularReferenceDisconnection() { + + let forceViewConfig: (View) -> Void = { _ in } + + let layoutsForReusedView = StackLayout( + axis: .vertical, + viewReuseGroup: "A", + sublayouts: [ + SizeLayout( + size: .zero, + viewReuseGroup: "B", + config: forceViewConfig + ) + ], + config: forceViewConfig + ) + + let newLayouts = StackLayout( + axis: .vertical, + viewReuseGroup: "B", + sublayouts: [ + SizeLayout( + size: .zero, + viewReuseGroup: "A", + sublayout: SizeLayout( + size: .zero, + viewReuseGroup: "A", + config: forceViewConfig + ), + config: forceViewConfig + ) + ], + config: forceViewConfig + ) + + let container = View() + layoutsForReusedView.arrangement().makeViews(in: container) + newLayouts.arrangement().makeViews(in: container) + + XCTAssertEqual(container.subviews.count, 1) + XCTAssertEqual(container.subviews.first?.subviews.count, 1) + XCTAssertEqual(container.subviews.first?.subviews.first?.subviews.count, 1) + } + func testAnimation() { let forceViewConfig: (View) -> Void = { _ in } var redSquare: View? = nil diff --git a/LayoutKitTests/ViewRecyclerTests.swift b/LayoutKitTests/ViewRecyclerTests.swift index 5f4e4541..a1931b1b 100644 --- a/LayoutKitTests/ViewRecyclerTests.swift +++ b/LayoutKitTests/ViewRecyclerTests.swift @@ -19,7 +19,7 @@ class ViewRecyclerTests: XCTestCase { let recycler = ViewRecycler(rootView: root) let expectedView = View() - let v: View? = recycler.makeOrRecycleView(havingViewReuseId: nil, viewProvider: { + let v: View? = recycler.makeOrRecycleView(havingViewReuseId: nil, orViewReuseGroup: nil, viewProvider: { return expectedView }) XCTAssertEqual(v, expectedView) @@ -36,7 +36,7 @@ class ViewRecyclerTests: XCTestCase { let recycler = ViewRecycler(rootView: root) let expectedView = View() - let v: View? = recycler.makeOrRecycleView(havingViewReuseId: nil, viewProvider: { + let v: View? = recycler.makeOrRecycleView(havingViewReuseId: nil, orViewReuseGroup: nil, viewProvider: { return expectedView }) XCTAssertEqual(v, expectedView) @@ -51,7 +51,7 @@ class ViewRecyclerTests: XCTestCase { root.addSubview(one) let recycler = ViewRecycler(rootView: root) - let v: View? = recycler.makeOrRecycleView(havingViewReuseId: "1", viewProvider: { + let v: View? = recycler.makeOrRecycleView(havingViewReuseId: "1", orViewReuseGroup: nil, viewProvider: { XCTFail("view should have been recycled") return View() }) @@ -67,7 +67,7 @@ class ViewRecyclerTests: XCTestCase { root.addSubview(one) let recycler = ViewRecycler(rootView: root) - let v: View? = recycler.makeOrRecycleView(havingViewReuseId: nil, viewProvider: { + let v: View? = recycler.makeOrRecycleView(havingViewReuseId: nil, orViewReuseGroup: nil, viewProvider: { return one }) XCTAssertEqual(v, one) @@ -82,7 +82,7 @@ class ViewRecyclerTests: XCTestCase { root.addSubview(one) let recycler = ViewRecycler(rootView: root) - let v: View? = recycler.makeOrRecycleView(havingViewReuseId: nil, viewProvider: { + let v: View? = recycler.makeOrRecycleView(havingViewReuseId: nil, orViewReuseGroup: nil, viewProvider: { return one }) XCTAssertEqual(v, one) @@ -91,6 +91,22 @@ class ViewRecyclerTests: XCTestCase { XCTAssertNotNil(one.superview) } + func testViewProviderViewCreationShouldSetViewIdentifiers() { + let root = View() + let oldView = View() + root.addSubview(oldView) + + let newView = View() + + let recycler = ViewRecycler(rootView: root) + let v: View? = recycler.makeOrRecycleView(havingViewReuseId: "2", orViewReuseGroup: "group2", viewProvider: { + return newView + }) + XCTAssertEqual(v, newView) + XCTAssertEqual(newView.viewReuseId, "2") + XCTAssertEqual(newView.viewReuseGroup, "group2") + } + /// Test for safe subview-purge in composite view e.g. UIButton. /// - SeeAlso: https://github.com/linkedin/LayoutKit/pull/85 #if os(iOS) || os(tvOS) @@ -104,7 +120,7 @@ class ViewRecyclerTests: XCTestCase { XCTAssertEqual(button.subviews.count, 1, "UIButton should have 1 subview because `title` is set") let recycler = ViewRecycler(rootView: root) - let v: View? = recycler.makeOrRecycleView(havingViewReuseId: "1", viewProvider: { + let v: View? = recycler.makeOrRecycleView(havingViewReuseId: "1", orViewReuseGroup: nil, viewProvider: { XCTFail("button should have been recycled") return View() }) @@ -116,6 +132,23 @@ class ViewRecyclerTests: XCTestCase { XCTAssertEqual(button.subviews.count, 1, "UIButton's subviews should not be removed by `recycler`") } #endif + + func testRecycledFromViewGroup() { + let root = View() + let one = View(viewReuseId: "1", viewReuseGroup: "view") + root.addSubview(one) + + let recycler = ViewRecycler(rootView: root) + let v: View? = recycler.makeOrRecycleView(havingViewReuseId: "2", orViewReuseGroup: "view", viewProvider: { + XCTFail("view should have been recycled") + return View() + }) + XCTAssertEqual(v, one) + + recycler.purgeViews() + XCTAssertNotNil(one.superview) + } + } extension View { @@ -123,4 +156,10 @@ extension View { self.init(frame: .zero) self.viewReuseId = viewReuseId } + + convenience init(viewReuseId: String?, viewReuseGroup: String?) { + self.init(frame: .zero) + self.viewReuseId = viewReuseId + self.viewReuseGroup = viewReuseGroup + } } diff --git a/LayoutKitTests/ViewRecyclerViewStorageTests.swift b/LayoutKitTests/ViewRecyclerViewStorageTests.swift new file mode 100644 index 00000000..a8efebc6 --- /dev/null +++ b/LayoutKitTests/ViewRecyclerViewStorageTests.swift @@ -0,0 +1,87 @@ +// Copyright 2016 LinkedIn Corp. +// Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. +// You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + +import XCTest +@testable import LayoutKit + +class ViewRecyclerViewStorageTests: XCTestCase { + + func testViewWithReuseId_viewWithReuseIdExists_shouldReturnView() { + let view1 = View(viewReuseId: "1") + let view2 = View(viewReuseId: "2") + let storage = ViewRecyclerViewStorage() + storage.add(view: view1) + storage.add(view: view2) + + let v = storage.popView(withReuseId: "2") + XCTAssertEqual(view2, v) + } + + func testViewWithReuseId_viewWithReuseIdExists_shouldNotBeAbleToPopSameViewTwice() { + let view1 = View(viewReuseId: "1") + let view2 = View(viewReuseId: "2") + let storage = ViewRecyclerViewStorage() + storage.add(view: view1) + storage.add(view: view2) + + XCTAssertEqual(storage.popView(withReuseId: "2"), view2) + XCTAssertNil(storage.popView(withReuseId: "2"), "View should already been popped") + } + + func testViewWithReuseGroup_viewWithReuseGroupMultipleMatches_shouldKeepReturningViewsMatchingGroup() { + let view1 = View(viewReuseId: "1", viewReuseGroup: "group1") + let view2 = View(viewReuseId: "2", viewReuseGroup: "group1") + let storage = ViewRecyclerViewStorage() + storage.add(view: view1) + storage.add(view: view2) + + XCTAssertEqual(storage.popView(withReuseGroup: "group1"), view2) + XCTAssertEqual(storage.popView(withReuseGroup: "group1"), view1) + XCTAssertNil(storage.popView(withReuseGroup: "group1")) + } + + func testViewWithReuseGroup_viewWithReuseGroupMatch_shouldNotBeAbleToRetrieveThatViewAgain() { + let view1 = View(viewReuseId: "1", viewReuseGroup: "group1") + let view2 = View(viewReuseId: "2", viewReuseGroup: "group1") + let storage = ViewRecyclerViewStorage() + storage.add(view: view1) + storage.add(view: view2) + + XCTAssertEqual(storage.popView(withReuseGroup: "group1"), view2) + XCTAssertNil(storage.popView(withReuseId: "2"), "View should already been popped") + } + + func testRemoveView() { + let view1 = View(viewReuseId: "1") + let view2 = View(viewReuseId: "2") + let view3 = View(viewReuseId: "3", viewReuseGroup: "group") + let storage = ViewRecyclerViewStorage() + storage.add(view: view1) + storage.add(view: view2) + storage.add(view: view3) + + storage.remove(view: view2) + XCTAssertNil(storage.popView(withReuseId: "2"), "View should have been removed") + } + + func testRemoveAll_viewReuseIds() { + let view1 = View(viewReuseId: "1") + let view2 = View(viewReuseId: "2") + let view3 = View(viewReuseId: "3", viewReuseGroup: "group") + let storage = ViewRecyclerViewStorage() + storage.add(view: view1) + storage.add(view: view2) + storage.add(view: view3) + + storage.removeAll() + + XCTAssertNil(storage.popView(withReuseId: "1"), "View should have been removed") + XCTAssertNil(storage.popView(withReuseId: "2"), "View should have been removed") + XCTAssertNil(storage.popView(withReuseGroup: "group"), "View should have been removed") + } +} diff --git a/Sources/Layout.swift b/Sources/Layout.swift index c4a0af00..9aae9674 100644 --- a/Sources/Layout.swift +++ b/Sources/Layout.swift @@ -105,6 +105,14 @@ public protocol Layout { then that view will be reused for the new layout. If there is more than one view with the same viewReuseId, then an arbitrary one will be reused. */ var viewReuseId: String? { get } + + + /** + An identifier that specifies the type view content + + Use this identifier enable view reuse between layouts that produce same kind of views. + */ + var viewReuseGroup: String? { get } } public extension Layout { diff --git a/Sources/LayoutArrangement.swift b/Sources/LayoutArrangement.swift index 94289741..b06ae14e 100644 --- a/Sources/LayoutArrangement.swift +++ b/Sources/LayoutArrangement.swift @@ -119,7 +119,7 @@ public struct LayoutArrangement { return sublayout.makeSubviews(from: recycler, prepareAnimation: prepareAnimation) }) // If we are preparing an animation, then we don't want to update frames or configure views. - if layout.needsView, let view = recycler.makeOrRecycleView(havingViewReuseId: layout.viewReuseId, viewProvider: layout.makeView) { + if layout.needsView, let view = recycler.makeOrRecycleView(havingViewReuseId: layout.viewReuseId, orViewReuseGroup: layout.viewReuseGroup, viewProvider: layout.makeView) { if !prepareAnimation { view.frame = frame layout.configure(baseTypeView: view) @@ -163,6 +163,12 @@ extension View { will be adjusted so that its absolute position on the screen does not change. */ fileprivate func addSubview(_ view: View, maintainCoordinates: Bool) { + + // Disconnect reference cycle + if isChildOf(view) { + removeFromSuperview() + } + if maintainCoordinates { let frame = view.convertToAbsoluteCoordinates(view.frame) addSubview(view) @@ -171,5 +177,16 @@ extension View { addSubview(view) } } + + fileprivate func isChildOf(_ view: View) -> Bool { + + guard let superview = self.superview else { return false } + + if superview == view { + return true + } + + return superview.isChildOf(view) + } } diff --git a/Sources/Layouts/BaseLayout.swift b/Sources/Layouts/BaseLayout.swift index e4fc5b09..014ac2a4 100644 --- a/Sources/Layouts/BaseLayout.swift +++ b/Sources/Layouts/BaseLayout.swift @@ -23,6 +23,10 @@ open class BaseLayout { /// It is used to identify which views should be reused when animating from one layout to another. open let viewReuseId: String? + /// An identifier for layouts that produce same view + /// It is used to identify if view can be reused for another layout + open let viewReuseGroup: String? + /// A configuration block that is run on the main thread after the view is created. open let config: ((V) -> Void)? @@ -30,10 +34,11 @@ open class BaseLayout { return config != nil } - public init(alignment: Alignment, flexibility: Flexibility, viewReuseId: String? = nil, config: ((V) -> Void)?) { + public init(alignment: Alignment, flexibility: Flexibility, viewReuseId: String? = nil, viewReuseGroup: String? = nil, config: ((V) -> Void)?) { self.alignment = alignment self.flexibility = flexibility self.viewReuseId = viewReuseId + self.viewReuseGroup = viewReuseGroup self.config = config } diff --git a/Sources/Layouts/ButtonLayout.swift b/Sources/Layouts/ButtonLayout.swift index ef5e5b95..2d743883 100644 --- a/Sources/Layouts/ButtonLayout.swift +++ b/Sources/Layouts/ButtonLayout.swift @@ -36,6 +36,7 @@ open class ButtonLayout: BaseLayout