From aec4501e2d539f5881d0fe48a54fc2df02fc148d Mon Sep 17 00:00:00 2001 From: Bryan Keller Date: Wed, 19 Nov 2025 13:00:24 -0800 Subject: [PATCH] Fix targetContentOffsetAnchor calculation --- MagazineLayout/LayoutCore/LayoutState.swift | 10 ++--- .../LayoutStateTargetContentOffsetTests.swift | 43 +++++++++++++++++++ 2 files changed, 47 insertions(+), 6 deletions(-) diff --git a/MagazineLayout/LayoutCore/LayoutState.swift b/MagazineLayout/LayoutCore/LayoutState.swift index 58c9d6c..2bcefb2 100644 --- a/MagazineLayout/LayoutCore/LayoutState.swift +++ b/MagazineLayout/LayoutCore/LayoutState.swift @@ -64,22 +64,20 @@ struct LayoutState { var targetContentOffsetAnchor: TargetContentOffsetAnchor { var visibleItemLocationFramePairs = [ElementLocationFramePair]() for itemLocationFramePair in modelState.itemLocationFramePairs(forItemsIn: bounds) { - // Only consider fully-visible items - guard bounds.contains(itemLocationFramePair.frame) else { continue } visibleItemLocationFramePairs.append(itemLocationFramePair) } visibleItemLocationFramePairs.sort { $0.elementLocation < $1.elementLocation } let firstVisibleItemLocationFramePair = visibleItemLocationFramePairs.first { // When scrolling up, only calculate a target content offset based on visible, already-sized - // cells. Otherwise, scrolling will be jumpy. - modelState.isItemHeightSettled(indexPath: $0.elementLocation.indexPath) + // cells. Otherwise, scrolling will be jumpy. Also, only consider fully-visible items. + modelState.isItemHeightSettled(indexPath: $0.elementLocation.indexPath) && bounds.contains($0.frame) } ?? visibleItemLocationFramePairs.first // fallback to the first item if we can't find one with a settled height let lastVisibleItemLocationFramePair = visibleItemLocationFramePairs.last { // When scrolling down, only calculate a target content offset based on visible, already-sized - // cells. Otherwise, scrolling will be jumpy. - modelState.isItemHeightSettled(indexPath: $0.elementLocation.indexPath) + // cells. Otherwise, scrolling will be jumpy. Also, only consider fully-visible items. + modelState.isItemHeightSettled(indexPath: $0.elementLocation.indexPath) && bounds.contains($0.frame) } ?? visibleItemLocationFramePairs.last // fallback to the last item if we can't find one with a settled height guard diff --git a/Tests/LayoutStateTargetContentOffsetTests.swift b/Tests/LayoutStateTargetContentOffsetTests.swift index 7367ce9..2441158 100644 --- a/Tests/LayoutStateTargetContentOffsetTests.swift +++ b/Tests/LayoutStateTargetContentOffsetTests.swift @@ -65,6 +65,23 @@ final class LayoutStateTargetContentOffsetTests: XCTestCase { XCTAssert(layoutState.targetContentOffsetAnchor == .topItem(id: id, distanceFromTop: 25)) } + func testAnchor_TopToBottom_NoFullyVisibleCells_UsesFallback() throws { + // Create a model state with very large items (500px each) that are taller than the bounds (400px) + let bounds = CGRect(x: 0, y: 250, width: 300, height: 400) + let modelState = modelStateWithLargeItems(bounds: bounds) + let layoutState = LayoutState( + modelState: modelState, + bounds: bounds, + contentInset: UIEdgeInsets(top: 50, left: 0, bottom: 30, right: 0), + scale: 1, + verticalLayoutDirection: .topToBottom) + + // Since no items are fully visible, the fallback should use the first partially visible item + // instead of returning .top or .bottom + let id = layoutState.modelState.idForItemModel(at: IndexPath(item: 0, section: 0))! + XCTAssert(layoutState.targetContentOffsetAnchor == .topItem(id: id, distanceFromTop: -300)) + } + // MARK: Bottom-to-Top Anchor Tests func testAnchor_BottomToTop_ScrolledToTop() throws { @@ -243,6 +260,32 @@ final class LayoutStateTargetContentOffsetTests: XCTestCase { return modelState } + private func modelStateWithLargeItems(bounds: CGRect) -> ModelState { + let modelState = ModelState(currentVisibleBoundsProvider: { bounds }) + let sections = [ + SectionModel( + itemModels: [ + // Create items that are 500px tall, larger than the 400px bounds height + ItemModel(widthMode: .fullWidth(respectsHorizontalInsets: true), preferredHeight: 500), + ItemModel(widthMode: .fullWidth(respectsHorizontalInsets: true), preferredHeight: 500), + ItemModel(widthMode: .fullWidth(respectsHorizontalInsets: true), preferredHeight: 500), + ], + headerModel: nil, + footerModel: nil, + backgroundModel: nil, + metrics: MagazineLayoutSectionMetrics( + collectionViewWidth: bounds.width, + collectionViewContentInset: .zero, + verticalSpacing: 0, + horizontalSpacing: 0, + sectionInsets: .zero, + itemInsets: .zero, + scale: 1)) + ] + modelState.setSections(sections) + return modelState + } + } // MARK: - ItemModel