diff --git a/MagazineLayout/LayoutCore/Types/TargetContentOffsetAnchor.swift b/MagazineLayout/LayoutCore/Types/TargetContentOffsetAnchor.swift index 1252b1d..bdefeaa 100644 --- a/MagazineLayout/LayoutCore/Types/TargetContentOffsetAnchor.swift +++ b/MagazineLayout/LayoutCore/Types/TargetContentOffsetAnchor.swift @@ -22,8 +22,8 @@ import UIKit enum TargetContentOffsetAnchor: Equatable { case top case bottom - case topItem(id: String, distanceFromTop: CGFloat) - case bottomItem(id: String, distanceFromBottom: CGFloat) + case topItem(id: String, itemEdge: ItemEdge, distanceFromTop: CGFloat) + case bottomItem(id: String, itemEdge: ItemEdge, distanceFromBottom: CGFloat) static func targetContentOffsetAnchor( verticalLayoutDirection: MagazineLayoutVerticalLayoutDirection, @@ -31,16 +31,19 @@ enum TargetContentOffsetAnchor: Equatable { bottomInset: CGFloat, bounds: CGRect, contentHeight: CGFloat, + scale: CGFloat, firstVisibleItemID: String, lastVisibleItemID: String, firstVisibleItemFrame: CGRect, lastVisibleItemFrame: CGRect) -> Self { + let top = (-topInset).alignedToPixel(forScreenWithScale: scale) + let bottom = (contentHeight + bottomInset - bounds.height).alignedToPixel(forScreenWithScale: scale) let position: Position - if bounds.minY <= -topInset { + if bounds.minY <= top { position = .atTop - } else if bounds.minY >= contentHeight + bottomInset - bounds.height { + } else if bounds.minY >= bottom { position = .atBottom } else { position = .inMiddle @@ -52,14 +55,38 @@ enum TargetContentOffsetAnchor: Equatable { case .atTop: return .top case .inMiddle, .atBottom: - let distanceFromTop = firstVisibleItemFrame.minY - (bounds.minY + topInset) - return .topItem(id: firstVisibleItemID, distanceFromTop: distanceFromTop) + let top = bounds.minY + topInset + let topDistanceFromTop = firstVisibleItemFrame.value(for: .top) - top + let bottomDistanceFromTop = firstVisibleItemFrame.value(for: .bottom) - top + if abs(topDistanceFromTop) < abs(bottomDistanceFromTop) { + return .topItem( + id: firstVisibleItemID, + itemEdge: .top, + distanceFromTop: topDistanceFromTop.alignedToPixel(forScreenWithScale: scale)) + } else { + return .topItem( + id: firstVisibleItemID, + itemEdge: .bottom, + distanceFromTop: bottomDistanceFromTop.alignedToPixel(forScreenWithScale: scale)) + } } case .bottomToTop: switch position { case .atTop, .inMiddle: - let distanceFromBottom = lastVisibleItemFrame.maxY - (bounds.maxY - bottomInset) - return .bottomItem(id: lastVisibleItemID, distanceFromBottom: distanceFromBottom) + let bottom = bounds.maxY - bottomInset + let topDistanceFromBottom = lastVisibleItemFrame.value(for: .top) - bottom + let bottomDistanceFromBottom = lastVisibleItemFrame.value(for: .bottom) - bottom + if abs(topDistanceFromBottom) < abs(bottomDistanceFromBottom) { + return .bottomItem( + id: lastVisibleItemID, + itemEdge: .top, + distanceFromBottom: topDistanceFromBottom.alignedToPixel(forScreenWithScale: scale)) + } else { + return .bottomItem( + id: lastVisibleItemID, + itemEdge: .bottom, + distanceFromBottom: bottomDistanceFromBottom.alignedToPixel(forScreenWithScale: scale)) + } case .atBottom: return .bottom } @@ -82,15 +109,35 @@ enum TargetContentOffsetAnchor: Equatable { case .bottom: return contentHeight - bounds.height + bottomInset - case .topItem(let id, let distanceFromTop): + case .topItem(let id, let itemEdge, let distanceFromTop): guard let indexPath = indexPathForItemID(id) else { return bounds.minY } let itemFrame = frameForItemAtIndexPath(indexPath) - return itemFrame.minY - topInset - distanceFromTop + return itemFrame.value(for: itemEdge) - topInset - distanceFromTop - case .bottomItem(let id, let distanceFromBottom): + case .bottomItem(let id, let itemEdge, let distanceFromBottom): guard let indexPath = indexPathForItemID(id) else { return bounds.minY } let itemFrame = frameForItemAtIndexPath(indexPath) - return itemFrame.maxY - bounds.height + bottomInset - distanceFromBottom + return itemFrame.value(for: itemEdge) - bounds.height + bottomInset - distanceFromBottom + } + } + +} + +// MARK: ItemEdge + +enum ItemEdge { + case top + case bottom +} + +// MARK: CGRect + Item Edge Value + +private extension CGRect { + + func value(for itemEdge: ItemEdge) -> CGFloat { + switch itemEdge { + case .top: minY + case .bottom: maxY } } diff --git a/MagazineLayout/Public/MagazineLayout.swift b/MagazineLayout/Public/MagazineLayout.swift index e0db1fd..e21602e 100755 --- a/MagazineLayout/Public/MagazineLayout.swift +++ b/MagazineLayout/Public/MagazineLayout.swift @@ -99,13 +99,6 @@ public final class MagazineLayout: UICollectionViewLayout { override public func prepare() { super.prepare() - // Save the most recent content inset. We read this later in `invalidateLayout:`. Unlike the - // bounds and content size, the content insets are updated _before_ `invalidateLayout` is - // called, leaving us no choice other than tracking it separately here. - lastContentInset = contentInset - - guard !prepareActions.isEmpty else { return } - // Save the previous collection view width if necessary if prepareActions.contains(.cachePreviousWidth) { cachedCollectionViewWidth = currentCollectionView.bounds.width @@ -255,6 +248,9 @@ public final class MagazineLayout: UICollectionViewLayout { modelState.applyUpdates(updates) hasDataSourceCountInvalidationBeforeReceivingUpdateItems = false + lastSizedElementMinY = nil + lastSizedElementPreferredHeight = nil + super.prepare(forCollectionViewUpdates: updateItems) } @@ -266,7 +262,14 @@ public final class MagazineLayout: UICollectionViewLayout { targetContentOffsetAnchor = nil preInvalidationContentSize = nil - preInvalidationContentInset = nil + + if let stagedContentOffsetAdjustment { + let context = MagazineLayoutInvalidationContext() + context.invalidateLayoutMetrics = false + context.contentOffsetAdjustment = stagedContentOffsetAdjustment + invalidateLayout(with: context) + } + stagedContentOffsetAdjustment = nil super.finalizeCollectionViewUpdates() } @@ -276,15 +279,19 @@ public final class MagazineLayout: UICollectionViewLayout { targetContentOffsetAnchor = targetContentOffsetAnchor( bounds: oldBounds, - contentHeight: preInvalidationContentSize?.height ?? collectionViewContentSize.height, - topInset: preInvalidationContentInset?.top ?? contentInset.top, - bottomInset: preInvalidationContentInset?.bottom ?? contentInset.bottom) + contentHeight: preInvalidationContentSize?.height ?? currentCollectionView.contentSize.height, + // There doesn't seem to be a reliable way to get the correct content insets here. We can try + // track them in invalidateLayout or prepare, but then there are edge cases where we need to + // track multiple past inset values. It's a huge mess, and I don't think it's worth solving. + // The downside is that if your insets change on rotation, you won't always land in the exact + // correct spot if you're in the middle of the content. Being at the top or bottom works fine. + topInset: 0, + bottomInset: 0) } override public func finalizeAnimatedBoundsChange() { targetContentOffsetAnchor = nil preInvalidationContentSize = nil - preInvalidationContentInset = nil super.finalizeAnimatedBoundsChange() } @@ -665,6 +672,8 @@ public final class MagazineLayout: UICollectionViewLayout { withOriginalAttributes originalAttributes: UICollectionViewLayoutAttributes) -> UICollectionViewLayoutInvalidationContext { + let originalAttributes = originalAttributes.copy() as! UICollectionViewLayoutAttributes + switch preferredAttributes.representedElementCategory { case .cell: modelState.updateItemHeight( @@ -715,29 +724,60 @@ public final class MagazineLayout: UICollectionViewLayout { forPreferredLayoutAttributes: preferredAttributes, withOriginalAttributes: originalAttributes) as! MagazineLayoutInvalidationContext - // If layout information is discarded above our current scroll position (on rotation, for - // example), we need to compensate for preferred size changes to items as we're scrolling up, - // otherwise, the collection view will appear to jump each time an element is sized. - // Since size adjustments can occur for multiple items in the same soon-to-be-visible row, we - // need to account for this by considering the preferred height for previously sized elements in - // the same row so that we only adjust the content offset by the exact amount needed to create - // smooth scrolling. let isScrolling = currentCollectionView.isDragging || currentCollectionView.isDecelerating - let isSizingElementAboveTopEdge = originalAttributes.frame.minY < currentCollectionView.contentOffset.y - - if isScrolling && isSizingElementAboveTopEdge { + let top = currentCollectionView.bounds.minY + contentInset.top + let bottom = currentCollectionView.bounds.maxY + let isSizingElementAboveTopEdge = originalAttributes.frame.minY < top + let isSizingElementBelowBottomEdge = originalAttributes.frame.minY > bottom + + let contentOffsetAdjustment: CGPoint + if verticalLayoutDirection == .topToBottom, isSizingElementAboveTopEdge { + // If layout information is discarded above our current scroll position (on rotation, for + // example), we need to compensate for preferred size changes to items as we're scrolling up, + // otherwise, the collection view will appear to jump each time an element is sized. + // Since size adjustments can occur for multiple items in the same soon-to-be-visible row, we + // need to account for this by considering the preferred height for previously sized elements + // in the same row so that we only adjust the content offset by the exact amount needed to + // create smooth scrolling. let isSameRowAsLastSizedElement = lastSizedElementMinY?.isEqual( to: currentElementY, threshold: 1 / scale) ?? false + if isSameRowAsLastSizedElement { let lastSizedElementPreferredHeight = self.lastSizedElementPreferredHeight ?? 0 if preferredAttributes.size.height > lastSizedElementPreferredHeight { - context.contentOffsetAdjustment.y = preferredAttributes.size.height - lastSizedElementPreferredHeight + contentOffsetAdjustment = CGPoint( + x: 0, + y: preferredAttributes.size.height - lastSizedElementPreferredHeight) + } else { + contentOffsetAdjustment = .zero } } else { - context.contentOffsetAdjustment.y = preferredAttributes.size.height - originalAttributes.size.height + contentOffsetAdjustment = CGPoint( + x: 0, + y: preferredAttributes.size.height - originalAttributes.size.height) } + } else if + verticalLayoutDirection == .bottomToTop, + !isSizingElementBelowBottomEdge, + modelState.isPerformingBatchUpdates + { + contentOffsetAdjustment = CGPoint( + x: 0, + y: preferredAttributes.size.height - originalAttributes.size.height) + } else { + contentOffsetAdjustment = .zero + } + + if modelState.isPerformingBatchUpdates { + // If we're in the middle of a batch update, we need to adjust our content offset. Doing it + // here in the middle of a batch update gets ignored for some reason. Instead, we delay + // slightly and do it in `finalizeCollectionViewUpdates`. + // This needs to be set in `finalizeCollectionViewUpdates`, otherwise it doesn't get applied. + stagedContentOffsetAdjustment = contentOffsetAdjustment + } else if isScrolling { + context.contentOffsetAdjustment = contentOffsetAdjustment } lastSizedElementMinY = currentElementY @@ -754,11 +794,6 @@ public final class MagazineLayout: UICollectionViewLayout { return } - if collectionViewContentSize.width > 0, collectionViewContentSize.height > 0 { - preInvalidationContentSize = preInvalidationContentSize ?? collectionViewContentSize - } - preInvalidationContentInset = preInvalidationContentInset ?? lastContentInset ?? contentInset - let shouldInvalidateLayoutMetrics = !context.invalidateEverything && !context.invalidateDataSourceCounts @@ -829,10 +864,7 @@ public final class MagazineLayout: UICollectionViewLayout { private let _flipsHorizontallyInOppositeLayoutDirection: Bool private let verticalLayoutDirection: MagazineLayoutVerticalLayoutDirection - private var lastContentInset: UIEdgeInsets? private var cachedCollectionViewWidth: CGFloat? - private var preInvalidationContentSize: CGSize? - private var preInvalidationContentInset: UIEdgeInsets? // These properties are used to prevent scroll jumpiness due to self-sizing after rotation; see // comment in `invalidationContext(forPreferredLayoutAttributes:withOriginalAttributes:)` for more @@ -870,6 +902,8 @@ public final class MagazineLayout: UICollectionViewLayout { private var isPerformingAnimatedBoundsChange = false private var targetContentOffsetAnchor: TargetContentOffsetAnchor? + private var preInvalidationContentSize: CGSize? + private var stagedContentOffsetAdjustment: CGPoint? // Used to provide the model state with the current visible bounds for the sole purpose of // supporting pinned headers and footers. @@ -1241,18 +1275,15 @@ public final class MagazineLayout: UICollectionViewLayout { bottomInset: CGFloat) -> TargetContentOffsetAnchor? { + let insetBounds = bounds.inset(by: .init(top: topInset, left: 0, bottom: bottomInset, right: 0)) var visibleItemLocationFramePairs = [ElementLocationFramePair]() - for itemLocationFramePair in modelState.itemLocationFramePairs(forItemsIn: bounds) { + for itemLocationFramePair in modelState.itemLocationFramePairs(forItemsIn: insetBounds) { visibleItemLocationFramePairs.append(itemLocationFramePair) } visibleItemLocationFramePairs.sort { $0.elementLocation < $1.elementLocation } - let firstVisibleItemLocationFramePair = visibleItemLocationFramePairs.first { - $0.frame.minY >= bounds.minY + topInset - } - let lastVisibleItemLocationFramePair = visibleItemLocationFramePairs.last { - $0.frame.maxY <= bounds.minY + bounds.height - bottomInset - } + let firstVisibleItemLocationFramePair = visibleItemLocationFramePairs.first + let lastVisibleItemLocationFramePair = visibleItemLocationFramePairs.last guard let firstVisibleItemLocationFramePair, @@ -1273,6 +1304,7 @@ public final class MagazineLayout: UICollectionViewLayout { bottomInset: bottomInset, bounds: bounds, contentHeight: contentHeight, + scale: scale, firstVisibleItemID: firstVisibleItemID, lastVisibleItemID: lastVisibleItemID, firstVisibleItemFrame: firstVisibleItemLocationFramePair.frame, diff --git a/Tests/TargetContentOffsetAnchorTests.swift b/Tests/TargetContentOffsetAnchorTests.swift index abf6167..19f6964 100644 --- a/Tests/TargetContentOffsetAnchorTests.swift +++ b/Tests/TargetContentOffsetAnchorTests.swift @@ -28,6 +28,7 @@ final class TargetContentOffsetAnchorTests: XCTestCase { bottomInset: 30, bounds: CGRect(x: 0, y: -50, width: 300, height: 400), contentHeight: 2000, + scale: 1, firstVisibleItemID: "0", lastVisibleItemID: "4", firstVisibleItemFrame: CGRect(x: 0, y: 0, width: 300, height: 20), @@ -42,11 +43,12 @@ final class TargetContentOffsetAnchorTests: XCTestCase { bottomInset: 30, bounds: CGRect(x: 0, y: 500, width: 300, height: 400), contentHeight: 2000, + scale: 1, firstVisibleItemID: "2", lastVisibleItemID: "6", firstVisibleItemFrame: CGRect(x: 0, y: 560, width: 300, height: 20), lastVisibleItemFrame: CGRect(x: 0, y: 800, width: 300, height: 20)) - XCTAssert(anchor == .topItem(id: "2", distanceFromTop: 10)) + XCTAssert(anchor == .topItem(id: "2", itemEdge: .top, distanceFromTop: 10)) } func testAnchor_TopToBottom_ScrolledToBottom() throws { @@ -56,11 +58,12 @@ final class TargetContentOffsetAnchorTests: XCTestCase { bottomInset: 30, bounds: CGRect(x: 0, y: 1630, width: 300, height: 400), contentHeight: 2000, + scale: 1, firstVisibleItemID: "6", lastVisibleItemID: "10", firstVisibleItemFrame: CGRect(x: 0, y: 1700, width: 300, height: 20), lastVisibleItemFrame: CGRect(x: 0, y: 1950, width: 300, height: 20)) - XCTAssert(anchor == .topItem(id: "6", distanceFromTop: 20)) + XCTAssert(anchor == .topItem(id: "6", itemEdge: .top, distanceFromTop: 20)) } // MARK: Bottom-to-Top Anchor Tests @@ -72,11 +75,12 @@ final class TargetContentOffsetAnchorTests: XCTestCase { bottomInset: 30, bounds: CGRect(x: 0, y: -50, width: 300, height: 400), contentHeight: 2000, + scale: 1, firstVisibleItemID: "0", lastVisibleItemID: "4", firstVisibleItemFrame: CGRect(x: 0, y: 0, width: 300, height: 20), lastVisibleItemFrame: CGRect(x: 0, y: 290, width: 300, height: 20)) - XCTAssert(anchor == .bottomItem(id: "4", distanceFromBottom: -10)) + XCTAssert(anchor == .bottomItem(id: "4", itemEdge: .bottom, distanceFromBottom: -10)) } func testAnchor_BottomToTop_ScrolledToMiddle() throws { @@ -86,11 +90,12 @@ final class TargetContentOffsetAnchorTests: XCTestCase { bottomInset: 30, bounds: CGRect(x: 0, y: 500, width: 300, height: 400), contentHeight: 2000, + scale: 1, firstVisibleItemID: "2", lastVisibleItemID: "6", firstVisibleItemFrame: CGRect(x: 0, y: 560, width: 300, height: 20), lastVisibleItemFrame: CGRect(x: 0, y: 800, width: 300, height: 20)) - XCTAssert(anchor == .bottomItem(id: "6", distanceFromBottom: -50)) + XCTAssert(anchor == .bottomItem(id: "6", itemEdge: .bottom, distanceFromBottom: -50)) } func testAnchor_BottomToTop_ScrolledToBottom() throws { @@ -100,6 +105,7 @@ final class TargetContentOffsetAnchorTests: XCTestCase { bottomInset: 30, bounds: CGRect(x: 0, y: 1630, width: 300, height: 400), contentHeight: 2000, + scale: 1, firstVisibleItemID: "6", lastVisibleItemID: "10", firstVisibleItemFrame: CGRect(x: 0, y: 1700, width: 300, height: 20), @@ -122,7 +128,7 @@ final class TargetContentOffsetAnchorTests: XCTestCase { } func testOffset_TopToBottom_ScrolledToMiddle() { - let anchor = TargetContentOffsetAnchor.topItem(id: "2", distanceFromTop: 10) + let anchor = TargetContentOffsetAnchor.topItem(id: "2", itemEdge: .top, distanceFromTop: 10) let offset = anchor.yOffset( topInset: 50, bottomInset: 30, @@ -134,7 +140,7 @@ final class TargetContentOffsetAnchorTests: XCTestCase { } func testOffset_TopToBottom_ScrolledToBottom() { - let anchor = TargetContentOffsetAnchor.topItem(id: "2", distanceFromTop: 10) + let anchor = TargetContentOffsetAnchor.topItem(id: "2", itemEdge: .top, distanceFromTop: 10) let offset = anchor.yOffset( topInset: 50, bottomInset: 30, @@ -148,7 +154,7 @@ final class TargetContentOffsetAnchorTests: XCTestCase { // MARK: Bottom-to-Top Target Content Offset Tests func testOffset_BottomToTop_ScrolledToTop() { - let anchor = TargetContentOffsetAnchor.bottomItem(id: "4", distanceFromBottom: -10) + let anchor = TargetContentOffsetAnchor.bottomItem(id: "4", itemEdge: .bottom, distanceFromBottom: -10) let offset = anchor.yOffset( topInset: 50, bottomInset: 30, @@ -160,7 +166,7 @@ final class TargetContentOffsetAnchorTests: XCTestCase { } func testOffset_BottomToTop_ScrolledToMiddle() { - let anchor = TargetContentOffsetAnchor.bottomItem(id: "6", distanceFromBottom: -50) + let anchor = TargetContentOffsetAnchor.bottomItem(id: "6", itemEdge: .bottom, distanceFromBottom: -50) let offset = anchor.yOffset( topInset: 50, bottomInset: 30,