Repository navigation
feat(core): BottomSheetStack — stack bottom sheets in one dialog - #7503
imdreamrunner wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
Core host that stacks BottomSheet and BottomSheetSwitcher levels in one shared native dialog. Every sheet rendered anywhere under the stack is one level: through wrapper components with any prop names, DOM wrappers, Suspense, Activity, providers, and mapped lists. The stack never reads its children; each level registers itself in a layout effect (useId key, hidden in-place anchor, parent-level id, level-owned portal box) and reports open, presented, purpose, label, finalFocusRef, and dismiss. Order is tree order (anchor document position, a sheet inside a level's content right above it), derived every commit, with a MutationObserver applying memoised reorders before the next paint. Only the top level is interactive; covered levels stay mounted, inert, and receded. Escape, scrim tap, and swipe close the top level through its own callback; a stacked panel ignores gestures that started outside it. A nested stack merges into the outer one with a dev warning. A sheet joins a stack only at the stack's own layer depth, so a sheet opened inside a Dialog, AlertDialog, or popover layer stays a standalone sheet above it. Replaces the Lab prototype (#7409): the Lab-only @astryxdesign/core/BottomSheet/unstable_stack subpath and its unstable_ exports are gone; the host context, parent-level context, registration hook and layer-depth check are internal modules (BottomSheetStackContext.ts), and the stack reuses BottomSheetEdgeTint and focusForLastInput instead of copies. BottomSheetStack and BottomSheetStackProps are exported from @astryxdesign/core/BottomSheet and the package root; no new subpath. Additive only: BottomSheet and BottomSheetSwitcher keep their props, types, and behavior outside a stack, and the existing Core suites pass unedited. Staged behind the AST-044 API decision alongside #6728.
PR Analysis Report
No new or modified components detected. Bundle Size Summary
Accessibility AuditStatus: No accessibility violations detected. Visual Regression44 of 448 shot(s) changed. A change here is a question, not a failure: check whether the after is the
and 24 more. Generated by PR Enrichment workflow | View full report |
735c3d6 to
4c15804
Compare
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.
There was a problem hiding this comment.
Approval is on hold for a maintainer decision: no current spec on main admits stacked bottom sheets, so a maintainer needs to provide or identify the governing spec that chooses between this BottomSheetStack host and BottomSheetSwitcher activeSheets (#6728) and adds the new surface to the overlay-dismissal family membership. Whichever model is chosen, these changes are needed before this can merge:
- Split the 6,017 authored changed lines into pull requests of under 2,000 lines each.
- Host a
hasScrim={false}stack in the browser top layer instead of atz-index: 1000(AST-027 FR7), and focus the bottom level'sfinalFocusRefwhen it closes, as a standalone non-modal sheet does. - Connect or warn about a switcher level's
refandonCancel, forward itsonClickandonKeyDown, and keep level boxes and anchors out of React state (react-component-runtime INV1). - Derive the two latch-only layout-effect states during render, observe memoised reorders of sheets nested inside a level, and add Button's required
labelto the examples inBottomSheetStack.doc.mjs.
[Automated review]
| }, | ||
| dialogNonModal: { | ||
| pointerEvents: 'none', | ||
| zIndex: 1000, |
There was a problem hiding this comment.
A hasScrim={false} stack is shown with show() at z-index: 1000 instead of entering the browser top layer. AST-027 FR7 says a local component must not recreate layering with positioned DOM and page-level z-index, and its gap table already lists this pattern in BottomSheet and BottomSheetSwitcher as a confirmed leak. Route non-modal hosting through Layer or the native top layer, or ship the stack modal-only for now.
| setOrder(next); | ||
| }); | ||
| }); | ||
| observer.observe(region, {childList: true, subtree: true}); |
There was a problem hiding this comment.
Nested levels render their anchors inside their parent's panel, which lives in the slot rather than in this region. So a memoised reorder of sheets inside a level is never observed, and the old top stays interactive until some later commit. Observe the slot subtree as well.
| // Releases the display latch after close(), in the same synchronous pass. | ||
| // eslint-disable-next-line @eslint-react/set-state-in-effect -- releases the display latch from the same layout effect that calls close() | ||
| setIsShellOpen(false); | ||
| if (dialogModeRef.current === 'modal') { |
There was a problem hiding this comment.
When a hasScrim={false} stack closes, this gate skips the bottom level's finalFocusRef along with the trigger, so focus is left on <body>. A standalone non-modal BottomSheet honours finalFocusRef, and the new docs promise it. Apply finalFocusRef whatever the modality, and keep only the trigger capture modal-only.
| const [box] = useState(() => | ||
| host == null || typeof document === 'undefined' | ||
| ? null | ||
| : document.createElement('div'), |
There was a problem hiding this comment.
This keeps a DOM node in React state. architecture:react-component-runtime INV1 says DOM nodes must not be stored in state: own the node through a ref and keep only a rendering key. The host's entries state copies each registration's anchor and box the same way.
| // step's content becomes a level right above this one. The level element | ||
| // is a <div>; the inherited dialog attributes it receives are the same | ||
| // HTML attributes under a different element type. | ||
| const levelProps = props as unknown as HTMLAttributes<HTMLDivElement>; |
There was a problem hiding this comment.
In a stack the switcher's ref, onCancel, onClick and onKeyDown are dropped: the ref stays null and the handlers never run, with no development warning (unlike hasScrim). Compose onClick and onKeyDown onto this level element, and connect or warn about ref and onCancel, per public-component-api INV5 and INV8.
| if (isOpen) { | ||
| // Latch presentation so a later close stays mounted through its exit. | ||
| // eslint-disable-next-line @eslint-react/set-state-in-effect -- latches controlled open state for exit animation | ||
| setIsPresented(true); |
There was a problem hiding this comment.
This layout effect only latches state, and its lint reason names no external system, which react-component-runtime INV3 requires. The standalone sheet's identical latch is fine because it sits in the effect that calls showModal(). Derive the latch during render instead. The same applies to setHasBeenPlaced(true) in BottomSheetStackContext.ts.
| <BottomSheetStack> | ||
| <BottomSheet isOpen={isListOpen} onOpenChange={setIsListOpen} label="Orders"> | ||
| {orders.map(id => ( | ||
| <Button key={id} onClick={() => setOrder(id)}>Order {id}</Button> |
There was a problem hiding this comment.
Button requires label (ButtonProps.label: string), so this example and the Edit button below fail type-checking when copied. Pass label.
User need
Touch flows drill in: a list opens an item, the item opens an editor, a filter
step opens its help. Builders want each step to stay visible beneath the next, so
the person keeps their place and can back out one step at a time, and they want
to express that without hoisting every step's state into one coordinator.
Today each
BottomSheetowns its own native<dialog>. Opening a second sheetover a first one opens a second modal: the first disappears behind a second
scrim, Escape and the scrim are resolved by whichever dialog happens to be
newest, focus return skips the sheet in between, and the two sheets cannot
recede as a stack.
BottomSheetSwitcherkeeps several steps in one dialog butby design shows exactly one at a time (replace, not stack), so a "sheet over a
sheet" has no supported composition.
Why that harms the task: a person loses the context they came from (the list is
gone, not receded), and a wrong Escape or scrim tap closes the whole flow rather
than one step. Why that matters: these are the most common mobile flows the
system is meant to make easy, and every team currently rebuilds the stacking by
hand or avoids it.
Problem and solution fit
BottomSheetStackis one host that stacks everyBottomSheet(
isOpen/onOpenChange) andBottomSheetSwitcher(
activeSheet/onActiveSheetChange) rendered anywhere under it, in oneshared native dialog with one scrim, one modal boundary, one scroll lock and
one dismissal owner. Each sheet keeps its existing controlled API and its own
state; the stack only decides order, which level is interactive, and how
covered levels recede.
Primary delta: the new component and its docs, tests, stories and showcase.
Supporting delta:
BottomSheet,BottomSheetSwitcherand the shared panel gaina third, internal rendering path used only inside a stack. No tagalongs: no
existing prop, type, default or behavior changes outside a stack.
API
BottomSheetStackPropsisBaseProps<HTMLDialogElement>plus:childrenReactNodehasScrimbooleantruerefRef<HTMLDialogElement>onCancel(event: SyntheticEvent<HTMLDialogElement>) => voidpurpose.Behavior
the builder's own wrapper components (whatever their props are called), DOM
wrappers,
Suspense,Activity, context providers, portals and mapped lists.The stack never reads its children or infers anything from props; each level
registers itself in a layout effect with a
useIdkey, so a render that nevercommits registers nothing and StrictMode leaves one entry per level.
sheet rendered inside another level's content directly above that level. Order
is re-derived on every commit; a reorder of memoized subtrees that re-renders
nothing is caught by a
MutationObserverand applied before the next paint.mounted with their scroll position and inputs, and are
inertandaria-hidden; pointer input over a covered level's visible sliver reaches thescrim, never the covered panel.
(
scale(0.96)/-8px,scale(0.92)/-16px); deeper levels share the secondstep. CSS motion only, reduced-motion aware.
purpose. Escape, a scrim tap and a downward swipeeach close only the top level, through that level's own callback
(
onOpenChange(false)oronActiveSheetChange(null)), as the top level'spurposeallows:infoallows all three,formblocks swipe and scrim,requiredblocks all three and the shared dialog takesrole="alertdialog".A stacked panel ignores gestures that started outside it, so a swipe on a
nested child never also drags its parent.
focus to the control in the level beneath that opened it, when it is still
there; otherwise focus enters the new top. When the last level closes, focus
returns to the bottom level's
finalFocusRefif it has one, else to the elementthat opened the first level, with the ring following the last input (no ring
after a pointer close). One
showModal()/close()pair per stack cycle.layer depth becomes a pass-through: its sheets join the outer stack at that
place in the tree, and a development warning says its own props are ignored.
layer depth.
Dialog,AlertDialog, popover-based layers,LightboxandMobileNavalready push their content one layer deeper, so a sheet openedinside one of them is a standalone sheet above that layer, and a stack inside
it is its own stack. The same holds for a
LayerDepthProviderthe builderplaces. No change to those layers.
Current specification
No current record admits stacked bottom sheets yet. The behavior is the subject
of draft spec AST-044 (#6042), which #6728 implements as
activeSheetsonBottomSheetSwitcher. This PR is the other candidate for that decision and doesnot create or amend a record; the owning record and the
overlay-dismissalfamily membership follow the decision.
Public delta and caller burden
Additive only.
BottomSheetStackandBottomSheetStackProps, exported from@astryxdesign/core/BottomSheetand the package root through the existingbarrel. No new package subpath.
BottomSheetandBottomSheetSwitcherprop, type, defaultand behavior outside a stack. Their only new behavior is as a level inside a
stack. The emitted
.d.tsfor both differs frommainonly in file-headercomment lines; the
exportsmap is identical; the existing Core suites passunedited.
@astryxdesign/core/BottomSheet/unstable_stacksubpath and every
unstable_export. The host context, parent-level context,registration hook and layer-depth check are internal modules.
Caller burden: wrap the flow in one
BottomSheetStack; keep each level's openstate where it already lives; set
hasScrimon the stack rather than on a level(a level's
hasScrimis ignored with a development warning); close every levelyourself when the flow is done (there is deliberately no close-all prop, see
below). A single open level behaves like a normal sheet.
Caveats
rendered into a hidden region of the dialog so a wrapper's own output (a
Suspensefallback, a paddeddiv) never shows on the scrim. Put triggersand page content outside the stack; put sheets inside.
rendered into the shared dialog, not where the sheet sits in the markup, so a
font or color set on a wrapper
divdoes not apply to that panel. Reactcontext does, so a
ThemeProvideraround a sheet still applies.builder's own
createPortalis ordered by where that portal's content landsin the document, not by where the portal is called.
the mutation observer before the next paint, one step after React's own
commit; tests that assert order synchronously after such a reorder need an
act()flush.callback, what state to reset), so callers close every level themselves; a
one-update close animates every level out together and returns focus once.
finalFocusRef.BottomSheetSwitcherhasno such prop today, so when a switcher is the bottom level and the stack
closes, focus returns to the element that opened the first level.
Design and alternatives
Chosen model: a host that stacks independently controlled sheets. The stack
owns presentation (one dialog, scrim, modal boundary, scroll lock, dismissal
owner) and derives everything else — the open set, the top, each level's depth,
inert— from what the sheets report. State stays where the data lives, in thecomponents that already own it, and composition stays local: a detail sheet can
live next to the row that opens it, inside the list sheet, with no hoisting.
Tree order rather than open order makes the result deterministic under
deep links and reorders.
Alternative: an ordered path on
BottomSheetSwitcher(#6728, AST-044).activeSheets: ReadonlyArray<string>names the bottom-to-top path of sheet ids;onActiveSheetsChange(nextIds, {reason, dismissedSheetId})reports one-leveldismissal;
finalFocusRefnames the destination after[]. Both candidatesagree on the user-facing rules (top-only interaction, covered levels mounted,
inert and receded, one-level dismissal by
purpose, recorded covered focus,hasScrimkept on the shared dialog). They differ in where the model lives:BottomSheetStack(this PR)BottomSheetSwitcher activeSheets(#6728)isOpensheetIdon every child; invalid or unknown ids are validated and warnedsheetIdcontract)activeSheets = []reasonanddismissedSheetIdisOpenin one update; order is the treeBoth are additive and both keep the standalone paths untouched, so the decision
is about the model builders should learn, not about compatibility.
Rejected inside this design: a global stack keyed by mount order (not
deterministic under deep links and reorders); inferring levels from children
props (breaks through wrappers and renames); a per-level
hasScrim(twomodalities in one dialog); a close-all prop (hides an app decision about which
state resets).
Evidence
concurrent React (transitions that suspend register nothing until commit;
useDeferredValue); the first frame underflushSync(right order, depth andinertwith no microtask); nested stacks merging; tree order includingmemoized reorders; Core layers above the stack staying standalone through the
layer-depth rule (
Dialog,AlertDialog,Popover,Selector,Lightbox,MobileNav); nested-sheet gesture gating, close sequencing and focus rules;the known-bug regressions; and SSR/hydration. The internal context suite shows
the contexts are inert without a stack: a standalone
BottomSheetandBottomSheetSwitcherown their own modal dialog, render no anchor and norecede, and do not warn.
main..d.tsand theexportsmap diffed againstmain(seeTesting).
hit-testing over covered slivers, Escape / scrim / touch swipe each popping one
level, a nested touch swipe closing only the child (pointer capture stays on
the child), one
showModal/closeper cycle, focus return with the ringfollowing the last input, a
Dialogover the stack with a sheet over theDialog, no horizontal overflow and no console errors. Screenshots andmeasurements are attached.
Scope
Testing
API audit (
.d.tsonmainvs this branch, plus theexportsmap): the onlydifferences are the two new files (
BottomSheetStack.d.ts,BottomSheetStackContext.d.ts), the two new names inBottomSheet/index.d.ts,and file-header comment lines in
BottomSheet.d.ts,BottomSheetSwitcher.d.tsand
BottomSheetPanel.d.ts.index.d.tsand every other emitted.d.tsarebyte-identical; the
exportsmap is identical.Real-browser matrix (Chromium, 390×844, touch emulation, static Storybook):
Orders drill-in (open → depth 3 → Escape → swipe → scrim → Close all), deep link
at depth 3, switcher base with help overlay, list with switcher top, mapped file
browser, sheets anywhere (row-owned detail, nested touch swipe and mouse drag,
Escape with a nested child on top,
Dialogover the stack, sheet over theDialog,Suspenselevel), nested stack merging into one dialog with onebackdrop.