Repository navigation
feat(core): BottomSheet replace handoff on open-order stacking (prototype, stacked on #7648) - #7712
Closed
imdreamrunner wants to merge 2 commits into
Closed
imdreamrunner wants to merge 2 commits into
imdreamrunner wants to merge 2 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
|
AI review status for this pull request.
|
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Thanks for the thorough prototype. The measured handoff tracks BottomSheetSwitcher closely, but approval waits on a maintainer deciding whether a standalone BottomSheet should implicitly treat the top sheet closing while one other sheet opens in the same update as a replace. That decision also needs to name the spec record that will own the rule, because the BottomSheet and BottomSheetSwitcher records are both still draft. Whatever the decision, these points need changes:
- The non-modal path orders the two dialogs with a page-level
z-index: 1001, which AST-027 rules out; keeping non-modal sheets out of replace pairing would resolve it. BottomSheet.doc.mjsstill sends multi-step flows toBottomSheetSwitcher(thehasScrimprop doc and both descriptions), while the new paragraph says no switcher is needed.- The new
setIsCloseSettled(false)reset in a layout effect names no external system, and the animation frames thatwriteScrimInstantlyschedules are not cancelled on unmount.
[Automated review]
| // A non-modal sheet replacing another must paint above it whatever their | ||
| // document order; show() dialogs have no top layer to order them. | ||
| dialogNonModalReplacing: { | ||
| zIndex: 1001, |
There was a problem hiding this comment.
This orders two non-modal show() dialogs with a page-level number: during a replace the incoming dialog computes z-index: 1001 against the outgoing dialog's 1000, both position: fixed. AST-027 (DEC-1, FR4, FR8) keeps z-index local and rules out a larger number as a substitute for top-layer promotion. Keeping non-modal sheets out of replace pairing until their hosting moves to the top layer would avoid adding a second page band.
| // eslint-disable-next-line @eslint-react/set-state-in-effect -- latches controlled open state for exit animation | ||
| setIsPresented(true); | ||
| // eslint-disable-next-line @eslint-react/set-state-in-effect -- the next close starts unsettled again | ||
| setIsCloseSettled(false); |
There was a problem hiding this comment.
This reset runs in an Effect, but its reason does not name an external system or a DOM measurement (react-component-runtime INV3). It also costs an extra synchronous render after every open. Resetting isCloseSettled during render when isOpen turns true, or keying the flag to the close it settles, avoids both.
| if (scrimDurationRestoreRef.current != null) { | ||
| cancelAnimationFrame(scrimDurationRestoreRef.current); | ||
| } | ||
| scrimDurationRestoreRef.current = requestAnimationFrame(() => { |
There was a problem hiding this comment.
These chained frames are cancelled only by the next write; nothing cancels them on unmount (react-component-runtime INV5). Cancelling scrimDurationRestoreRef in the registration effect's cleanup would close that.
| anatomy, | ||
| description: | ||
| "A mobile touch surface for filters, actions, forms, and detail views that should rise from the bottom of the viewport; use BottomSheetSwitcher for multi-step flows that replace one step with the next, and open a second sheet over the first when a step should stay visible beneath it.\n\nStacking. Every sheet keeps its own isOpen. Open a sheet while another scrim sheet is open and it stacks on top: the sheet beneath stays mounted with its scroll position and inputs, recedes behind the new one (the nearest covered sheet shrinks to 0.96 and lifts 8px, deeper ones to 0.92 and 16px), and is blocked from pointer, keyboard, and assistive technology by the top sheet's modal dialog. The sheets can be siblings, live in separate components, sit inside another sheet's content, or sit inside a BottomSheetSwitcher step; a whole switcher is one level, and switching its step in place does not change its position. Nothing moves in the DOM: each sheet's dialog stays where it is written, and no wrapper component is needed.\n\nOrder is open order. The top sheet is the one presented most recently that is still open; a sheet that closes and reopens comes back on top. When several sheets open in one update (a deep link into a flow), there is no open order between them, so tree order decides: a sheet below the sheets rendered in its content, siblings in the order they are written. That order is right in the first painted frame.\n\nOne scrim. Only the bottom sheet's backdrop dims the page; the backdrops of the sheets above it are transparent, so a stack never darkens per level. The dim holds steady while sheets push and pop, and if the bottom sheet closes while sheets remain above it, the next sheet's backdrop takes over in the same frame.\n\nDismissal. Escape, a tap on the scrim, a downward swipe, and a platform close request (Android back, a close watcher) reach only the top sheet, through that sheet's own onOpenChange, and that sheet's purpose applies: required blocks all of them, form blocks the scrim and the swipe. Nothing closes a whole stack at once; set every sheet closed in one update when a flow is done and they leave together.\n\nClosing a covered sheet from code. Sheets rendered inside the closing sheet's content are part of it: they are presented only while it is open, and when it closes their onOpenChange(false) is called and they leave with it. Sheets above it that are not in its content stay open and keep their order; the closing sheet slides out from beneath them, and if it owned the scrim, the sheet above takes it over.\n\nFocus. Opening a sheet moves focus into it (a data-autofocus descendant first). Closing the top sheet returns focus into the sheet it reveals: to the control that opened it when that control is still there, otherwise to the revealed sheet's panel. When the last sheet closes, the standalone rules apply: finalFocusRef, else the element that opened the sheet, with a focus ring only after keyboard input.\n\nCaveats. A Dialog, AlertDialog, or Popover opened from a sheet paints above the sheet and does not make it recede; a sheet opened from inside that Dialog paints above the Dialog and stacks over the sheet beneath. Sheets with hasScrim={false} stay out of the stack entirely. A sheet React hides (a suspended Suspense boundary, Activity mode=\"hidden\") leaves the stack and closes its dialog; when it is shown again it is presented again, on top. Only sheets rendered in the same document stack; a sheet inside an iframe is its own stack.", | ||
| "A mobile touch surface for filters, actions, forms, and detail views that should rise from the bottom of the viewport; use BottomSheetSwitcher for multi-step flows that replace one step with the next, and open a second sheet over the first when a step should stay visible beneath it.\n\nStacking. Every sheet keeps its own isOpen. Open a sheet while another scrim sheet is open and it stacks on top: the sheet beneath stays mounted with its scroll position and inputs, recedes behind the new one (the nearest covered sheet shrinks to 0.96 and lifts 8px, deeper ones to 0.92 and 16px), and is blocked from pointer, keyboard, and assistive technology by the top sheet's modal dialog. The sheets can be siblings, live in separate components, sit inside another sheet's content, or sit inside a BottomSheetSwitcher step; a whole switcher is one level, and switching its step in place does not change its position. Nothing moves in the DOM: each sheet's dialog stays where it is written, and no wrapper component is needed.\n\nOrder is open order. The top sheet is the one presented most recently that is still open; a sheet that closes and reopens comes back on top. When several sheets open in one update (a deep link into a flow), there is no open order between them, so tree order decides: a sheet below the sheets rendered in its content, siblings in the order they are written. That order is right in the first painted frame.\n\nOne scrim. Only the bottom sheet's backdrop dims the page; the backdrops of the sheets above it are transparent, so a stack never darkens per level. The dim holds steady while sheets push and pop, and if the bottom sheet closes while sheets remain above it, the next sheet's backdrop takes over in the same frame.\n\nDismissal. Escape, a tap on the scrim, a downward swipe, and a platform close request (Android back, a close watcher) reach only the top sheet, through that sheet's own onOpenChange, and that sheet's purpose applies: required blocks all of them, form blocks the scrim and the swipe. Nothing closes a whole stack at once; set every sheet closed in one update when a flow is done and they leave together.\n\nClosing a covered sheet from code. Sheets rendered inside the closing sheet's content are part of it: they are presented only while it is open, and when it closes their onOpenChange(false) is called and they leave with it. Sheets above it that are not in its content stay open and keep their order; the closing sheet slides out from beneath them, and if it owned the scrim, the sheet above takes it over.\n\nReplacing a sheet. Close the top sheet and open another in the same update (one state value that names the open step does this) and the new sheet replaces it instead of stacking on it: the new sheet rises above the one it replaces; if the replaced sheet is taller it moves down to meet the new sheet's top edge at the same time; the replaced sheet fades only after both motions finish, and is inert and hidden from assistive technology from the start. The scrim holds steady throughout, with no dip; focus moves into the new sheet; and when the flow closes, focus returns to the element that opened its first sheet, whichever sheet closed last. This is the handoff BottomSheetSwitcher runs between its steps, so a multi-step flow needs no switcher: give each step its own BottomSheet and one state value. A replace happens only when the closing sheet was the top sheet of its stack and exactly one sheet closes and one opens in the update; a covered sheet closing while a sheet opens, or a close and an open in different updates, keep the ordinary push and pop motion. A replace over an open stack runs one level up and leaves the sheets beneath covered. Non-modal sheets (hasScrim={false}) replace each other the same way, without a scrim.\n\nFocus. Opening a sheet moves focus into it (a data-autofocus descendant first). Closing the top sheet returns focus into the sheet it reveals: to the control that opened it when that control is still there, otherwise to the revealed sheet's panel. When the last sheet closes, the standalone rules apply: finalFocusRef, else the element that opened the sheet, with a focus ring only after keyboard input.\n\nCaveats. A Dialog, AlertDialog, or Popover opened from a sheet paints above the sheet and does not make it recede; a sheet opened from inside that Dialog paints above the Dialog and stacks over the sheet beneath. Sheets with hasScrim={false} stay out of the stack entirely. A sheet React hides (a suspended Suspense boundary, Activity mode=\"hidden\") leaves the stack and closes its dialog; when it is shown again it is presented again, on top. Only sheets rendered in the same document stack; a sheet inside an iframe is its own stack.", |
There was a problem hiding this comment.
The new "Replacing a sheet" paragraph says a multi-step flow needs no switcher. This description still opens with "use BottomSheetSwitcher for multi-step flows that replace one step with the next", and the dense description (line 399) and the hasScrim doc (line 168) say the same. Whichever way the decision goes, all three should agree.
This was referenced Oct 20, 2026
A modal BottomSheet opened while another modal sheet is open stacks on top of it: the covered sheet stays mounted with its state, recedes (0.96 / -8px, then 0.92 / -16px) and is blocked; only the bottom sheet's backdrop dims the page; Escape, a scrim tap, a swipe and a platform close request pop only the top sheet through its own callback. Order is open order wherever the sheets are written (siblings, separate components, inside another sheet's content, inside a BottomSheetSwitcher step; a whole switcher is one level). Sheets opened in one commit are ordered by tree, right in the first painted frame, through a private commit-phase registry that reconciles the native top layer. A sheet rendered in a closing sheet's content closes with it. No public API change; a sheet on its own behaves exactly as before. Candidate C for the AST-044 stacked-sheet decision, alongside #6728 and #7503.
imdreamrunner
force-pushed
the
feat/sheet-replace-on-open-order
branch
from
October 20, 2026 22:55
e191db2 to
2d85819
Compare
imdreamrunner
force-pushed
the
feat/core-bottom-sheet-open-order
branch
from
October 20, 2026 22:55
ebb29cd to
0c62409
Compare
3 tasks done
imdreamrunner
force-pushed
the
feat/core-bottom-sheet-open-order
branch
from
October 20, 2026 23:05
0c62409 to
308556e
Compare
Contributor
Author
|
Folded into #7648, which now carries open-order stacking, the replace handoff, and the BottomSheetSwitcher deprecation as one change. |
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Primary intent
Decide whether
BottomSheetSwitcheris still needed once sheets stack by open order (#7648). The switcher's one job the stacking model does not cover is the replace handoff between steps: the entering step rises above the retained one, a taller retained step moves down to the entering step's top edge at the same time, the retained step fades only after both motions complete, it is inert and hidden from assistive technology at once, there is one scrim throughout with no dip, focus moves into the new step, and closing returns focus to the flow's opener (component:BottomSheetSwitcherFR3 / ORD1 / FR4). Under C alone,setA(false); setB(true)in one update reads as a push plus a pop.The other templates do not fit: nothing is broken (
bug-fix), the capability is a prototype of an existing component's behaviour on a different structure (new-featurewould imply a decision), and pixels are deliberately unchanged (visual-update).User or maintainer impact
Builders: a multi-step flow becomes one
BottomSheetper step plus one state value naming the open step — noBottomSheetSwitcher, nosheetId. Nothing changes for a sheet on its own, for stacked sheets, or for switcher flows (the switcher stays supported and is one stack level, as in C).Maintainers: the answer to the deprecation question, with the exact cost of achieving it on per-step dialogs, and the hardening an independent verification of the registry asked for.
Problem and solution fit
Mechanism chosen — (a), no new API. The private stack registry (
bottomSheetStack.ts) recognises a replace and runs the switcher's handoff on the two standalone dialogs.The rule. A replace is a move the person makes from one sheet to the next:
showModal()cannot have moved focus yet) is in the closing sheet's dialog, or nowhere (a control that left with the step). A close and an open driven from a control outside the closing sheet are two unrelated owners acting in one update: they keep their own openers and their own push and pop motion.Condition 5 is the answer to the verification report's C7 case (two owners in one tree; a page control closes X's sheet and opens Y's) — the only case where pairing was wrong, because B then adopted A's opener and closing B returned focus to X's control. Why this condition and not another:
replaces/ a flow key): rejected as long as a rule with no API covers every real flow. Every step flow passes condition 5 by construction: Continue and Back live in the step being left; a step change made by code after a delay (a save that resolves) passes too, because under a modal sheet focus cannot be anywhere but inside it. Kept in reserve.document.activeElementat pairing time: wrong on a reversal (Back while the previous step is fading), because the returning sheet's layout effect runs first in tree order and itsshowModal()moves focus before the pairing is seen. Hence the actor is captured at the update's first flip.The handoff itself is the switcher's state machine, ported into the registry and driven by the panels' existing motion reports:
covered→ (entering starts; a taller retained sheet getsaligningwith the measured offset) → both complete →fading→ fade complete → the replaced dialog closes and the replacing sheet takes the scrim in the same frame. The replaced sheet isinert/aria-hiddenat once, stands down from the dismissal stack (so Escape and a platform close reach the replacing sheet even when the replaced one registered later), and lends its opener to the replacer. A reversal mid-handoff swaps the roles; a dismissal mid-handoff drops the retained sheet at once, as the switcher does.Non-modal sheets (
hasScrim={false}) do not replace. The first version ordered the twoshow()dialogs with a page-levelz-index: 1001, which AST-027 rules out (z-index is local only; no larger number as a substitute for top-layer promotion). Until non-modal hosting moves to the top layer (the migration gap AST-027 already records for the switcher'sshow()path), a non-modal step change is a plain exit and entrance. The switcher still covers that one case alone, and its deprecation says so.Alternatives for the mechanism, rejected: (b) a prop on
BottomSheet— see above; (c) a helper hook or component over standalone sheets — it would need the same registry-level facts (who is top, who owns the scrim, the top-layer order), a second coordinator next to C's, and cannot stop per-step dialogs fighting on backdrops and focus without the registry's cooperation anyway; letting the replacer own the scrim from the start — its backdrop paints above and dims the retained panel; cross-fading the two backdrops — dips to ~44% of the dim halfway; deferring the reconcile to a microtask — breaks C's first-frame deep-link guarantee.Public API: none. No prop, type, default or export changes. Before/after callsites for a flow:
Caller burden added: knowing that a same-update close+open of the top sheet, made from inside it, is a replace (the new "Replacing a sheet" paragraph of the Stacking docs).
Authority
component:BottomSheetSwitcher(draft) FR1, FR3, FR4, FR6, ORD1 define the handoff reproduced here.family:overlay-dismissal(current): both sheets stay in the one shared dismissal stack; a replaced sheet answersisPresentfalse so one Escape reaches the one interactive sheet (FR2/FR3 of the family).architecture:layer-runtime(current): native-dialog hosting unchanged.spec:AST-027(current): no page-level z-index; the non-modal path stays out of the replace.Owning record for the replace rule:
component:BottomSheet. Both sheet records are stilldraft; the rule is a property of the standalone sheet and belongs in its contract, not the switcher's (which it supersedes) and not a new record (no distinct fact boundary). Proposed claim text, to be added toBottomSheet.spec.mdin a spec PR once the A/B/C decision picks this model (no spec PR is opened by this prototype):Change and evidence
Files (delta over C):
bottomSheetStack.ts(+357/−11: batch flips and actor,detectReplace, the replace state machine,reportBottomSheetMotion, replaced sheets excluded from the replacer's recede depth, scrim owner = bottom-most open or retained sheet),BottomSheet.tsx(+84/−14: retained/entering panel state from the registry's view, held open pose until the registry answers, motion reports, dismissal stand-down while replaced, opener handover),BottomSheetReplace.test.tsx(new, 32 tests),BottomSheet.doc.mjs("Replacing a sheet" in Stacking; multi-step guidance points at plain sheets),apps/storybook/stories/BottomSheetReplace.stories.tsx(new: the four switcher stories rebuilt from standalone sheets, same content, labels, heights and steps), a[feat]Changeset. 1,728 lines added, 44 removed.Hardening (from the independent verification of the registry)
globalThisunderSymbol.for('astryx.bottomSheetStack'); a copy of another shape keeps its own.ownerDocument; a sheet portaled into another document has its own stack, top layer and focus.layerStack,modalOutlet,useScrollLock); a context cannot reach a second root, a second package copy or a widget outside it. The shared dismissal stack has the same two-copies problem and is a follow-up outside these PRs.Review findings resolved
show()dialogs with a page-levelz-index: 1001(AST-027).BottomSheet.doc.mjsstill sent multi-step flows to the switcher (thehasScrimdoc, both descriptions, a best practice, the example).@deprecatedwording on the switcher's own surfaces.check:knowledgesetIsCloseSettled(false)reset in a layout effect with no external system.isOpenresets it during render (the previous-value pattern), and only the layout effect that follows the registry's answer sets it.writeScrimInstantlyscheduled were not cancelled on unmount.writeScrimOpacitywith a per-host pending handle; hosts cancel it in an effect cleanup.Parity, measured
Chromium (Playwright 1.61, Chrome Headless Shell 149) at 390×844 with touch against the static Storybooks of
main(BottomSheetSwitcherstories) and this branch (BottomSheetReplacestories). Per switch, every animation frame: each panel's computedtransformandopacity, every dialog's::backdropopacity (combined into the page dim),elementFromPointon each panel, inert /aria-hidden,document.activeElement, the nativeshowModal/show/closelog, mid-handoff and settled screenshots with a pixel diff. Timings are relative to the click; the two builds can sit one frame apart purely from where the click fell in the frame, so deltas are read with a ±1 frame (≈17 ms) tolerance. "Back-during-handoff" is Back pressed 150 ms into 1→2.Chromium, default motion
Chromium,
prefers-reduced-motion: reduce(every motion completes in one frame on both builds; the dim must still hold at 0.5)Before this revision the reduced-motion case failed: the stylesheet's literal
0.01sreduced-motion duration overrode the inline0spin, and the page dim dropped to 0 for one or two frames on every switch (mid-switch screenshots differed by 124k–207k px). Measured on the previous head of this PR (e191db295):WebKit (Playwright's WebKit 26.5 in its container, same harness). Headless WebKit's first frames after a click arrive 50–150 ms apart and vary between runs, so per-frame motion cannot be compared there; the main-vs-main noise floor below shows deltas of the same size as main-vs-branch. What WebKit does establish: the page dim constant at 0.5 on every switch, settled screenshots identical (0 px) on every pair, mid-handoff screenshots identical on 12 of 16 modal switches, focus and inertness identical.
WebKit noise floor,
mainagainst itself with the same harness:Dismissal on the active step (step 2), after reopening with focus on the opener (Chromium; reduced motion and WebKit agree):
No regressions in C. C's eight
Core/BottomSheetStackingstories driven identically on C's head and on this branch (open, drill in up to three levels, Escape pop with the dim sampled per frame, "Close all" where offered, and the owner-closes-under-others scrim handoff on the out-of-tree-order story): every state summary, screenshot (0 px), call log and dim range identical. Existing tests pass unedited: the validation set goes from 88 files / 2509 tests on C to 89 / 2537 here (+1 file, +28 tests — the new suite; the three registry-state tests live in C's suite).Deltas remaining, with root causes
show()into each new step wheremainleaves it onbody. Until non-modal hosting reaches the top layer,hasScrim={false}flows keep the switcher (which stays supported); the deprecation says so.close()+showModal(), two forced layouts) and runs one extra synchronous render in the same task. Absent without alignment travel (capped: 0.0 px).API audit
Core built on C's head and on this branch; every emitted
.d.tsand theexportsmap compared.exportsidentical.BottomSheet/BottomSheet.d.tsdiffers only in doc comments.BottomSheet/bottomSheetStack.d.ts(internal; not reachable through the exports map or any public entry) gains the view fieldsreplacedMotion,alignmentOffset,isReplacing, the typesBottomSheetReplacedMotion,BottomSheetReplaceMotion,BottomSheetReplaceable, the optionalreplaceableregistration field andreportBottomSheetMotion. Public additions: none.If #7719 (CSS scroll lock) and/or #7720 (per-step switcher dialogs) land on
mainfirstNoted, not implemented. #7719: the registry does not touch scroll locking;
useScrollLock(shouldPresent && hasScrim)in both hosts is the only coupling, and the handoff keeps the lock held through the whole switch (both dialogs presented), so a CSS lock changes nothing here beyond the rebase. #7720: a switcher whose steps each own a dialog is no longer one dialog to register; C's "a whole switcher is one level" would become "the active step's dialog is the flow's level" — the switcher would register the active step's dialog (re-pointinggetElement/show/hideon each handoff, with the same key so the rank is kept) and this PR's replace would need to exclude switcher-owned step dialogs from pairing, or the switcher could drop its own handoff machinery and let its steps be ordinary sheets that replace through this registry — at which point #7720's prototype and this PR converge, and the switcher becomes a thin controller overisOpen.Scope
Testing
Verdict
Yes, for modal flows; the no-scrim flow is the one gap. On top of C, a switcher flow rebuilt from standalone sheets is pixel-identical at rest and mid-handoff, frame-identical in motion and timing (within one frame), identical in scrim behaviour (one steady dim, no dip, also under reduced motion), inertness, hit-testing, dismissal and focus — for modal flows, which is what
BottomSheetSwitcheris used for. The replace is recognised without a new API by a rule that excludes the unrelated-owners case the verification found. The switcher can be deprecated now (the stacked PR does it) and removed in a later minor once non-modal hosting has moved to the top layer, which is the one case the switcher still covers alone.