Repository navigation
fix(BottomSheet, Tokenizer, PowerSearch): the scrim returns with a sheet whose swipe dismissal the owner refuses - #10141
Conversation
|
AI review status for this pull request.
|
There was a problem hiding this comment.
Approve. After a drag past the dismiss line that the sheet's owner refuses (the Tokenizer and PowerSearch "Discard changes?" confirmation), the scrim now returns with the sheet. The page stays dimmed while the confirmation is open and after Keep editing. The new hook test fails without the fix and passes with it, and CI's Chromium run of the new browser spec and all required checks pass on this head.
Two optional comment-style suggestions (architecture:knowledge-contracts INV29) are left inline. Neither blocks.
[Automated review]
| // The scrim under the detent a dismissing release left, kept from the | ||
| // release for the host's answer, which arrives as `isOpenRequested` in the | ||
| // render after it: the release itself cannot know whether the host takes | ||
| // the close. |
There was a problem hiding this comment.
Optional, comment style (INV29): this is one 37-word sentence joining three topics, and INV29 keeps a description at 25 words or fewer, one topic per sentence. A shorter version that keeps the reason:
// The release cannot know whether the host takes the close. The host answers
// through `isOpenRequested` in the next render. Keep the left detent's scrim
// until then.| // Each refusal is counted here, in the same render that spends the | ||
| // request, and the layout effect below answers it. |
There was a problem hiding this comment.
Optional, comment style (INV29): passive voice and two topics in one sentence. Suggested:
// The render that spends a refused request counts it. The layout effect below
// answers each count.
PR Analysis Report🔎 Preview ·
|
| Component | Size | LoC | Visual · A11y · RTL |
|---|---|---|---|
| PowerSearch | 260.5 kB (+104 B) · 22.6 kB CSS (0) | — | — · — · — |
| DateInput | 164.1 kB (+89 B) · 15.5 kB CSS (0) | — | — · — · — |
| DateTimeInput | 186.7 kB (+88 B) · 17.2 kB CSS (0) | — | — · — · — |
22 more components
| Component | Size | LoC | Visual · A11y · RTL |
|---|---|---|---|
| TimeInput | 140.9 kB (+80 B) · 14.5 kB CSS (0) | — | — · — · — |
| BottomSheet | 56.6 kB (+55 B) · 7.5 kB CSS (0) | — | ✓ · ✓ · — |
| lab/Comments | 273.8 kB (+2 B) · 2.5 kB CSS (0) | — | — · — · — |
| MultiSelector | 80.3 kB (+1 B) · 15.6 kB CSS (0) | — | — · — · — |
| Popover | 55.4 kB (+1 B) · 12.8 kB CSS (0) | — | — · — · — |
| lab/ChatEmojiPicker | 69.1 kB (+1 B) · 773 B CSS (0) | — | — · — · — |
| BottomSheetSwitcher | 17.4 kB (0) · 3.4 kB CSS (0) | — | ✓ · ✓ · — |
| ContextMenu | 83.2 kB (0) · 16.3 kB CSS (0) | — | — · — · — |
| DropdownMenu | 77.9 kB (0) · 16.4 kB CSS (0) | — | — · — · — |
| Markdown | 153.9 kB (0) · 19.2 kB CSS (0) | — | — · — · — |
| MoreMenu | 78.2 kB (0) · 16.4 kB CSS (0) | — | — · — · — |
| Pagination | 93.2 kB (0) · 16.6 kB CSS (0) | — | — · — · — |
| Selector | 83.2 kB (0) · 16.2 kB CSS (0) | — | — · — · — |
| Table | 92.6 kB (0) · 16.9 kB CSS (0) | — | — · — · — |
| TableCell | 84.4 kB (0) · 16.7 kB CSS (0) | — | — · — · — |
| Tokenizer | 74.8 kB (0) · 17.1 kB CSS (0) | — | — · — · — |
| lab/ChatReactionBar | 69.6 kB (0) · 1.1 kB CSS (0) | — | — · — · — |
| lab/CommentComposer | 109.0 kB (0) · 1.9 kB CSS (0) | — | — · — · — |
| lab/CommentThread | 268.3 kB (0) · 2.4 kB CSS (0) | — | — · — · — |
| lab/TourStep | 61.3 kB (0) · 590 B CSS (0) | — | — · — · — |
| TableHeaderCell | 84.0 kB (−1 B) · 16.5 kB CSS (0) | — | — · — · — |
| richtext/RichTextEditorToolbar | 234.1 kB (−1 B) · 454 B CSS (0) | — | — · — · — |
Package size: @astryxdesign/core 6.58 MB packed (+610 B); no new dependencies.
API changes: none in this pull request
Committed API report behind this build: cli 9 rows, core 8 rows; run pnpm api:report.
Touched components checked; add ci:run-visual, ci:run-a11y or ci:run-rtl to run the full suites.
Generated by PR Enrichment workflow | View full report
…eet whose swipe dismissal the owner refuses
d9a82d0 to
ac9d9fa
Compare
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Approve. After a drag past the dismiss line that the sheet's owner refuses (the Tokenizer and PowerSearch "Discard changes?" confirmation), the scrim now returns with the sheet, so the page stays dimmed while the confirmation is open and after Keep editing. The new hook test fails without the fix and passes with it, and the new browser spec's Chromium cases and the other CI check runs pass on this head.
One optional comment-style suggestion (INV29) is left inline; it does not block.
[Automated review]
| // refused it (a guarded onOpenChange left it open: the sheet snaps back, | ||
| // its scrim with it, and the request is spent). Either way the flag is | ||
| // settled during render, the prev-prop pattern, and derived back to false | ||
| // once the sheet is open again — so a later close from a control, Escape | ||
| // or code is never mistaken for a drag. |
There was a problem hiding this comment.
Miss: optional, comment style (INV29). The first sentence of this comment runs to about 60 words and joins two topics with a semicolon; INV29 keeps a description at 25 words or fewer, one topic per sentence. A shorter version of the whole block (lines 559-567) that keeps the reason:
// Scoped to the one close the release requested. The release calls
// onDismiss() and sets `pending`. The host answers in the same update. If it
// takes the close, its state says closed, and the exit under way is the
// drag's. If it refuses, the sheet and its scrim go back to the detent, and
// the request is spent. Render settles the flag, the prev-prop pattern. The
// flag goes back to false once the sheet is open again. Then no later close
// from a control, Escape or code looks like a drag.useSheetGestures.ts: main's refused-dismissal branch (meta#10141) runs in this branch's closeSheet, for touch and for mouse or pen. A refused request counts itself, and the scrim returns to the detent the drag left. A taken close keeps its drag exit.
…scrim fades with it (#8105) * feat(core): a CSS part times itself from its spring's settle resolveSettleMs gives a spring role's settling time in milliseconds at the tempo of the theme an element is in, so a CSS part that plays with the spring starts and lands with it on one clock. A fixed role, or a spring that is not a role, gives its own. * fix(core): a BottomSheet touch release takes UIKit's sheet values, and turning back cancels A touch drag's release takes UIKit's measured sheet values. It is projected 0.1 s ahead (measured), with closed counted as one position, and a release slower than 250 px/s is not projected (measured). A release at 1,008 px/s or faster is a flick, as UIKit's sheet counts one. It lands on the first position at or past its projection, and at least on the next one. So a short fast flick at the lowest detent closes the sheet instead of springing back. From a higher detent, a flick that 0.1 s of its speed carries past the next detent goes on: a hard flick closes the sheet, and a softer one stops at the next detent (AST-094 FR36). A faster touch release never travels less than a slower one. A touch release moving back toward where the drag began cancels to that detent (AST-094 FR37). The touch release calls the shared release policy, resolveSnapPoint, whose new flickPastProjection option lands a flick at or past its projection. A slow or medium touch throw now travels less than it did, and a slow touch drag closes the sheet only past halfway from the lowest detent to closed. A sheet a touch drag closes holds where the finger left it, then leaves on its close spring (springs.sheetDismissSwipe, UIKit's measured swipe dismissal, which AST-094 FR4 and FR45 keep for a gesture's motion), played by the shared animator from the release speed. Its scrim's fade is decided as the host takes the close, before the scrim starts to clear. It lasts as long as the animator plays that move at the theme's tempo (resolveSettleMs, which now also times one move), so the two finish together (AST-094 FR45's spring tiers, AST-108 FR4a). A reopen during the exit takes it back. Under reduced motion the stylesheet's exit runs. A mouse or pen drag keeps its release: the scroll projection and the dismiss line, with no flick, no close spring and no paced scrim. The BottomSheet record's FR19 adds these touch values to the release rule, with the mouse and pen differences as implementation gaps, and its verification rows merge. AST-094 FR36's BottomSheet sentence names the touch values. DEC-4 and its compatibility notes say so. * Merge branch 'main' into uikit/c17-sheet-release-wiring useSheetGestures.ts: main's refused-dismissal branch (#10141) runs in this branch's closeSheet, for touch and for mouse or pen. A refused request counts itself, and the scrim returns to the detent the drag left. A taken close keeps its drag exit. * Merge branch 'main' into uikit/c17-sheet-release-wiring motion/uikit/index.ts: main's header and exports, with this branch's sheet export. * fix(core): a switcher flow's scrim fades with a touch drag's close A touch drag that closes a BottomSheetSwitcher flow now paces the flow's scrim on the close's own clock, as a standalone sheet's does. The step a drag closes is the flow's retained step, whose backdrop is the scrim, and it takes the panel's fade duration. Both hosts share one writer, which leaves a stack's 0s pin alone and clears only its own value. The scrim's fade now follows the close on one clock: once the close has started, the running fade that clears the scrim takes the close's start time. The close itself is unchanged. Under load a CSS transition and the animator's close otherwise start frames apart. WebKit reports the backdrop's fade on the dialog itself, so where no backdrop effect is reported, the fade on the dialog is taken. sheetDismissSwipe registers itself at import, like every other role row, so the panel no longer calls registerSpringsTable(). * fix(core): a BottomSheet touch drag's close keeps its speed, and its scrim fades with it A sheet a touch drag closes holds where the finger left it, then leaves on its close spring (springs.sheetDismissSwipe, UIKit's measured swipe dismissal), played by the shared animator from the release speed. The spring is a motion role that registers itself, so the theme's tempo scales it. Its scrim fades for as long as that close plays (resolveSettleMs now times one move), and the fade takes the close's start time, so the two end together (AST-094 FR45, AST-108 FR4a). A BottomSheetSwitcher flow's scrim does the same. A reopen during the exit takes the sheet back. Under reduced motion the stylesheet's exit runs. A mouse or pen drag keeps the stylesheet's exit. The touch landing rule (the 0.1 s projection, the flick and the cancel), its tests and its records move to the pull request stacked on this one. * fix(core): FR19 keeps the mouse and pen close gap, and a comment reads as a sentence FR19 states again that a mouse or pen close plays the stylesheet's exit, not the close spring, as an implementation gap against AST-094 FR43 and FR45. The scrim-fade clear's comment becomes a sentence.
User impact
A person editing in the Tokenizer or PowerSearch bottom sheet on a phone (
presentation="bottom-sheet", or the adaptive default on a compact touch screen) changes the draft, then swipes the sheet down to close it. The sheet asks "Discard changes?" — and the page behind the sheet is no longer dimmed: the scrim is gone while the confirmation is open, and it stays gone after Keep editing. The sheet is still modal (the page is inert and takes no input), so what the person sees contradicts what the sheet does. After this change the page stays dimmed behind the sheet the whole time the confirmation is open, Keep editing leaves everything as it was, and Discard closes the sheet with its normal exit.Problem and solution fit
useSheetGestures, the move handler). A release past that line asks the host to close (onDismiss) and writes nothing more to the scrim: when the host takes the close, its own exit owns the backdrop from there. When the host refuses — the Tokenizer and PowerSearch sheets keepisOpentrue and open the discard confirmation instead — the panel returns to its detent (the panel's resting-transform write runs on every commit), but nothing puts the scrim back. The host's own scrim writes run onisOpen,isPresentedand the stack's view, none of which a refusal changes, so the drag's last value, 0, stands.The fix answers the refusal where the hook already recognizes it. The release keeps the scrim opacity of the detent it left (
scrimOpacityForOffsetat the base offset: 1 for a working detent, the peek floor at a peek); the render that spends a refused request (isOpenRequestedstill true while a dismissal is pending) counts it; a layout effect then writes that value back throughonScrimOpacity, in the same commit the panel's resting transform lands, so the scrim returns with the sheet over the sheet's own transition. A dismissal the host takes writes nothing, as before.Scrim tap and Escape never had the problem (no drag, so nothing thinned the scrim); the measurement below confirms it, and the browser spec covers them so the three close paths stay alike.
Expected behavior and authority
component:BottomSheetFR19: aninfosheet whose projection passes the dismiss point closes — through its ownonOpenChange(FR7), withisOpenfully controlled. A controlled owner that keeps the sheet open leaves it presented at its detent; the Scrim part "dims and blocks the page in a scrim-backed host".spec:AST-094(companion motion): a sheet's scrim fade "runs on the pace of the component's main motion: it derives from the main spring's progress and settle time … and never runs on independent timing". A sheet at rest has its scrim at rest.component:TokenizerFR27 andcomponent:PowerSearchFR15c: a swipe on a changed draft opens the discard confirmation; Keep editing closes it and keeps the sheet; Discard closes the sheet.No record changes: the records already state the outcome; the gesture hook did not deliver it on this path.
Not a regression. The gap is as old as
useSheetGestures(the drag-to-scrim mirror and the dismiss branch both date from the file's first commit). A hook-level oracle — a slow drag past the dismiss line whose host keepsisOpentrue, asserting the lastonScrimOpacityis 1 — fails at every sampled main commit:dedecd03(main),8c226246(before #9895),26f5e5d8(before #9708),fbbe8978(before #7648), and9613feb6(#7170). It became visible when a host first refused a swipe: the Tokenizer sheet's discard confirmation (#9708) and PowerSearch's (#9895). #8109, #10022, #10055, #10061, #9882 and #9914 do not change it.Smallest restoration
One gesture-hook change: keep the base detent's scrim value at a dismissing release, count a refused request in the render that spends it, and write the value back in a layout effect. No API, no visual change, no change to the Tokenizer or PowerSearch sheets, to
SheetConfirmation, or to the host's scrim writes. The honored-dismissal path is untouched.Evidence
--_sheet-scrim-opacityand the computed::backdropopacity):var=1, backdrop 1; with the confirmation openvar=0, backdrop 0(panel back at rest); after Keep editingvar=0, backdrop 0— the page behind the sheet undimmed.var=0, backdrop 0with the confirmation open and after Keep editing.var=1, backdrop 1throughout (never affected).opacity: 0; the scrim-tap and Escape cases pass.var=1, backdrop 1with the confirmation open and after Keep editing, for Tokenizer and PowerSearch, in Chromium and WebKit. The new browser spec passes 16/16 across both projects (3 repeats of the WebKit PowerSearch cases too).leaves the thinned scrim alone when the host takes the dismissal). Discard after a swipe closes the sheet with its exit and returns focus to the trigger (browser spec, both components, both engines).useSheetGestures.test.ts—a dismissal the host refuses: a drag past the dismiss line with a host that keepsisOpentrue ends withonScrimOpacity(1)(fails on main with0); from a peek detent the restored value is the peek floor0.3(a fix that wrote1would fail it); a host that takes the close gets no further write.BottomSheetRefusedDismissal.a11y.browser.spec.ts(Chromium and WebKit): for Tokenizer and PowerSearch, after a real-touch swipe, a scrim tap and Escape on a changed draft, the sheet dialog staysopenand:modalwith::backdropopacity 1 and the panel at rest while the confirmation is open, and the same after Keep editing with focus back where each record puts it; Discard after a swipe closes the sheet and returns focus to the trigger.Scope
Testing
pnpm exec vitest run packages/core/src/BottomSheet --project ui— 13 files, 528 tests pass.pnpm storybook:build && pnpm exec playwright test --config playwright.config.ts BottomSheetRefusedDismissal— 16 passed (chromium + webkit).pnpm -F @astryxdesign/core typecheck,eslintandprettier --checkon the changed files,pnpm specs:for --diff origin/main,pnpm review:pre— 0 findings.