From f2a886b75b74b9fecfed1e6db90b227fa52ba319 Mon Sep 17 00:00:00 2001 From: Shin Yamamoto Date: Tue, 28 Jan 2025 21:05:59 +0900 Subject: [PATCH 1/6] Stop pinning the scroll offset in moving programmatically --- Sources/Core.swift | 41 +++++++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/Sources/Core.swift b/Sources/Core.swift index 579763a9..8b8da279 100644 --- a/Sources/Core.swift +++ b/Sources/Core.swift @@ -71,7 +71,7 @@ class Core: NSObject, UIGestureRecognizerDelegate { var removalVector: CGVector = .zero // Scroll handling - private var initialScrollOffset: CGPoint = .zero + private var initialScrollOffset: CGPoint? private var scrollBounce = false private var scrollIndictorVisible = false private var scrollBounceThreshold: CGFloat = -30.0 @@ -411,7 +411,7 @@ class Core: NSObject, UIGestureRecognizerDelegate { if insideMostExpandedAnchor { // Prevent scrolling if needed - if isScrollable(state: state) { + if isScrollable(state: state), let initialScrollOffset = initialScrollOffset { if interactionInProgress { os_log(msg, log: devLog, type: .debug, "settle offset -- \(value(of: initialScrollOffset))") // Return content offset to initial offset to prevent scrolling @@ -429,7 +429,7 @@ class Core: NSObject, UIGestureRecognizerDelegate { stopScrolling(at: initialScrollOffset) } } - } else { + } else if let initialScrollOffset = initialScrollOffset { // Return content offset to initial offset to prevent scrolling stopScrolling(at: initialScrollOffset) } @@ -471,7 +471,8 @@ class Core: NSObject, UIGestureRecognizerDelegate { } if isScrollable(state: state) { // Adjust a small gap of the scroll offset just after swiping down starts in the grabber area. - if surfaceView.grabberAreaContains(location), surfaceView.grabberAreaContains(initialLocation) { + if surfaceView.grabberAreaContains(location), surfaceView.grabberAreaContains(initialLocation), + let initialScrollOffset = initialScrollOffset { stopScrolling(at: initialScrollOffset) } } @@ -499,7 +500,8 @@ class Core: NSObject, UIGestureRecognizerDelegate { } } // Adjust a small gap of the scroll offset just before swiping down starts in the grabber area, - if surfaceView.grabberAreaContains(location), surfaceView.grabberAreaContains(initialLocation) { + if surfaceView.grabberAreaContains(location), surfaceView.grabberAreaContains(initialLocation), + let initialScrollOffset = initialScrollOffset { stopScrolling(at: initialScrollOffset) } } @@ -894,17 +896,6 @@ class Core: NSObject, UIGestureRecognizerDelegate { return true } - func endWithoutAttraction(_ target: FloatingPanelState) { - self.state = target - self.updateLayout(to: target) - self.unlockScrollView() - // The `floatingPanelDidEndDragging(_:willAttract:)` must be called after the state property changes. - // This allows library users to get the correct state in the delegate method. - if let vc = ownerVC { - vc.delegate?.floatingPanelDidEndDragging?(vc, willAttract: false) - } - } - private func startAttraction(to state: FloatingPanelState, with velocity: CGPoint, completion: @escaping (() -> Void)) { os_log(msg, log: devLog, type: .debug, "startAnimation to \(state) -- velocity = \(value(of: velocity))") guard let vc = ownerVC else { return } @@ -934,8 +925,8 @@ class Core: NSObject, UIGestureRecognizerDelegate { self.backdropView.alpha = self.getBackdropAlpha(at: current, with: translation) // Pin the offset of the tracking scroll view while moving by this animator - if let scrollView = self.scrollView { - self.stopScrolling(at: self.initialScrollOffset) + if let scrollView = self.scrollView, let initialScrollOffset = self.initialScrollOffset { + self.stopScrolling(at: initialScrollOffset) os_log(msg, log: devLog, type: .debug, "move -- pinning scroll offset = \(scrollView.contentOffset)") } @@ -958,6 +949,7 @@ class Core: NSObject, UIGestureRecognizerDelegate { private func endAttraction(_ tryUnlockScroll: Bool) { self.isAttracting = false self.moveAnimator = nil + self.initialScrollOffset = nil if let vc = ownerVC { vc.delegate?.floatingPanelDidEndAttracting?(vc) @@ -981,6 +973,19 @@ class Core: NSObject, UIGestureRecognizerDelegate { } } + func endWithoutAttraction(_ target: FloatingPanelState) { + self.initialScrollOffset = nil + + self.state = target + self.updateLayout(to: target) + self.unlockScrollView() + // The `floatingPanelDidEndDragging(_:willAttract:)` must be called after the state property changes. + // This allows library users to get the correct state in the delegate method. + if let vc = ownerVC { + vc.delegate?.floatingPanelDidEndDragging?(vc, willAttract: false) + } + } + func value(of point: CGPoint) -> CGFloat { return layoutAdapter.position.mainLocation(point) } From ea5ecf35e1841210f1d1882cb79329a116f1095c Mon Sep 17 00:00:00 2001 From: Shin Yamamoto Date: Wed, 29 Jan 2025 18:13:56 +0900 Subject: [PATCH 2/6] Add 'optional' string interpolation --- Sources/Core.swift | 2 +- Sources/Logging.swift | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/Sources/Core.swift b/Sources/Core.swift index 8b8da279..77cc0b77 100644 --- a/Sources/Core.swift +++ b/Sources/Core.swift @@ -849,7 +849,7 @@ class Core: NSObject, UIGestureRecognizerDelegate { } else { initialScrollOffset = scrollView.contentOffset } - os_log(msg, log: devLog, type: .debug, "initial scroll offset -- \(initialScrollOffset)") + os_log(msg, log: devLog, type: .debug, "initial scroll offset -- \(optional: initialScrollOffset)") } initialTranslation = translation diff --git a/Sources/Logging.swift b/Sources/Logging.swift index 5c4f3c0f..e813254b 100644 --- a/Sources/Logging.swift +++ b/Sources/Logging.swift @@ -15,3 +15,14 @@ struct Logging { static let category = "FloatingPanel" private init() {} } + +extension String.StringInterpolation { + mutating func appendInterpolation(optional: T?, defaultValue: String = "nil") { + switch optional { + case let value?: + appendLiteral(String(describing: value)) + case nil: + appendLiteral(defaultValue) + } + } +} From 12cec7d1ed4eb9f5615d57742093893cb03d9a7f Mon Sep 17 00:00:00 2001 From: Shin Yamamoto Date: Wed, 29 Jan 2025 18:14:12 +0900 Subject: [PATCH 3/6] Add comments --- Sources/Core.swift | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Sources/Core.swift b/Sources/Core.swift index 77cc0b77..1f764e1c 100644 --- a/Sources/Core.swift +++ b/Sources/Core.swift @@ -949,6 +949,11 @@ class Core: NSObject, UIGestureRecognizerDelegate { private func endAttraction(_ tryUnlockScroll: Bool) { self.isAttracting = false self.moveAnimator = nil + + // We need to reset `initialScrollOffset` because the scroll offset can become unexpected + // under the following circumstances: + // 1. The scroll offset changes while the panel does not move. + // 2. The panel is then moved using `move(to:animate:completion:)`. self.initialScrollOffset = nil if let vc = ownerVC { @@ -974,6 +979,7 @@ class Core: NSObject, UIGestureRecognizerDelegate { } func endWithoutAttraction(_ target: FloatingPanelState) { + // See comments in `endAttraction` self.initialScrollOffset = nil self.state = target From 36472f6a319621e2b5626009dac5b93543029fd0 Mon Sep 17 00:00:00 2001 From: Shin Yamamoto Date: Wed, 29 Jan 2025 20:37:54 +0900 Subject: [PATCH 4/6] Add CoreTests.test_initial_scroll_offset_reset() --- Tests/CoreTests.swift | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/Tests/CoreTests.swift b/Tests/CoreTests.swift index 11201949..fd2f502b 100644 --- a/Tests/CoreTests.swift +++ b/Tests/CoreTests.swift @@ -916,6 +916,34 @@ class CoreTests: XCTestCase { } } + func test_initial_scroll_offset_reset() { + let fpc = FloatingPanelController() + let scrollView = UIScrollView() + fpc.layout = FloatingPanelBottomLayout() + fpc.track(scrollView: scrollView) + fpc.showForTest() + + fpc.move(to: .full, animated: false) + + fpc.panGestureRecognizer.state = .began + fpc.floatingPanel.handle(panGesture: fpc.panGestureRecognizer) + + fpc.panGestureRecognizer.state = .cancelled + fpc.floatingPanel.handle(panGesture: fpc.panGestureRecognizer) + + waitRunLoop(secs: 1.0) + + let expect = CGPoint(x: 0, y: 100) + + scrollView.setContentOffset(expect, animated: false) + + fpc.move(to: .half, animated: true) + + waitRunLoop(secs: 1.0) + + XCTAssertEqual(expect, scrollView.contentOffset) + } + func test_handleGesture_endWithoutAttraction() throws { class Delegate: FloatingPanelControllerDelegate { var willAttract: Bool? From 3eb26c6b7e63ab61d5177b9a412dd8fee5f3af75 Mon Sep 17 00:00:00 2001 From: Shin Yamamoto Date: Thu, 30 Jan 2025 09:59:21 +0900 Subject: [PATCH 5/6] ci: remove macos-12 jobs for the deprecation --- .circleci/config.yml | 25 +++++++++++++++++++++++++ .github/workflows/ci.yml | 21 --------------------- 2 files changed, 25 insertions(+), 21 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 5d2aae3c..bcc849a3 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -1,6 +1,21 @@ version: 2.1 jobs: + build-swift_5_7: + macos: + xcode: 13.4.1 + steps: + - checkout + - run: xcodebuild -scheme FloatingPanel -workspace FloatingPanel.xcworkspace SWIFT_VERSION=5.7 clean build + + build-swiftpm_ios15_7: + macos: + xcode: 13.4.1 + steps: + - checkout + - run: swift build -Xswiftc "-sdk" -Xswiftc "`xcrun --sdk iphonesimulator --show-sdk-path`" -Xswiftc "-target" -Xswiftc "x86_64-apple-ios15.7-simulator" + - run: swift build -Xswiftc "-sdk" -Xswiftc "`xcrun --sdk iphonesimulator --show-sdk-path`" -Xswiftc "-target" -Xswiftc "arm64-apple-ios15.7-simulator" + test-ios14_5-iPhone_12_Pro: macos: xcode: 13.4.1 @@ -8,7 +23,17 @@ jobs: - checkout - run: xcodebuild clean test -scheme FloatingPanel -workspace FloatingPanel.xcworkspace -destination 'platform=iOS Simulator,OS=14.5,name=iPhone 12 Pro' + test-ios15_5-iPhone_13_Pro: + macos: + xcode: 13.4.1 + steps: + - checkout + - run: xcodebuild clean test -scheme FloatingPanel -workspace FloatingPanel.xcworkspace -destination 'platform=iOS Simulator,OS=15.5,name=iPhone 13 Pro' + workflows: test: jobs: + - build-swift_5_7 + - build-swiftpm_ios15_7 - test-ios14_5-iPhone_12_Pro + - test-ios15_5-iPhone_13_Pro diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4f2e3508..dbb14f2f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -27,15 +27,6 @@ jobs: - swift: "5.8" xcode: "14.3.1" runs-on: macos-13 - - swift: "5.7" - xcode: "14.1" - runs-on: macos-12 - - swift: "5.6" - xcode: "13.4.1" - runs-on: macos-12 - - swift: "5.5" - xcode: "13.2.1" - runs-on: macos-12 steps: - uses: actions/checkout@v4 - name: Building in Swift ${{ matrix.swift }} @@ -59,11 +50,6 @@ jobs: sim: "iPhone 14 Pro" parallel: NO # Stop random test job failures runs-on: macos-13 - - os: "15.5" - xcode: "13.4.1" - sim: "iPhone 13 Pro" - parallel: NO # Stop random test job failures - runs-on: macos-12 steps: - uses: actions/checkout@v4 - name: Testing in iOS ${{ matrix.os }} @@ -137,13 +123,6 @@ jobs: - target: "arm64-apple-ios16.4-simulator" xcode: "14.3.1" runs-on: macos-13 - # 15.7 - - target: "x86_64-apple-ios15.7-simulator" - xcode: "14.1" - runs-on: macos-12 - - target: "arm64-apple-ios15.7-simulator" - xcode: "14.1" - runs-on: macos-12 steps: - uses: actions/checkout@v4 - name: "Swift Package Manager build" From fa1627cdd786b9f1dac61481bcee63fb7f88d573 Mon Sep 17 00:00:00 2001 From: Shin Yamamoto Date: Thu, 30 Jan 2025 10:09:20 +0900 Subject: [PATCH 6/6] ci: name circleci jobs --- .circleci/config.yml | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index bcc849a3..05c2d44f 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -16,24 +16,28 @@ jobs: - run: swift build -Xswiftc "-sdk" -Xswiftc "`xcrun --sdk iphonesimulator --show-sdk-path`" -Xswiftc "-target" -Xswiftc "x86_64-apple-ios15.7-simulator" - run: swift build -Xswiftc "-sdk" -Xswiftc "`xcrun --sdk iphonesimulator --show-sdk-path`" -Xswiftc "-target" -Xswiftc "arm64-apple-ios15.7-simulator" - test-ios14_5-iPhone_12_Pro: + test-ios15_5-iPhone_13_Pro: macos: xcode: 13.4.1 steps: - checkout - - run: xcodebuild clean test -scheme FloatingPanel -workspace FloatingPanel.xcworkspace -destination 'platform=iOS Simulator,OS=14.5,name=iPhone 12 Pro' - - test-ios15_5-iPhone_13_Pro: + - run: xcodebuild clean test -scheme FloatingPanel -workspace FloatingPanel.xcworkspace -destination 'platform=iOS Simulator,OS=15.5,name=iPhone 13 Pro' + test-ios14_5-iPhone_12_Pro: macos: xcode: 13.4.1 steps: - checkout - - run: xcodebuild clean test -scheme FloatingPanel -workspace FloatingPanel.xcworkspace -destination 'platform=iOS Simulator,OS=15.5,name=iPhone 13 Pro' + - run: xcodebuild clean test -scheme FloatingPanel -workspace FloatingPanel.xcworkspace -destination 'platform=iOS Simulator,OS=14.5,name=iPhone 12 Pro' + workflows: test: jobs: - - build-swift_5_7 - - build-swiftpm_ios15_7 - - test-ios14_5-iPhone_12_Pro - - test-ios15_5-iPhone_13_Pro + - build-swift_5_7: + name: build (5.7, 13.4.1) + - build-swiftpm_ios15_7: + name: swiftpm ({x86_64,arm64}-apple-ios15.5-simulator, 13.4.1) + - test-ios14_5-iPhone_12_Pro: + name: test (15.5, 13.4.1, iPhone 12 Pro) + - test-ios15_5-iPhone_13_Pro: + name: test (14.5, 13.4.1, iPhone 13 Pro)