diff --git a/CHANGELOG.md b/CHANGELOG.md index 27bce5c90..5efed5e70 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ### Fixed - Fixed an issue where supplementary views (headers or footers) that contained a first responder would result in the view being duplicated when scrolled off-screen. +- Fixed an issue where a crash would occur when applying an external update to list content while a live reorder event was occurring. ### Added diff --git a/Demo/Sources/Demos/Demo Screens/PaymentTypesViewController.swift b/Demo/Sources/Demos/Demo Screens/PaymentTypesViewController.swift index 926e7fd83..b5096309f 100644 --- a/Demo/Sources/Demos/Demo Screens/PaymentTypesViewController.swift +++ b/Demo/Sources/Demos/Demo Screens/PaymentTypesViewController.swift @@ -12,12 +12,19 @@ import BlueprintUICommonControls final class PaymentTypesViewController : ListViewController { + override func viewDidLoad() { + super.viewDidLoad() + + self.updateNavigationItems() + } + override func configure(list: inout ListProperties) { list.layout = .table { table in table.layout.interSectionSpacingWithNoFooter = 10.0 table.bounds = .init( - padding: UIEdgeInsets(top: 0, left: 0, bottom: 100, right: 0) + padding: UIEdgeInsets(top: 0, left: 0, bottom: 100, right: 0), + width: .atMost(600) ) } @@ -30,21 +37,31 @@ final class PaymentTypesViewController : ListViewController { list += Section(SectionID.main) { section in section.header = PaymentTypeHeader(title: SectionID.main.title) - + section += types.filter { $0.isEnabled } .filter { $0.isMain } .sorted { $0.sortOrder < $1.sortOrder } .map(makeItem(with:)) + + if section.items.isEmpty { + section += EmptyRow() + } } list += Section(SectionID.more) { section in section.header = PaymentTypeHeader(title: SectionID.more.title) + section.reordering.minItemCount = 0 + section += types.filter { $0.isEnabled } .filter { $0.isMain == false } .sorted { $0.sortOrder < $1.sortOrder } .map(makeItem(with:)) + + if section.items.isEmpty { + section += EmptyRow() + } } list += Section(SectionID.disabled) { section in @@ -63,6 +80,56 @@ final class PaymentTypesViewController : ListViewController { } } + private var reloadingListTimer : Timer? = nil { + didSet { + oldValue?.invalidate() + } + } + + private func updateNavigationItems() { + + if reloadingListTimer == nil { + self.navigationItem.rightBarButtonItem = UIBarButtonItem( + title: "Start Reloading", + style: .plain, + target: self, + action: #selector(beginReloading) + ) + } else { + self.navigationItem.rightBarButtonItem = UIBarButtonItem( + title: "Stop Reloading", + style: .plain, + target: self, + action: #selector(endReloading) + ) + } + } + + @objc private func beginReloading() { + + let alert = UIAlertController( + title: "Will Repeatedly Reload List", + message: "The list will be reloaded every 0.5 seconds to verify that reordering + reloading does not cause a crash.", + preferredStyle: .alert + ) + + alert.addAction(.init(title: "OK", style: .default) { [weak self] _ in + self?.reloadingListTimer = Timer.scheduledTimer(withTimeInterval: 0.5, repeats: true, block: { _ in + self?.reload(animated: true) + }) + + self?.updateNavigationItems() + }) + + self.present(alert, animated: true) + } + + @objc private func endReloading() { + self.reloadingListTimer = nil + + updateNavigationItems() + } + private func save(with info : ListStateObserver.ItemReordered) { let main = info.sections.first { $0.identifier == Section.identifier(with: SectionID.main) }! let more = info.sections.first { $0.identifier == Section.identifier(with: SectionID.more) }! @@ -136,7 +203,7 @@ fileprivate struct PaymentTypeHeader : BlueprintHeaderFooterContent, Equatable { var elementRepresentation: Element { Label(text: title) { - $0.font = .systemFont(ofSize: 18.0, weight: .medium) + $0.font = .systemFont(ofSize: 18.0, weight: .bold) } .inset(uniform: 15.0) } diff --git a/ListableUI/Sources/ListView/ListChangesQueue.swift b/ListableUI/Sources/ListView/ListChangesQueue.swift index 8d650e312..0c3ef28ab 100644 --- a/ListableUI/Sources/ListView/ListChangesQueue.swift +++ b/ListableUI/Sources/ListView/ListChangesQueue.swift @@ -11,7 +11,7 @@ import Foundation /// A queue used to synchronized and serialize changes made to the backing collection view, /// to work around either bugs or confusing behavior. /// -/// ## Handling Re-ordering (`isQueuingForReorderEvent`) +/// ## Handling Applying Re-ordering / Move Events (`isQueuingToApplyReorderEvent`) /// Collection View has an issue wherein if you perform a re-order event, and then within /// the same runloop, deliver an update to the collection view as a result of that re-order event /// that removes a row or section, the collection view will crash because it's internal index path @@ -19,8 +19,46 @@ import Foundation /// we set this value to `true`, and then after one runloop, we set it back to `false`, after /// the collection view's updates have "settled". Please see `sendEndQueuingEditsAfterDelay` for more. /// +/// ## Disabling Updates During In-Progress Re-orders (`listHasUncommittedReorderUpdates`) +/// If an update is pushed into a `UICollectionView` while a reorder is in progress, there will be a crash +/// as the collection view tries to layout an index path that does not exist in the data source, as the reordering event +/// has not yet been committed. As such, we'll queue external updates while reordering is in progress. +/// +/// ``` +/// 💥 +/// Array.subscript.getter () +/// ListLayoutContent.item(at:) +/// ListLayoutContent.layoutAttributes(at:) +/// CollectionViewLayout.layoutAttributesForItem(at:) +/// @objc CollectionViewLayout.layoutAttributesForItem(at:) +/// -[UICollectionViewData layoutAttributesForItemAtIndexPath:] +/// -[UICollectionViewData layoutAttributesForGlobalItemIndex:] +/// __107-[UICollectionView _attributesForItemsVisibleDuringCurrentUpdateWithOldVisibleViews:attributesForNewModel:]_block_invoke +/// __NSDICTIONARY_IS_CALLING_OUT_TO_A_BLOCK__ +/// -[__NSDictionaryM enumerateKeysAndObjectsWithOptions:usingBlock:] +/// -[_UICollectionViewSubviewManager enumerateCellsWithEnumerator:] +/// -[UICollectionView _attributesForItemsVisibleDuringCurrentUpdateWithOldVisibleViews:attributesForNewModel:] +/// -[UICollectionView /// _createAndAppendViewAnimationsForExistingAndNewlyVisibleItemsInCurrentUpdate:animationsForOnScreenViews:newSubviewManager:oldVisibleViews:attributesF/// orNewModel:] +/// -[UICollectionView _viewAnimationsForCurrentUpdateWithCollectionViewAnimator:] +/// __102-[UICollectionView _updateWithItems:tentativelyForReordering:propertyAnimator:collectionViewAnimator:]_block_invoke.632 +/// +[UIView(Animation) performWithoutAnimation:] +/// -[UICollectionView _updateWithItems:tentativelyForReordering:propertyAnimator:collectionViewAnimator:] +/// -[UICollectionView _endItemAnimationsWithInvalidationContext:tentativelyForReordering:animator:collectionViewAnimator:] +/// -[UICollectionView _performBatchUpdates:completion:invalidationContext:tentativelyForReordering:animator:animationHandler:] +/// ListView.IOS16_4_First_Responder_Bug_CollectionView.performBatchUpdates(_:changes:completion:) +/// closure #3 in ListView.performBatchUpdates(with:animated:updateBackingData:collectionViewUpdateCompletion:animationCompletion:) +/// ListView.performBatchUpdates(with:animated:updateBackingData:collectionViewUpdateCompletion:animationCompletion:) +/// closure #1 in ListView.updatePresentationStateWith(firstVisibleIndexPath:for:completion:) +/// closure #1 in ListChangesQueue.add(async:) +/// closure #2 in ListChangesQueue.runIfNeeded() +/// ListChangesQueue.Operation.ifSynchronous(_:ifAsynchronous:) +/// ListChangesQueue.runIfNeeded() +/// ListChangesQueue.add(sync:) +/// ListView.configure(with:) +/// ``` +/// /// ## Handling async batch updates (`add(async:)`) -/// Because we peform updates to _our_ backing data model (`PresentationState`) alongside +/// Because we perform updates to _our_ backing data model (`PresentationState`) alongside /// our collection view in order to make sure they remain in sync, we need to handle cases where /// `UICollectionView.performBatchUpdates(_:completion:)` does not synchronously /// invoke its `update` block, which means state can get out of sync. @@ -104,18 +142,24 @@ final class ListChangesQueue { self.runIfNeeded() } - /// Set by consumers to enable and disable queueing during a reorder event. - var isQueuingForReorderEvent : Bool = false { + /// Set by consumers to enable and disable queueing when a reorder event is being applied. + var isQueuingToApplyReorderEvent : Bool = false { didSet { self.runIfNeeded() } } - /// Prevents processing other events in the queue. + /// Should be set to `{ collectionView.hasUncommittedUpdates }`. /// - /// Note: Right now this just checks `isQueuingForReorderEvent`, but may check more props in the future. + /// When this closure returns `true`, the queue is paused, to avoid crashes when applying + /// content updates while there are index-changing reorder events in process. + var listHasUncommittedReorderUpdates : () -> Bool = { + fatalError("Must set `listHasUncommittedReorderUpdates` before using `ListChangesQueue`.") + } + + /// Prevents processing other events in the queue. var isPaused : Bool { - self.isQueuingForReorderEvent + self.isQueuingToApplyReorderEvent || self.listHasUncommittedReorderUpdates() } var isEmpty : Bool { diff --git a/ListableUI/Sources/ListView/ListView.DataSource.swift b/ListableUI/Sources/ListView/ListView.DataSource.swift index efb4cd7d2..5d9071bec 100644 --- a/ListableUI/Sources/ListView/ListView.DataSource.swift +++ b/ListableUI/Sources/ListView/ListView.DataSource.swift @@ -152,7 +152,7 @@ internal extension ListView /// /// See `sendEndQueuingEditsAfterDelay` for a more in-depth explanation. /// - self.view.updateQueue.isQueuingForReorderEvent = true + self.view.updateQueue.isQueuingToApplyReorderEvent = true /// Perform the change in our data source. diff --git a/ListableUI/Sources/ListView/ListView.Delegate.swift b/ListableUI/Sources/ListView/ListView.Delegate.swift index ae9ba3aff..56f97cfe2 100644 --- a/ListableUI/Sources/ListView/ListView.Delegate.swift +++ b/ListableUI/Sources/ListView/ListView.Delegate.swift @@ -288,7 +288,7 @@ extension ListView } func listViewShouldEndQueueingEditsForReorder() { - self.view.updateQueue.isQueuingForReorderEvent = false + self.view.updateQueue.isQueuingToApplyReorderEvent = false } // MARK: UIScrollViewDelegate diff --git a/ListableUI/Sources/ListView/ListView.swift b/ListableUI/Sources/ListView/ListView.swift index cb49055da..7c60736a4 100644 --- a/ListableUI/Sources/ListView/ListView.swift +++ b/ListableUI/Sources/ListView/ListView.swift @@ -67,6 +67,8 @@ public final class ListView : UIView self.closeActiveSwipesGesture = TouchDownGestureRecognizer() + self.updateQueue = ListChangesQueue() + // Super init. super.init(frame: frame) @@ -91,6 +93,10 @@ public final class ListView : UIView self?.shouldRecognizeCloseSwipeTouch(touch) ?? false } + self.updateQueue.listHasUncommittedReorderUpdates = { [weak collectionView] in + collectionView?.hasUncommittedUpdates ?? false + } + // Register supplementary views. SupplementaryKind.allCases.forEach { @@ -846,7 +852,7 @@ public final class ListView : UIView self.configure(with: description) } - let updateQueue = ListChangesQueue() + let updateQueue : ListChangesQueue public func configure(with properties : ListProperties) { diff --git a/ListableUI/Tests/ListView/ListChangesQueueTests.swift b/ListableUI/Tests/ListView/ListChangesQueueTests.swift index 6af5f5de4..b7406d1fd 100644 --- a/ListableUI/Tests/ListView/ListChangesQueueTests.swift +++ b/ListableUI/Tests/ListView/ListChangesQueueTests.swift @@ -14,9 +14,10 @@ class ListChangesQueueTests : XCTestCase { func test_pausing() { let queue = ListChangesQueue() + queue.listHasUncommittedReorderUpdates = { false } XCTAssertFalse(queue.isPaused) - XCTAssertFalse(queue.isQueuingForReorderEvent) + XCTAssertFalse(queue.isQueuingToApplyReorderEvent) var calls : [Int] = [] @@ -35,10 +36,10 @@ class ListChangesQueueTests : XCTestCase { XCTAssertEqual(queue.count, 0) XCTAssertEqual(calls, [1, 2]) - queue.isQueuingForReorderEvent = true + queue.isQueuingToApplyReorderEvent = true XCTAssertTrue(queue.isPaused) - XCTAssertTrue(queue.isQueuingForReorderEvent) + XCTAssertTrue(queue.isQueuingToApplyReorderEvent) queue.add { calls += [3] @@ -57,10 +58,10 @@ class ListChangesQueueTests : XCTestCase { XCTAssertEqual(queue.count, 3) XCTAssertEqual(calls, [1, 2]) - queue.isQueuingForReorderEvent = false + queue.isQueuingToApplyReorderEvent = false XCTAssertFalse(queue.isPaused) - XCTAssertFalse(queue.isQueuingForReorderEvent) + XCTAssertFalse(queue.isQueuingToApplyReorderEvent) waitFor { queue.isEmpty @@ -72,6 +73,7 @@ class ListChangesQueueTests : XCTestCase { func test_synchronous() { let queue = ListChangesQueue() + queue.listHasUncommittedReorderUpdates = { false } var calls : [Int] = [] @@ -99,6 +101,7 @@ class ListChangesQueueTests : XCTestCase { func test_asynchronous() { let queue = ListChangesQueue() + queue.listHasUncommittedReorderUpdates = { false } var calls : [Int] = [] @@ -157,6 +160,7 @@ class ListChangesQueueTests : XCTestCase { func test_both() { let queue = ListChangesQueue() + queue.listHasUncommittedReorderUpdates = { false } var calls : [Int] = [] @@ -225,6 +229,7 @@ class ListChangesQueueTests : XCTestCase { func test_fuzzing() { let queue = ListChangesQueue() + queue.listHasUncommittedReorderUpdates = { false } var calls : [Int] = []