Simultaneous gesture support#457
Conversation
| } | ||
|
|
||
| // SKIP DECLARE: suspend fun PointerInputScope.detectSimultaneousDragGestures(onDrag: (PointerInputChange, Offset) -> Unit, onDragEnd: () -> Unit, onDragCancel: () -> Unit, shouldAwaitTouchSlop: () -> Boolean) | ||
| func detectSimultaneousDragGestures(onDragEnd: () -> Void, onDragCancel: () -> Void, onDrag: (PointerInputChange, Offset) -> Void, shouldAwaitTouchSlop: () -> Bool) { |
There was a problem hiding this comment.
This function doesn't look like it should need "SKIP DECLARE". Can you explain why it was added? There's also an oddity with it: the ordering of the SKIP DECLARE parameters are onDrag:onDragEnd:onDragCancel:shouldAwaitTouchSlop:, but the order of the actual function is onDragEnd:onDragCancel:onDrag:shouldAwaitTouchSlop:.
There was a problem hiding this comment.
//SKIP DECLARE is needed because the generated Kotlin must be a suspend fun PointerInputScope...; without it, Skip emits a plain function, so awaitEachGesture, awaitFirstDown, viewConfiguration, etc. lose their receiver context.
I will resubmit after fixing the parameters order
There was a problem hiding this comment.
Couldn't you just declare detectSimultaneousDragGestures async in order to generate the suspend fun?
There was a problem hiding this comment.
That's cleaner but after I made the change to async, the drag observer Android unit test fails.
I am also investigating a new unrelated issue that I think may have been introduced by a recent merge? Unresolved reference 'rememberSaveablePreferenceCollector'
I need to spend a little more time sorting it out, but I should be posting an update tomorrow
There was a problem hiding this comment.
I am also investigating a new unrelated issue that I think may have been introduced by a recent merge? Unresolved reference 'rememberSaveablePreferenceCollector'
Yes, this got broken from a PR collision. It should be fixed in #464.
There was a problem hiding this comment.
Submitting a new version with a fix of the unrelated Unresolved reference 'rememberSaveablePreferenceCollector' (can't build without the fix) + brief comments to explain //SKIP DECLARE Vs async.
I kept the //SKIP DECLARE in place because it is slightly more robust and can be unit-tested on roboelectric while the async version cannot. This is very much a tradeoff decision, I am happy using async and removing the unit test if that's your preference
Here is my current understanding:
....with // SKIP DECLARE we get
suspend fun PointerInputScope.detectSimultaneousDragGestures(...) {
awaitEachGesture {
...
}
}
kotlin generated from async:
internal suspend fun PointerInputScope.detectSimultaneousDragGestures(...): Unit = Async.run {
awaitEachGesture {
...
}
}Notice the Async.run wrapper, the main difference between the 2 flavors
This is the failing unit test (With the async flavor, the test failure is dragChangeCount.value is still 0):
rule.waitForIdle()
rule.onNodeWithTag("simultaneous.scroll.content").performTouchInput { swipeUp() }
rule.waitForIdle()
XCTAssertGreaterThan(dragChangeCount.value, 0)In a sandbox SkipFuse app running on a real device, dragChangeCount.value does reflect the changed values and everything is working as designed. In the roboelectric unit test, the value is never updated even after I tried inserting more delays to simulate a human interaction.
I am not entirely sure why the behaviors diverge (possible Compose / Bridging ?), but going with async would essentially mean we would lose the ability to detect regressions via unit-tests (if there is a smarter way to unit test, I could not figure it out)
There was a problem hiding this comment.
I agree that keeping it testable is worth the "SKIP DECLARE" ugliness. There's probably some way we could get the Robolectric test to work (e.g., with some explicit mechanism to wait for the await call), but I won't hold up this PR for it.
Support simultaneous gesture observers alongside normal gestures on Android, including .none mask behavior for disabled observers. Fix conditional TabView rendering and titleless NavigationStack top-bar spacing. Update Skip dependencies and add Compose-focused regression tests.
f7484f2 to
c269fa3
Compare
Summary
View.simultaneousGestureso tap gestures and drag observers can run alongside an existing gesture instead of replacing it.GestureMask.nonehandling so a simultaneous gesture can be explicitly disabled.API Shape
Usage Example
Limitations
GestureMask.all,.gesture, and.subviewsare not yet full SwiftUI parity.swift testcurrently fails in unrelated Android snapshot rendering tests (CanvasTests.testZStackMultiOpacityOverlay,LayoutTests.testRotatedSquareAliased,LayoutTests.testRotatedSquare), so PR validation used focused simultaneous-gesture Robolectric tests.Testing
swift test --filter testSimultaneousgradle testDebug --project-dir .build/plugins/outputs/skip-ui/SkipUITests/destination/skipstone --tests "skip.ui.SkipUITests.testSimultaneous*"Active horizontal rail swipe the tile rail sideways, then drag vertically on it. The rail should still scroll horizontally, while the simultaneous drag observer updates the Active changed, Active ended, translation counter, and the small handle position.
Vertical scroll observer drag the vertical row list. The list should keep scrolling normally while the simultaneous drag observer updates the Scroll changed, Scroll ended, and translation counters.
.none gesture mask rail drag the second tile rail. It uses the same drag observer with including: .none, so the rail can still be interacted with, but the .none counters should stay at zero.
video-capture-20260603-133314-stream.mp4