Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refine top and bottom item anchor calculation #129

Merged
merged 2 commits into from
May 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 9 additions & 12 deletions .github/workflows/swift.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
name: Swift
name: CI

on:
push:
Expand All @@ -8,17 +8,14 @@ on:

jobs:
build:

runs-on: macos-latest

runs-on: macos-13
strategy:
matrix:
destination: ['platform=iOS Simulator,OS=11.0,name=iPhone 8', 'platform=iOS Simulator,OS=16.2,name=iPhone 14']

xcode:
- '15.0' # Swift 5.9
steps:
- uses: actions/checkout@v2
- name: Build
run: xcodebuild clean build -scheme MagazineLayout
- name: Run tests
run: xcodebuild clean test -project MagazineLayout.xcodeproj -scheme MagazineLayout -destination "name=iPhone 14,OS=16.2"

- uses: actions/checkout@v4
- name: Build
run: xcodebuild clean build -scheme MagazineLayout -destination "generic/platform=iOS Simulator"
- name: Run tests
run: xcodebuild clean test -project MagazineLayout.xcodeproj -scheme MagazineLayout -destination "name=iPhone 14,OS=17.2"
68 changes: 14 additions & 54 deletions MagazineLayout/LayoutCore/Types/TargetContentOffsetAnchor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ import UIKit
enum TargetContentOffsetAnchor: Equatable {
case top
case bottom
case topItem(id: String, itemEdge: ItemEdge, distanceFromTop: CGFloat)
case bottomItem(id: String, itemEdge: ItemEdge, distanceFromBottom: CGFloat)
case topItem(id: String, distanceFromTop: CGFloat)
case bottomItem(id: String, distanceFromBottom: CGFloat)

static func targetContentOffsetAnchor(
verticalLayoutDirection: MagazineLayoutVerticalLayoutDirection,
Expand Down Expand Up @@ -65,37 +65,19 @@ enum TargetContentOffsetAnchor: Equatable {
return .top
case .inMiddle, .atBottom:
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))
}
let distanceFromTop = firstVisibleItemFrame.minY - top
return .topItem(
id: firstVisibleItemID,
distanceFromTop: distanceFromTop.alignedToPixel(forScreenWithScale: scale))
}
case .bottomToTop:
switch position {
case .atTop, .inMiddle:
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))
}
let distanceFromBottom = lastVisibleItemFrame.maxY - bottom
return .bottomItem(
id: lastVisibleItemID,
distanceFromBottom: distanceFromBottom.alignedToPixel(forScreenWithScale: scale))
case .atBottom:
return .bottom
}
Expand All @@ -121,44 +103,22 @@ enum TargetContentOffsetAnchor: Equatable {
case .bottom:
return maxYOffset

case .topItem(let id, let itemEdge, let distanceFromTop):
case .topItem(let id, let distanceFromTop):
guard let indexPath = indexPathForItemID(id) else { return bounds.minY }
let itemFrame = frameForItemAtIndexPath(indexPath)
let proposedYOffset = itemFrame.value(for: itemEdge) - topInset - distanceFromTop
let proposedYOffset = itemFrame.minY - topInset - distanceFromTop
return min(max(proposedYOffset, minYOffset), maxYOffset) // Clamp between minYOffset...maxYOffset

case .bottomItem(let id, let itemEdge, let distanceFromBottom):
case .bottomItem(let id, let distanceFromBottom):
guard let indexPath = indexPathForItemID(id) else { return bounds.minY }
let itemFrame = frameForItemAtIndexPath(indexPath)
let proposedYOffset = itemFrame.value(for: itemEdge) - bounds.height + bottomInset - distanceFromBottom
let proposedYOffset = itemFrame.maxY - bounds.height + bottomInset - distanceFromBottom
return min(max(proposedYOffset, minYOffset), maxYOffset) // Clamp between minYOffset...maxYOffset
}
}

}

// 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:
return minY
case .bottom:
return maxY
}
}

}

// MARK: - Position

