Skip to content

Commit c10ce2f

Browse files
committed
Fix stale binding value reverting panel state during SwiftUI re-renders
Add lastKnownBindingState tracking to CoordinatorProxy.update(state:) to skip moves when the binding value hasn't actually changed, preventing delegate callbacks from reverting the panel to a stale state.
1 parent 686e0f4 commit c10ce2f

2 files changed

Lines changed: 107 additions & 0 deletions

File tree

Sources/SwiftUI/FloatingPanelView.swift

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,9 @@ extension FloatingPanelView {
161161
class FloatingPanelCoordinatorProxy {
162162
private let origin: any FloatingPanelCoordinator
163163
private var stateBinding: Binding<FloatingPanelState?>
164+
/// Tracks the last binding value seen by `update(state:)` to detect actual changes
165+
/// vs. stale values from unrelated SwiftUI re-renders.
166+
private var lastKnownBindingState: FloatingPanelState?
164167

165168
private var subscriptions: Set<AnyCancellable> = Set()
166169

@@ -249,8 +252,10 @@ extension FloatingPanelCoordinatorProxy {
249252

250253
// Update the state of FloatingPanelController
251254
func update(state: FloatingPanelState?) {
255+
defer { lastKnownBindingState = state }
252256
guard
253257
let state = state,
258+
state != lastKnownBindingState,
254259
controller.state != state
255260
else { return }
256261
controller.move(to: state, animated: false)

Tests/SwiftUI/CoordinatorProxyTests.swift

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,3 +139,105 @@ extension CoordinatorProxyTests {
139139
)
140140
}
141141
}
142+
143+
// MARK: - Stale binding guard: prevents revert when unrelated @State triggers re-render
144+
145+
/// When `observeStateChanges()` defers a binding update via `Task { @MainActor }`,
146+
/// a delegate callback (e.g. `didEndAttracting`) can trigger a SwiftUI re-render
147+
/// before that deferred task runs. In that re-render, `update(state:)` receives
148+
/// the OLD binding value. These tests verify that the proxy does not move the panel
149+
/// back to the stale state.
150+
@available(iOS 14, *)
151+
extension CoordinatorProxyTests {
152+
153+
/// Simulates the exact bug scenario:
154+
/// 1. Panel internally reaches `.full` (via drag/attraction)
155+
/// 2. A delegate callback causes a SwiftUI re-render
156+
/// 3. `update(state:)` is called with the stale `.half` binding
157+
/// The panel must NOT revert to `.half`.
158+
func test_updateState_skipsMove_whenBindingIsStaleAfterInternalStateChange() {
159+
let spy = SpyFloatingPanelController()
160+
let proxy = makeProxy(spy: spy)
161+
XCTAssertEqual(spy.state, .half)
162+
163+
// Establish lastKnownBindingState = .half
164+
proxy.update(state: .half)
165+
spy.moveCalls.removeAll()
166+
167+
// Simulate the panel internally moving to .full (e.g. user drag completed)
168+
spy.move(to: .full, animated: false)
169+
spy.moveCalls.removeAll()
170+
171+
// Simulate stale re-render: a delegate callback updates an unrelated @State,
172+
// causing updateUIViewController to be called with the OLD binding value (.half)
173+
proxy.update(state: .half)
174+
175+
XCTAssertTrue(
176+
spy.moveCalls.isEmpty,
177+
"Stale binding value must not cause move(to:), "
178+
+ "but was called \(spy.moveCalls.count) time(s)"
179+
)
180+
XCTAssertEqual(spy.state, .full, "Panel must remain at .full")
181+
}
182+
183+
/// After a stale re-render, the deferred `Task` updates the binding to match
184+
/// the controller's current state. This synced value must not trigger a redundant move.
185+
func test_updateState_skipsRedundantMove_whenDeferredBindingSyncsToControllerState() {
186+
let spy = SpyFloatingPanelController()
187+
let proxy = makeProxy(spy: spy)
188+
XCTAssertEqual(spy.state, .half)
189+
190+
proxy.update(state: .half)
191+
spy.moveCalls.removeAll()
192+
193+
// Panel moves internally to .full
194+
spy.move(to: .full, animated: false)
195+
spy.moveCalls.removeAll()
196+
197+
// Stale re-render (skipped by lastKnownBindingState guard)
198+
proxy.update(state: .half)
199+
XCTAssertTrue(spy.moveCalls.isEmpty)
200+
201+
// Deferred Task finally updates the binding to .full.
202+
// update(state:) is called again with the synced value.
203+
proxy.update(state: .full)
204+
205+
XCTAssertTrue(
206+
spy.moveCalls.isEmpty,
207+
"When binding syncs to controller's current state, no move should occur, "
208+
+ "but was called \(spy.moveCalls.count) time(s)"
209+
)
210+
}
211+
212+
/// After the stale-binding cycle resolves, a new intentional state change
213+
/// (e.g. user taps "Move to tip") must still be applied.
214+
func test_updateState_movesPanel_whenNewStateRequestedAfterStaleCycle() {
215+
let spy = SpyFloatingPanelController()
216+
let proxy = makeProxy(spy: spy)
217+
XCTAssertEqual(spy.state, .half)
218+
219+
proxy.update(state: .half)
220+
spy.moveCalls.removeAll()
221+
222+
// Panel moves internally to .full
223+
spy.move(to: .full, animated: false)
224+
spy.moveCalls.removeAll()
225+
226+
// Stale re-render (skipped)
227+
proxy.update(state: .half)
228+
229+
// Deferred binding sync (no move needed — controller already at .full)
230+
proxy.update(state: .full)
231+
spy.moveCalls.removeAll()
232+
233+
// User requests a new state (e.g. "Move to tip" button)
234+
proxy.update(state: .tip)
235+
236+
XCTAssertEqual(
237+
spy.moveCalls.count, 1,
238+
"A new intentional state change must trigger move(to:)"
239+
)
240+
XCTAssertEqual(spy.moveCalls.first?.state, .tip)
241+
XCTAssertEqual(spy.moveCalls.first?.animated, false)
242+
}
243+
}

0 commit comments

Comments
 (0)