Repository navigation
fix(BottomSheet): a sheet opened inside another sheet's content drags to dismiss on its own - #8820
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
meta-cla
Bot
added
the
CLA Signed
PR Analysis Report
|
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
One test is needed before this can be approved; the fix itself works: dragging a nested sheet's grabber now dismisses only that sheet, and the sheet beneath stays open. The new guard in the native touchstart handler (useSheetGestures.ts:1500) is untested: the regression test fires pointer events only, so reverting that line keeps the BottomSheet suite green although a real touch drag then dismisses the covered sheet again. Please add a test in useSheetGestures.test.ts, using the existing touchOn helper, where the touch starts inside a <dialog> appended to the body and expects no preventDefault and no drag.
[Automated review]
| if (!touch) { | ||
| // content while the finger is still down (see onTouchMove). A touch | ||
| // inside a layer this sheet hosts is that layer's, never tracked here. | ||
| if (!touch || isInsideNestedLayer(event.target, body)) { |
There was a problem hiding this comment.
No test reaches this guard: the new regression test uses drag(), which fires pointer events only. With this line reverted to if (!touch) {, all 443 BottomSheet tests still pass, while a real touch swipe on a nested sheet's grabber in Chromium drags and dismisses the host again. A test in useSheetGestures.test.ts using the touchOn helper, with the touch starting inside a <dialog> appended to the body (expect defaultPrevented false and isDragging false), would catch it.
ab485ab to
219e873
Compare
There was a problem hiding this comment.
Recommending approval: dragging a sheet opened from inside another sheet's content now moves and dismisses only that sheet, by touch or pointer, and the sheet beneath it stays put. Each new guard is pinned by a test that fails when that guard is removed. Optionally, the changeset could also mention that a pull inside a Dialog opened from a sheet no longer drags the sheet beneath it, since the same check now covers that case.
[Automated review]
…heet's gestures alone A BottomSheet opened from inside another sheet's content renders its dialog as a descendant of the host's body, so a drag on the nested sheet's handle bubbled into the covered host's body gesture handlers: the host armed a body drag on the pointerdown, promoted it on the first move and took pointer capture, which cancelled the nested handle's drag (lostpointercapture) and dragged the covered host instead — a sheet FR6 says is blocked, by a swipe FR7 gives only to the top sheet. The host's pointer and touch arming now ignores a gesture whose target sits inside a <dialog> between it and the body.
219e873 to
278ae76
Compare
…ayer's touches to it, on the recognizers Main's meta#8820 kept a host sheet's pointer path off a gesture that starts inside a <dialog> rendered in its body (a sheet or a Dialog opened from its content). With the sheet's drag on the gesture recognizers, the body's pans refuse such a touch (isInsideNestedLayer, now scoped to the body); the handle's touches are the sheet's own. The nested-sheet test drives touches on measured boxes; a pull from a nested <dialog> fails without the guard. Also moves a release test off a tie: 150 px in 100 ms projected exactly onto the midpoint between the 200 px detent and gone, where float rounding of the event times decided the outcome; 160 px projects clearly past it.
…ayer's touches to it, on the recognizers Main's meta#8820 kept a host sheet's pointer path off a gesture that starts inside a <dialog> rendered in its body (a sheet or a Dialog opened from its content). With the sheet's drag on the gesture recognizers, the body's pans refuse such a touch (isInsideNestedLayer, now scoped to the body); the handle's touches are the sheet's own. The nested-sheet test drives touches on measured boxes; a pull from a nested <dialog> fails without the guard. Also moves a release test off a tie: 150 px in 100 ms projected exactly onto the midpoint between the 200 px detent and gone, where float rounding of the event times decided the outcome; 160 px projects clearly past it.
…ayer's touches to it, on the recognizers Main's meta#8820 kept a host sheet's pointer path off a gesture that starts inside a <dialog> rendered in its body (a sheet or a Dialog opened from its content). With the sheet's drag on the gesture recognizers, the body's pans refuse such a touch (isInsideNestedLayer, now scoped to the body); the handle's touches are the sheet's own. The nested-sheet test drives touches on measured boxes; a pull from a nested <dialog> fails without the guard. Also moves a release test off a tie: 150 px in 100 ms projected exactly onto the midpoint between the 200 px detent and gone, where float rounding of the event times decided the outcome; 160 px projects clearly past it.
fix(core): a sheet hosting another sheet in its content leaves that sheet's gestures alone
User impact
Anyone on a touch device using a
BottomSheetopened from inside another open sheet's content —component:BottomSheetFR5's stack, as the Tokenizer's list-value sheet opens over LabMobilePowerSearch's value step, or a detail sheet opens from a row inside a sheet. Dragging the top sheet down by its grab handle does not dismiss it. Where the covered host allows swipe dismissal (the stacking stories), the host dismisses instead — and takes the top sheet with it (FR8); where the host does not (apurpose="form"step), the top sheet springs back and nothing closes. Restoring it gives the gesture to the sheet it was made on: the top sheet follows the finger and dismisses; the covered host does not move.Problem and solution fit
Why does it occur? A sheet rendered in another sheet's content renders its
<dialog>as a DOM descendant of the host's body, so the pointer events of a drag on the nested sheet's handle bubble through the host body's gesture handlers (useSheetGestures): the host'sonPointerDownarms a body drag — its scroll chain from the handle up to its body is at the top — and on the first move promotes it, callingbeginDrag, which takes pointer capture on the host's body. Capture moves to the host, the nested handle getslostpointercaptureand cancels its own drag (the hook treats that as the drag losing its grip), and the rest of the gesture drives the host. The nativetouchstartlistener on the host's body arms the same way for a touch inside the nested sheet.Why does it harm the task? The visible, top sheet is the one the person is dragging; a gesture that moves the sheet behind it — a sheet FR6 says is blocked — or moves nothing is a sheet that cannot be dismissed the way every other sheet can.
Why does that harm matter? FR7 gives Escape, a scrim tap, a swipe and a platform close request to the top sheet of a stack only; the swipe was the one path that still reached a covered sheet, through DOM bubbling the stack never saw.
The fix is one predicate at the two points where the host arms a gesture:
isInsideNestedLayer(target, body)walks from the event target up to the body and answers true when a<dialog>lies between — a nested sheet's, or a Dialog's opened from the content. The host's pointer arming (handleBodyPointerDown) and touch arming (the body'stouchstart) then do nothing with that gesture, so the host neither drags nor takes capture, and the nested sheet's own handlers run as they do for a standalone sheet.Expected behavior and authority
component:BottomSheet(current): FR6 — a covered modal sheet stays mounted and is blocked; FR7 — a swipe reaches only the top sheet of a stack, through that sheet's ownonOpenChangeandpurpose; FR8 — a nested sheet is presented only while its host is open.Smallest restoration
useSheetGestures.ts: a new exportedisInsideNestedLayer(12 lines) and a guard inhandleBodyPointerDownand in the body'sonTouchStart. No prop, visual, default or interaction change for a sheet with no layer in its content; no change to the handle, pinch, scroll-handoff or capture logic.Evidence
Chromium 156 headless shell via Playwright, iPhone profile 390×844, a real touch drag through CDP
Input.dispatchTouchEvent(start, ten moves of 42 px, end) from the top sheet's grab handle; static Storybook builds ofmain(before) and this head (after). Sampled per move: the covered host's and the top sheet'stranslateY.lab-mobilepowersearch--touch-flow, Add filter → Assignee → tokenizer trigger (the tokenizer sheet over the value step): host/top per move42/42, 84/42, 126/33, 168/18, 210/9, 252/1, 294/0 … 420/0— the top sheet moves once, loses capture and springs back while the covered value step tracks the finger; at release both dialogs remain (the step ispurpose="form", so it springs back). Instrumented: the top handle seespointerdown,gotpointercapture, onepointermove, thenlostpointercapture. Frames were captured stacked, mid-drag (top sheet at rest) and after release (both open). A drag from the top sheet's body, by contrast, does dismiss it (the native touch path promotes the top sheet first).core-bottomsheetstacking--row-owned-detail(Lisbon over Places): the same handle drag dismisses both sheets — the covered host dismissed and the nested one left with it.lab-mobiletokenizer--touch-flowstandalone: the handle drag dismisses the sheet, as expected.0/42, 0/84, 0/126 … 0/420— the top sheet follows the finger, the host does not move; at release only the value step remains. Frames were captured stacked, mid-drag (top sheet 210 px down over the still-placed step) and after release (tokenizer sheet gone). Stacking control: only Lisbon closes, Places stays. Standalone tokenizer: unchanged.BottomSheet.test.tsx"a sheet opened from inside another sheet's content drags to dismiss by its own handle, and the covered host neither drags nor takes the pointer (FR6, FR7)" — an inner sheet rendered in an outer's content, a handle drag on the inner: the inner'sonOpenChange(false)fires, the outer's does not, only the inner handle takes pointer capture, the outer panel keeps its resting transform. Onmainit fails: the outer'sonOpenChange(false)fires (the host dismissed).useSheetGestures.test.ts"leaves a touch that starts inside a nested<dialog>to that layer" pins the touch path: a native touch pull-down that starts inside a<dialog>in the body is neither prevented nor tracked, while the same pull from the body's own content promotes. With thetouchstartguard reverted it fails (1 failed, 443 passed). The same file also covers the predicate itself (own content, nested dialog, the sheet's own dialog above the body).Scope
isInsideNestedLayeris a module export for its unit test, not a package export).Testing
pnpm exec vitest run packages/core/src/BottomSheet— 10 files, 444 tests pass (the three new ones included); the stacked-drag test fails onmain, and the nested-touch test fails with thetouchstartguard reverted.pnpm -F @astryxdesign/core typecheck— exit 0; strict lint on the three touched files — 0 findings; prettier andgit diff --checkclean.Changeset:
[fix]patch for@astryxdesign/core.