private enum Position {
Expand Down
3 changes: 1 addition & 2 deletions MagazineLayout/Public/MagazineLayout.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1261,9 +1261,8 @@ 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: insetBounds) {
for itemLocationFramePair in modelState.itemLocationFramePairs(forItemsIn: bounds) {
visibleItemLocationFramePairs.append(itemLocationFramePair)
}
visibleItemLocationFramePairs.sort { $0.elementLocation < $1.elementLocation }
Expand Down
16 changes: 8 additions & 8 deletions Tests/TargetContentOffsetAnchorTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ final class TargetContentOffsetAnchorTests: XCTestCase {
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", itemEdge: .top, distanceFromTop: 10))
XCTAssert(anchor == .topItem(id: "2", distanceFromTop: 10))
}

func testAnchor_TopToBottom_ScrolledToBottom() throws {
Expand All @@ -63,7 +63,7 @@ final class TargetContentOffsetAnchorTests: XCTestCase {
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", itemEdge: .top, distanceFromTop: 20))
XCTAssert(anchor == .topItem(id: "6", distanceFromTop: 20))
}

func testAnchor_TopToBottom_SmallContentHeight() throws {
Expand Down Expand Up @@ -95,7 +95,7 @@ final class TargetContentOffsetAnchorTests: XCTestCase {
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", itemEdge: .bottom, distanceFromBottom: -10))
XCTAssert(anchor == .bottomItem(id: "4", distanceFromBottom: -10))
}

func testAnchor_BottomToTop_ScrolledToMiddle() throws {
Expand All @@ -110,7 +110,7 @@ final class TargetContentOffsetAnchorTests: XCTestCase {
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", itemEdge: .bottom, distanceFromBottom: -50))
XCTAssert(anchor == .bottomItem(id: "6", distanceFromBottom: -50))
}

func testAnchor_BottomToTop_ScrolledToBottom() throws {
Expand Down Expand Up @@ -158,7 +158,7 @@ final class TargetContentOffsetAnchorTests: XCTestCase {
}

func testOffset_TopToBottom_ScrolledToMiddle() {
let anchor = TargetContentOffsetAnchor.topItem(id: "2", itemEdge: .top, distanceFromTop: 10)
let anchor = TargetContentOffsetAnchor.topItem(id: "2", distanceFromTop: 10)
let offset = anchor.yOffset(
topInset: 50,
bottomInset: 30,
Expand All @@ -170,7 +170,7 @@ final class TargetContentOffsetAnchorTests: XCTestCase {
}

func testOffset_TopToBottom_ScrolledToBottom() {
let anchor = TargetContentOffsetAnchor.topItem(id: "2", itemEdge: .top, distanceFromTop: 10)
let anchor = TargetContentOffsetAnchor.topItem(id: "2", distanceFromTop: 10)
let offset = anchor.yOffset(
topInset: 50,
bottomInset: 30,
Expand All @@ -184,7 +184,7 @@ final class TargetContentOffsetAnchorTests: XCTestCase {
// MARK: Bottom-to-Top Target Content Offset Tests

func testOffset_BottomToTop_ScrolledToTop() {
let anchor = TargetContentOffsetAnchor.bottomItem(id: "4", itemEdge: .bottom, distanceFromBottom: -10)
let anchor = TargetContentOffsetAnchor.bottomItem(id: "4", distanceFromBottom: -10)
let offset = anchor.yOffset(
topInset: 50,
bottomInset: 30,
Expand All @@ -196,7 +196,7 @@ final class TargetContentOffsetAnchorTests: XCTestCase {
}

func testOffset_BottomToTop_ScrolledToMiddle() {
let anchor = TargetContentOffsetAnchor.bottomItem(id: "6", itemEdge: .bottom, distanceFromBottom: -50)
let anchor = TargetContentOffsetAnchor.bottomItem(id: "6", distanceFromBottom: -50)
let offset = anchor.yOffset(
topInset: 50,
bottomInset: 30,
Expand Down