Repository navigation
feat(lab): BottomSheetStack — stack Core bottom sheets - #7409
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.
|
PR Analysis Report
New ComponentsBottomSheetStack (@astryxdesign/lab)
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 |
1f240cc to
e7c65b4
Compare
Lab host that stacks Core BottomSheet and BottomSheetSwitcher levels in one shared 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 host 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. Core gains one unstable_-prefixed, Lab-only subpath (@astryxdesign/core/BottomSheet/unstable_stack): a host context, a parent-level context, and the level protocol hook. A sheet joins a stack only at the stack's own layer depth, so a sheet opened inside a Core Dialog, AlertDialog, or popover layer stays a standalone sheet above it. Without a host the contexts are null and Core behaves as before. No public prop, type, or barrel export changes.
e7c65b4 to
7c64844
Compare
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.
There was a problem hiding this comment.
Approval is on hold until a maintainer provides or identifies the spec that governs the Lab BottomSheetStack: a new component record, amendments to the BottomSheet records, or the ordered-path direction in draft spec:AST-044, covering tree-order levels in one shared dialog, inert receded covered levels, top-only dismissal and focus return. Independently of that decision, these changes are needed:
- Move
@astryxdesign/core/BottomSheet/unstable_stackbehind the/experimentalboundary that AST-017 FR21 defines (with@experimentaldeclarations,stability: 'experimental'docs and an[experimental]Changeset), or stop exporting it, because a reachable Core subpath is public API whatever its prefix. - Keep a stacked
BottomSheetSwitcher'sref,onKeyDown,onClick,onCancelandaria-labelworking, or warn in development and document that they are dropped. - Keep DOM nodes out of React state in the level protocol (
architecture:react-component-runtimeINV1), and document or warn that content rendered under the stack outside a sheet is not shown. - Split the 5,897 authored lines into separately reviewable pieces, such as the Core seam and the Lab host.
[Automated review]
| "types": "./dist/Markdown/remark.d.ts", | ||
| "default": "./dist/Markdown/remark.js" | ||
| }, | ||
| "./BottomSheet/unstable_stack": { |
There was a problem hiding this comment.
This entry point makes the unstable_ exports public: architecture:public-component-api INV12 treats anything reachable from a published path as public whatever its name, and AST-017 FR21 keeps experimental hooks, functions and types out of the stable promise only on the owning component's /experimental subpath, with @experimental declarations and stability: 'experimental' docs. Suggest ./BottomSheet/experimental with those markers and an [experimental] Changeset, or no export.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [chore] `BottomSheet` and `BottomSheetSwitcher` read internal, `unstable_`-prefixed stack contexts (`@astryxdesign/core/BottomSheet/unstable_stack`) that let an external host present several sheets inside one shared dialog; a sheet under such a host registers itself with it from wherever it is rendered. Without such a host the contexts are null and both components behave exactly as before. The subpath is Lab-only coordination with no stability promise and is not part of the documented API. |
There was a problem hiding this comment.
[chore] files a new Core package entry point as an internal change; AST-017 FR22 expects [experimental] for an explicitly experimental surface (or [feat] if it is meant to be stable).
| isStacked && hasScrim !== undefined, | ||
| ); | ||
|
|
||
| if (stackHost != null) { |
There was a problem hiding this comment.
In this branch the switcher's ref, onKeyDown, onClick and onCancel are never attached, and aria-label is spread onto a role-less div. With identical props the ref stays null (standalone: the DIALOG), both handlers fire 0 times (standalone: once), and the shared dialog is named after the active sheet instead of the caller's label. Please compose the handlers onto the level element and connect or warn about the rest (public-component-api INV5, INV8).
| // The level owns its box so a re-registration (StrictMode, Suspense, | ||
| // Activity) keeps the same portal target and the panel keeps its state. | ||
| // Created on the client only; the server renders no portal. | ||
| const [box] = useState(() => |
There was a problem hiding this comment.
This keeps a DOM node in React state, and the host keeps anchor and box elements in its entries state. architecture:react-component-runtime INV1: 'DOM nodes and whole DOM collections MUST NOT be stored in React state; store only the minimal rendering key or own the node through a ref.'
| ref={slotRef} | ||
| data-bottom-sheet-stack-levels="" | ||
| /> | ||
| <div hidden ref={childrenRegionRef} data-bottom-sheet-stack-children=""> |
There was a problem hiding this comment.
Everything under the stack that is not inside a sheet renders here, hidden inside the dialog, so a trigger button placed directly under the stack is invisible and out of the accessibility tree, with no warning. The doc says wrapper components and DOM wrappers 'all work'; please say that only sheet content is shown (and warn in development), or render this region in place outside the dialog.
|
Closing: superseded by BottomSheet's open-order stacking and same-update replace, merged in #7648. |
User need
Product flows on phones need sheets that open over other sheets. A list opens a detail sheet over it, the detail sheet opens an editor, and Back walks down one level at a time with every level's state intact. Some flows need a sheet over one step of a
BottomSheetSwitcher. TodayBottomSheetpresents one panel andBottomSheetSwitcherreplaces one step with another, so builders coordinate independent sheets by hand. The result is two native dialogs, two scrims, and focus and dismissal split across them.Problem and solution fit
BottomSheetStackin@astryxdesign/labcomposes the existing Core sheets instead of adding a new controller API:BottomSheetandBottomSheetSwitcherrendered anywhere under the stack is one level. That holds directly, behind wrapper components with any prop names, inside DOM wrappers,Suspense,Activityand providers, and in mapped lists. A component that renders two sheets gives two levels. The stack never reads its children.inertand hidden from accessibility, and recede in two visible steps: scale 0.96 / −8px, then 0.92 / −16px.onOpenChange(false)oronActiveSheetChange(null), and the top level'spurposeapplies.hasScrimlives on the stack, which owns one dialog, scrim, modal boundary, scroll lock and dismissal owner. AhasScrimset on a sheet inside the stack is ignored with a development warning.finalFocusRefstays per standalone sheet and applies when that level closes. When every level closes together, the bottom level's ref wins. Without one, focus returns to the element that opened the first level.BottomSheetStackinside another merges into the outer one: one dialog, one scrim, and a development warning.Dialog,AlertDialog, popover or other layer opened from a level opens above the stack, and a sheet inside that layer stays a standalone sheet above it.Current specification
Lab has no spec contract for this component, and none is claimed. The direction is related to the draft
spec:AST-044(#6042), which proposes an ordered path onBottomSheetSwitcher. This PR explores the composition alternative in Lab so the two can be compared in use. It changes no Core spec.Public delta and caller burden
BottomSheetStack/BottomSheetStackProps(children,hasScrim, plusref/onCancel/base props forwarded to the shared dialog). Canary only.BottomSheetandBottomSheetSwitcher, and the export lists of@astryxdesign/coreand@astryxdesign/core/BottomSheet, are unchanged.@astryxdesign/core/BottomSheet/unstable_stack. Its exports areunstable_-prefixed and documented as Lab-only coordination with no stability promise. It isn't in the root barrel, the component index or the docs.nulland Core takes the paths it takes today. No provider is added to any Core component's tree.finalFocusRef.createPortalis ordered by where that portal's content lands in the document.Design and alternatives
useIdkey, a hidden in-place anchor, and the id of the level whose content it's in.{isOpen, isPresented, purpose, label, finalFocusRef, dismiss}.inertall derive from registrations, so the stack never infers anything from element props or shape. Never registering during render keeps uncommitted and StrictMode renders from affecting the stack.inertand recede transform therefore never reach a child stacked above it.MutationObservercovers reorders that run no React callback.inert, andshowModalruns once.Dialogor popover stay standalone, with no edits to those components.isOpen/activeSheetfrom direct children (the first iteration of this PR) drops wrapped sheets and mis-handles empty or multi-sheet children.stackvalue withonStackChangeis the main API alternative; it's tracked by docs(spec): evolve BottomSheetSwitcher with an ordered sheet path #6042.Evidence
mainand on this branch, and the emitted.d.tswere compared.index,BottomSheet/index,Dialog,AlertDialog,LayerandPopoverare identical.BottomSheet,BottomSheetSwitcherandBottomSheetPaneldiffer only in file-header comment lines../BottomSheet/unstable_stack.div,Suspense,ErrorBoundary, provider,Activity, two-sheet component, mapped list);Suspensehide/reveal andActivitywith state kept,useDeferredValue, SSR + hydration);flushSyncbefore any microtask;Dialog,AlertDialog, popover,LightboxorMobileNavopened from a level;matrix(0.96,…,-8)at depth 1 andmatrix(0.92,…,-16)at depth 2.onClickhandlers around the stack still fire.showModal/closeper cycle, withclose()while the shell is still displayed.Scope
Testing
Stories under
Lab/BottomSheetStack:Suspense, a Dialog over the stack)