Repository navigation
Conversation
| c.getLocationOnScreen(location); | ||
| if (rawX < location[0] || rawY < location[1] || rawX >= location[0] + c.getWidth() || rawY >= location[1] + c.getHeight()) continue; | ||
| if (c instanceof ScrollBox && ((ScrollBox) c).scrolls(dx, dy)) return true; | ||
| if (scrollTakes(c, rawX, rawY, dx, dy)) return true; |
There was a problem hiding this comment.
[P2] Check the touched descendant rather than overlapping siblings
With a pressable overlay above a scrolled ScrollBox, this search falls through the overlay and finds the background scroll, preventing the parent pan from intercepting even though that scroll never received ACTION_DOWN; dragging the overlay then moves neither the sheet nor the list. Restrict the handoff to the actual touched descendant/ancestor chain, respecting pointerEvents, and add an overlapping-overlay regression test.
Reviewed by ChatGPT on behalf of @huacnlee.
There was a problem hiding this comment.
You're right, thanks. The search walked every view under the finger, so a scroll view below a button that took the DOWN still counted, and the drag then moved neither the sheet nor the list.
It now counts only a scroll view that took this touch's DOWN: ScrollBox records the down time of the touch it handled, and Box.scrollTakes skips any scroll view that did not take it, with everything inside it. It matches iOS, where pan_touch walks up from the touched view to the pan.
Test: sheet-scroll.test.tsx has a new case with a pressable button over the list: scroll the list, then drag the button down, and the sheet must close. On an Android 15 arm64 emulator it failed before the fix (sheet: closed never showed; on screen "sheet: open", "list: scrolled") and passes with it; the other two cases still pass. I did not add a case for a pass-through overlay.
… can take A swipe-to-close Sheet's pan took every drag that started on its content, so a list in the sheet never scrolled. The pan now leaves a drag to a scroll view under the finger that can move that way (at its top, a drag down still moves the sheet), as iOS shares the touch.
A button over a scrolled list in a sheet took the touch, but the pan still found the list under the finger and left the drag to it, so dragging the button moved neither the sheet nor the list. A scroll view now counts only when it took the touch's DOWN, as iOS looks only at the scroll views between the touched view and the pan.
Description
On Android, a list inside a swipe-to-close
Sheetdoes not scroll. The sheet's pan takes every drag that starts on its content: a drag up does nothing, so a sheet of options, a picker or a share sheet with more rows than fit never shows the rest, and a drag down closes it. iOS handles the same case:Sheetsetspan_scroll_handoff, and the iOS mount lets the pan share the touch with the scroll view under it (pan_touchandpan_shares_scrollinnsh-platform-ios/src/mount.rs; I read this code but did not run it).The fix: when a pan is about to start (
Box.onInterceptTouchEvent, past the touch slop), the pan first looks for a scroll view inside it that took this touch's DOWN and can move in the drag's direction (ScrollBox.took(downTime)andScrollBox.scrolls(dx, dy), fromcanScrollVertically/canScrollHorizontally). If there is one, the drag is left to it. Otherwise the pan starts as before, so at the list's top a drag down still moves the sheet, and a drag on a button over the list moves the sheet too: the button took the touch, so the list under it does not count. iOS also looks only at the scroll views between the touched view and the pan. The change is inBox.javaandScrollBox.java(+41 / −1), plus a regression test (examples/gallery/tests/sheet-scroll.test.tsxand its fixture). No API change, no new dependency.How to Test
The fixture opens a
SheetwithcloseOnSwipeholding a 400 pt list of 40 rows with a button placed over its lower part, and shows whether the sheet is open and whether the list has scrolled. The GIF drags the list up, then down. Before, the drag up does nothing and the drag down closes the sheet; after, the list scrolls down and back, and the sheet stays.The test has three cases: a drag up on the list scrolls it (the bug); at the list's top a drag down closes the sheet (what must keep working); after the list scrolls, a drag down on the button over it closes the sheet (from review). The last two run on Android only: a headless run does not close the sheet on these drags.
--target android(Android 15 arm64 emulator)`text:list: scrolled` showing never happened in 5000 ms(on screen: "sheet: open", "list: at top"); drag down passes; the button case fails at the same step`sheet: closed` showing never happened in 5000 ms(on screen: "sheet: open", "list: scrolled")CI runs no
--target androidjob, so the Android row is a command for the reviewer to run. The runtime was built withcargo xtask android --release --dev --abi arm64-v8a.Not tested: horizontal scroll views inside a pan; pannable views other than
Sheet; a scroll view inside a view placed outside its parent (the Box's overflow dispatch); a physical device; armeabi-v7a and x86_64. iOS is not changed.Reviewer action
One choice is yours. Android never receives
pan_scroll_handoff, so this PR applies the hand-off to every pannable view that has a scroll view under the finger. iOS applies it only where the flag is set (todaySheetwithcloseOnSwipe). I chose the broader rule because a pan that steals a scroll it cannot use looks wrong anywhere. The alternative is to send the flag to Android inencode.rsand check it inBox; tell me and I will switch.There is also a difference you may notice: from the iOS code, one drag there can scroll the list to its top and then move the sheet. Here the choice is made once, when the drag starts, so it takes a second drag to move the sheet.
Checklist
cargo test --workspace,cargo +nightly fmt --alland clippy (including the Catalyst and Android runs for platform code). (cargo test --workspaceon dde7f77 with this commit and the four other Android fixes of this series: 3260 passed, 0 failed, 43 ignored.cargo xtask clippy-androidpasses on main at 201d780, and this PR changes no Rust. The Java is compiled bycargo xtask android, and there is no Rust forfmtto check)tests/compat/(cargo xtask compat). (No API change.)examples/gallerydemos on iOS and Android (if the change is visible or platform-specific). (The test above, on Android.)