diff --git a/LayoutKit.xcodeproj/project.pbxproj b/LayoutKit.xcodeproj/project.pbxproj index 275da17d..a8a2baf9 100644 --- a/LayoutKit.xcodeproj/project.pbxproj +++ b/LayoutKit.xcodeproj/project.pbxproj @@ -290,6 +290,9 @@ 7EECD05B2053916C003DC4B1 /* LOKLabelLayoutBuilder.h in Headers */ = {isa = PBXBuildFile; fileRef = 7E7370EC2051E08F007C19FF /* LOKLabelLayoutBuilder.h */; settings = {ATTRIBUTES = (Public, ); }; }; 7EECD05C2053916C003DC4B1 /* LOKTextViewLayoutBuilder.h in Headers */ = {isa = PBXBuildFile; fileRef = 7E73710420520F5F007C19FF /* LOKTextViewLayoutBuilder.h */; settings = {ATTRIBUTES = (Public, ); }; }; 7EECD0632053942F003DC4B1 /* LayoutKitObjC.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 7EECD0612053916C003DC4B1 /* LayoutKitObjC.framework */; }; + A189721221B8BB8400DDA616 /* EmbeddedLayoutTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = A189721021B8BB3B00DDA616 /* EmbeddedLayoutTests.swift */; }; + A189721321B8BB8500DDA616 /* EmbeddedLayoutTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = A189721021B8BB3B00DDA616 /* EmbeddedLayoutTests.swift */; }; + A189721521B8CDA000DDA616 /* EmbeddedLayoutTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = A189721021B8BB3B00DDA616 /* EmbeddedLayoutTests.swift */; }; AD2C36441EA5AFB500550A03 /* ReloadableViewLayoutAdapterCollectionViewOverrideTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = AD2C36421EA5AF9500550A03 /* ReloadableViewLayoutAdapterCollectionViewOverrideTests.swift */; }; ADE5FCC11EA5B5F3006A3DC2 /* ReloadableViewLayoutAdapterTableViewOverrideTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = ADE5FCBF1EA5B5C8006A3DC2 /* ReloadableViewLayoutAdapterTableViewOverrideTests.swift */; }; CDD4F71020EC727800DB358C /* CollectionExtension.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0B193BB61D887BCF00FCA22D /* CollectionExtension.swift */; }; @@ -528,6 +531,7 @@ 7EEA2ACC201D1FE90077A088 /* Launch Screen.storyboard */ = {isa = PBXFileReference; lastKnownFileType = file.storyboard; path = "Launch Screen.storyboard"; sourceTree = ""; }; 7EECD0612053916C003DC4B1 /* LayoutKitObjC.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = LayoutKitObjC.framework; sourceTree = BUILT_PRODUCTS_DIR; }; 7EECD0622053916C003DC4B1 /* LayoutKit-iOS copy-Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; name = "LayoutKit-iOS copy-Info.plist"; path = "/Users/staguer/ws/lk0/LayoutKit-iOS copy-Info.plist"; sourceTree = ""; }; + A189721021B8BB3B00DDA616 /* EmbeddedLayoutTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EmbeddedLayoutTests.swift; sourceTree = ""; }; AD2C36421EA5AF9500550A03 /* ReloadableViewLayoutAdapterCollectionViewOverrideTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ReloadableViewLayoutAdapterCollectionViewOverrideTests.swift; sourceTree = ""; }; ADE5FCBF1EA5B5C8006A3DC2 /* ReloadableViewLayoutAdapterTableViewOverrideTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ReloadableViewLayoutAdapterTableViewOverrideTests.swift; sourceTree = ""; }; /* End PBXFileReference section */ @@ -750,6 +754,7 @@ 44F968161E42639500392763 /* TextViewLayoutTests.swift */, 0BCB76671D8725310065E02A /* UIFontExtension.swift */, 0BCB76681D8725310065E02A /* ViewRecyclerTests.swift */, + A189721021B8BB3B00DDA616 /* EmbeddedLayoutTests.swift */, ); path = LayoutKitTests; sourceTree = ""; @@ -1387,6 +1392,7 @@ 0B2D092C1D872F75007E487C /* ReloadableViewLayoutAdapterCollectionViewTests.swift in Sources */, CDD4F71320EC728200DB358C /* IndexSetExtension.swift in Sources */, 0B2D092E1D872F75007E487C /* ReloadableViewLayoutAdapterTestCase.swift in Sources */, + A189721321B8BB8500DDA616 /* EmbeddedLayoutTests.swift in Sources */, 0B2D092D1D872F75007E487C /* ReloadableViewLayoutAdapterTableViewTests.swift in Sources */, 0B2D09321D872F75007E487C /* StackLayoutSpacingTests.swift in Sources */, 75D94A3B1EA045F100A5FD01 /* OverlayLayoutTests.swift in Sources */, @@ -1474,6 +1480,7 @@ 0B2D093C1D872F75007E487C /* DensityAssertions.swift in Sources */, CDD4F71420EC728300DB358C /* IndexSetExtension.swift in Sources */, 0BB380DC1DB73EFF00E2614F /* TextExtension.swift in Sources */, + A189721221B8BB8400DDA616 /* EmbeddedLayoutTests.swift in Sources */, 0B8C078C1DC3E88A001CD5EE /* ButtonLayoutTests.swift in Sources */, 0BDDF95C1E25ACCE008B0A6F /* ReloadableViewTests.swift in Sources */, 0B2D093D1D872F75007E487C /* InsetLayoutTests.swift in Sources */, @@ -1527,6 +1534,7 @@ 0B2D09531D872F76007E487C /* InsetLayoutTests.swift in Sources */, CDD4F71220EC727900DB358C /* CollectionExtension.swift in Sources */, 0BA02E481D874BBB00F1E8D3 /* LayoutArrangementTests.swift in Sources */, + A189721521B8CDA000DDA616 /* EmbeddedLayoutTests.swift in Sources */, 75D94A3D1EA045F100A5FD01 /* OverlayLayoutTests.swift in Sources */, 0B2D095B1D872F76007E487C /* SizeLayoutTests.swift in Sources */, 0B2D09621D872F76007E487C /* TestStack.swift in Sources */, diff --git a/LayoutKitTests/EmbeddedLayoutTests.swift b/LayoutKitTests/EmbeddedLayoutTests.swift new file mode 100644 index 00000000..db39b804 --- /dev/null +++ b/LayoutKitTests/EmbeddedLayoutTests.swift @@ -0,0 +1,92 @@ +// 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 EmbeddedLayoutTests: XCTestCase { + + private let rootView = View() + private let singleViewArrangement = SizeLayout(minSize: .zero, config: { _ in }).arrangement() + + override func setUp() { + super.setUp() + + rootView.subviews.forEach { $0.removeFromSuperview() } + } + + func testKeepsSubviewsForEmbeddedLayoutWithReuseId() { + var parentView: View? + let parentLayout = SizeLayout(minSize: .zero, viewReuseId: "test", config: { view in + parentView = view + }) + + // Create Parent Layout + parentLayout.arrangement().makeViews(in: rootView) + + // Create Embedded Layout + XCTAssertNotNil(parentView) + singleViewArrangement.makeViews(in: parentView) + + // Re-create Parent Layout + parentLayout.arrangement().makeViews(in: rootView) + + XCTAssertEqual(parentView?.subviews.count, 1) + } + + func testRemovesSubviewsForEmbeddedLayoutWithoutReuseId() { + var parentView: View? + let parentLayout = SizeLayout(minSize: .zero, config: { view in + parentView = view + }) + + // Create Parent Layout + parentLayout.arrangement().makeViews(in: rootView) + + // Create Embedded Layout + XCTAssertNotNil(parentView) + singleViewArrangement.makeViews(in: parentView) + let originalParentView = parentView + + // Re-create Parent Layout + parentLayout.arrangement().makeViews(in: rootView) + + XCTAssertNotEqual(originalParentView, parentView) + XCTAssertEqual(parentView?.subviews.count, 0) + } + + func testEmbeddedLayoutsAreRemoved() { + var originalHostView: View? + SizeLayout( + minSize: .zero, + viewReuseId: "foo", + sublayout: SizeLayout(minSize: .zero, viewReuseId: "bar", config: { view in + originalHostView = view + self.singleViewArrangement.makeViews(in: view) + }), + config: { _ in }).arrangement().makeViews(in: rootView) + + XCTAssertEqual(rootView.subviews.count, 1) + XCTAssertNotNil(originalHostView) + XCTAssertEqual(originalHostView?.subviews.count, 1) + + var updatedHostView: View? + SizeLayout( + minSize: .zero, + viewReuseId: "foo", + sublayout: SizeLayout(minSize: .zero, viewReuseId: "baz", config: { view in + updatedHostView = view + }), + config: { _ in }).arrangement().makeViews(in: rootView) + + XCTAssertEqual(rootView.subviews.count, 1) + XCTAssertNotNil(updatedHostView) + XCTAssertEqual(updatedHostView?.subviews.count, 0) + XCTAssertNotEqual(originalHostView, updatedHostView) + } +} diff --git a/LayoutKitTests/ViewRecyclerTests.swift b/LayoutKitTests/ViewRecyclerTests.swift index 8fd291ad..81313ae8 100644 --- a/LayoutKitTests/ViewRecyclerTests.swift +++ b/LayoutKitTests/ViewRecyclerTests.swift @@ -14,7 +14,7 @@ class ViewRecyclerTests: XCTestCase { func testNilIdNotRecycledAndNotRemoved() { let root = View() let zero = View() - zero.isLayoutKitView = false // default + zero.type = .unmanaged // default root.addSubview(zero) let recycler = ViewRecycler(rootView: root) @@ -25,13 +25,13 @@ class ViewRecyclerTests: XCTestCase { XCTAssertEqual(v, expectedView) recycler.purgeViews() - XCTAssertNotNil(zero.superview, "`zero` should not be removed because `isLayoutKitView` is false") + XCTAssertNotNil(zero.superview, "`zero` should not be removed because `type` is unmanaged") } func testNilIdNotRecycledAndRemoved() { let root = View() let zero = View() - zero.isLayoutKitView = true // requires this flag to be removed by `ViewRecycler` + zero.type = .managed // requires this flag to be removed by `ViewRecycler` root.addSubview(zero) let recycler = ViewRecycler(rootView: root) @@ -42,7 +42,7 @@ class ViewRecyclerTests: XCTestCase { XCTAssertEqual(v, expectedView) recycler.purgeViews() - XCTAssertNil(zero.superview, "`zero` should be removed because `isLayoutKitView` is true") + XCTAssertNil(zero.superview, "`zero` should be removed because `type` is managed") } func testNonNilIdRecycled() { @@ -91,6 +91,49 @@ class ViewRecyclerTests: XCTestCase { XCTAssertNotNil(one.superview) } + func testRootSubviewsMarkedAsManaged() { + let root = View() + let one = View(viewReuseId: "1") + one.type = .root + root.addSubview(one) + let two = View(viewReuseId: "2") + two.type = .root + one.addSubview(two) + + let _ = ViewRecycler(rootView: root) + + XCTAssertEqual(one.type, .managed) + XCTAssertEqual(two.type, .root) + } + + func testDoesNotRecycleRootViews() { + let root = View() + let one = View(viewReuseId: "1") + one.type = .root + root.addSubview(one) + let two = View(viewReuseId: "2") + two.type = .root + one.addSubview(two) + + let recycler = ViewRecycler(rootView: root) + + // Reuse one so it is not purged from the view hierarchy + _ = recycler.makeOrRecycleView(havingViewReuseId: "1", viewProvider: { + XCTFail("view should have been recycled") + return View() + }) + + let expectedView = View() + let v: View? = recycler.makeOrRecycleView(havingViewReuseId: "2", viewProvider: { + return expectedView + }) + XCTAssertEqual(v, expectedView) + + recycler.purgeViews() + XCTAssertNotNil(one.superview) + XCTAssertNotNil(two.superview) + } + #if os(iOS) || os(tvOS) /// Test that a reused view's frame shouldn't change if its transform and layer anchor point /// get set to the default values. diff --git a/Sources/LayoutArrangement.swift b/Sources/LayoutArrangement.swift index 94289741..8adcef9b 100644 --- a/Sources/LayoutArrangement.swift +++ b/Sources/LayoutArrangement.swift @@ -73,6 +73,7 @@ public struct LayoutArrangement { private func makeViews(in view: View? = nil, direction: UserInterfaceLayoutDirection, prepareAnimation: Bool) -> View { let recycler = ViewRecycler(rootView: view) let views = makeSubviews(from: recycler, prepareAnimation: prepareAnimation) + recycler.markViewsAsRoot(views) let rootView: View if let view = view { diff --git a/Sources/ViewRecycler.swift b/Sources/ViewRecycler.swift index c9df554c..a7f014e3 100644 --- a/Sources/ViewRecycler.swift +++ b/Sources/ViewRecycler.swift @@ -18,6 +18,7 @@ import UIKit Initialize ViewRecycler with a root view whose subviews are eligible for recycling. Call `makeView(layoutId:)` to recycle or create a view of the desired type and id. Call `purgeViews()` to remove all unrecycled views from the view hierarchy. + Call `markViewsAsRoot(views:)` to mark the top level views of generated view hierarchy */ class ViewRecycler { @@ -30,7 +31,17 @@ class ViewRecycler { /// Retains all subviews of rootView for recycling. init(rootView: View?) { - rootView?.walkSubviews { (view) in + guard let rootView = rootView else { + return + } + + // Mark all direct subviews from rootView as managed. + // We are recreating the layout they were previously roots of. + for view in rootView.subviews where view.type == .root { + view.type = .managed + } + + rootView.walkNonRootSubviews { (view) in if let viewReuseId = view.viewReuseId { self.viewsById[viewReuseId] = view } else { @@ -77,7 +88,7 @@ class ViewRecycler { } let providedView = viewProvider() - providedView.isLayoutKitView = true + providedView.type = .managed // Remove the provided view from the list of cached views. if let viewReuseId = providedView.viewReuseId, let oldView = viewsById[viewReuseId], oldView == providedView { @@ -96,23 +107,36 @@ class ViewRecycler { } viewsById.removeAll() - for view in unidentifiedViews where view.isLayoutKitView { + for view in unidentifiedViews where view.type == .managed { view.removeFromSuperview() } unidentifiedViews.removeAll() } + + func markViewsAsRoot(_ views: [View]) { + views.forEach { $0.type = .root } + } } private var viewReuseIdKey: UInt8 = 0 -private var isLayoutKitViewKey: UInt8 = 0 +private var typeKey: UInt8 = 0 extension View { + enum ViewType: UInt8 { + // Indicates the view was not created by LayoutKit and should not be modified. + case unmanaged + // Indicates the view is managed by LayoutKit that can be safely removed. + case managed + // Indicates the view is managed by LayoutKit but was generated by another layout and should not be modified. + case root + } + /// Calls visitor for each transitive subview. - func walkSubviews(visitor: (View) -> Void) { - for subview in subviews { + func walkNonRootSubviews(visitor: (View) -> Void) { + for subview in subviews where subview.type != .root { visitor(subview) - subview.walkSubviews(visitor: visitor) + subview.walkNonRootSubviews(visitor: visitor) } } @@ -122,17 +146,17 @@ extension View { return objc_getAssociatedObject(self, &viewReuseIdKey) as? String } set { - objc_setAssociatedObject(self, &viewReuseIdKey, newValue, .OBJC_ASSOCIATION_RETAIN) + objc_setAssociatedObject(self, &viewReuseIdKey, newValue, .OBJC_ASSOCIATION_COPY_NONATOMIC) } } - /// Indicates the view is managed by LayoutKit that can be safely removed. - var isLayoutKitView: Bool { + var type: ViewType { get { - return (objc_getAssociatedObject(self, &isLayoutKitViewKey) as? NSNumber)?.boolValue ?? false + return objc_getAssociatedObject(self, &typeKey) as? ViewType ?? .unmanaged } set { - objc_setAssociatedObject(self, &isLayoutKitViewKey, NSNumber(value: newValue), .OBJC_ASSOCIATION_RETAIN) + let type: ViewType? = (newValue == .unmanaged) ? nil : newValue + objc_setAssociatedObject(self, &typeKey, type, .OBJC_ASSOCIATION_COPY_NONATOMIC) } } }