diff --git a/.changeset/popover-presentation-hold.md b/.changeset/popover-presentation-hold.md new file mode 100644 index 0000000000000..6f434d0fded43 --- /dev/null +++ b/.changeset/popover-presentation-hold.md @@ -0,0 +1,7 @@ +--- +'@astryxdesign/core': patch +--- + +[chore] Internal: Popover keeps its chosen presentation (the sheet or the anchored layer, the touch grow or the pointer entrance) from the open until its close settles through the layer runtime's one presentation hold (Layer/presentationHold.ts). No behavior change. + +@cixzhang diff --git a/docs/architecture/layer-runtime.md b/docs/architecture/layer-runtime.md index 977331838b27f..75181364b179b 100644 --- a/docs/architecture/layer-runtime.md +++ b/docs/architecture/layer-runtime.md @@ -34,6 +34,7 @@ verified_by: packages/core/src/Layer/useLayerDismissal.test.tsx, packages/core/src/Layer/layerDismissalInvariants.test.tsx, packages/core/src/Layer/layerExitCopy.test.ts, + packages/core/src/Layer/presentationHold.test.tsx, packages/core/src/Layer/__tests__/layerExitCopy.interaction.browser.spec.ts, packages/core/src/Popover/Popover.test.tsx, packages/core/src/DropdownMenu/DropdownMenu.test.tsx, @@ -532,12 +533,12 @@ migration, verification, and completion criteria for these gaps. This record mus be updated only as that work ships. INV14 and INV15 are not met yet, and `spec:AST-003` does not cover them. The -runtime owns the shared exit copy (INV13, `placeLayerExitCopy`); no shared -presentation hold, runtime backdrop tint or lift module exists, Dialog, Drawer, -BottomSheet, Lightbox, MobileNav, ToastViewport, and Selector each build their -own top-layer mounting, exit, or backdrop, Popover builds its own -presentation hold for its sheet presentation, and Popover latches its chosen -anchored presentation until the close settles. +runtime owns the shared exit copy (INV13, `placeLayerExitCopy`) and the shared +presentation hold (`Layer/presentationHold.ts`), which Popover uses for its +sheet and anchored presentations; no runtime backdrop tint or lift module +exists, and Dialog, Drawer, BottomSheet, Lightbox, MobileNav, ToastViewport, +and Selector each build their own top-layer mounting, exit, backdrop, or +presentation hold. ## Change coupling @@ -593,6 +594,8 @@ anchored presentation until the close settles. - `Layer/gestureCounter.ts` owns physical pointer/key gesture identity. - `Layer/layerExitCopy.ts` owns the exit copy a touch presentation's close plays on. +- `Layer/presentationHold.ts` owns the presentation hold: a layer's chosen + presentation, held from the open until its close settles (INV14). - `Layer/layerStack.ts`, `Layer/useLayerDismissal.ts`, and `Layer/LayerDepthContext.tsx` own current registration, presence, ordering, and Escape/platform routing. diff --git a/packages/core/src/Layer/presentationHold.test.tsx b/packages/core/src/Layer/presentationHold.test.tsx new file mode 100644 index 0000000000000..ef2d0a17ea7dd --- /dev/null +++ b/packages/core/src/Layer/presentationHold.test.tsx @@ -0,0 +1,80 @@ +// Copyright (c) Meta Platforms, Inc. and affiliates. + +import fs from 'node:fs'; +import path from 'node:path'; +import {fileURLToPath} from 'node:url'; +import {act, renderHook} from '@testing-library/react'; +import {describe, expect, it} from 'vitest'; +import * as core from '../index'; +import {createPresentationHold, usePresentationHold} from './presentationHold'; + +const here = path.dirname(fileURLToPath(import.meta.url)); + +describe('createPresentationHold', () => { + it('holds what the opening chose until it is released', () => { + const hold = createPresentationHold<'touch' | 'pointer'>(); + expect(hold.isHeld()).toBe(false); + expect(hold.current()).toBeUndefined(); + hold.take('touch'); + expect(hold.current()).toBe('touch'); + expect(hold.isHeld()).toBe(true); + hold.release(); + expect(hold.isHeld()).toBe(false); + expect(hold.current()).toBeUndefined(); + }); + + it('holds a false choice as held', () => { + const hold = createPresentationHold(); + hold.take(false); + expect(hold.isHeld()).toBe(true); + expect(hold.current()).toBe(false); + }); +}); + +describe('usePresentationHold', () => { + it('holds the presentation from the render that opens it, whatever it becomes, until release', () => { + const {result, rerender} = renderHook( + (props: {isOpen: boolean; presentation: string}) => + usePresentationHold(props), + {initialProps: {isOpen: false, presentation: 'sheet'}}, + ); + expect(result.current).toMatchObject({presentation: null, isHeld: false}); + rerender({isOpen: true, presentation: 'sheet'}); + expect(result.current).toMatchObject({presentation: 'sheet', isHeld: true}); + // The screen stops being compact while it is open: the hold keeps the + // opening's choice, and keeps it through the close. + rerender({isOpen: true, presentation: 'anchored'}); + expect(result.current.presentation).toBe('sheet'); + rerender({isOpen: false, presentation: 'anchored'}); + expect(result.current).toMatchObject({presentation: 'sheet', isHeld: true}); + act(() => result.current.release()); + expect(result.current).toMatchObject({presentation: null, isHeld: false}); + // The next opening chooses again. + rerender({isOpen: true, presentation: 'anchored'}); + expect(result.current.presentation).toBe('anchored'); + }); + + it('keeps release stable across renders', () => { + const {result, rerender} = renderHook( + (props: {isOpen: boolean}) => + usePresentationHold({...props, presentation: 1}), + {initialProps: {isOpen: true}}, + ); + const first = result.current.release; + rerender({isOpen: false}); + expect(result.current.release).toBe(first); + }); + + it('stays internal: not exported from the package or the Layer barrel', () => { + expect(core).not.toHaveProperty('usePresentationHold'); + expect(core).not.toHaveProperty('createPresentationHold'); + const barrel = fs.readFileSync(path.join(here, 'index.ts'), 'utf8'); + expect(barrel).not.toMatch(/presentationHold/); + const pkg = JSON.parse( + fs.readFileSync(path.join(here, '../../package.json'), 'utf8'), + ) as {exports: Record}; + expect( + Object.keys(pkg.exports).filter(key => key.includes('presentationHold')), + ).toEqual([]); + }); +}); diff --git a/packages/core/src/Layer/presentationHold.ts b/packages/core/src/Layer/presentationHold.ts new file mode 100644 index 0000000000000..e4b72eb703907 --- /dev/null +++ b/packages/core/src/Layer/presentationHold.ts @@ -0,0 +1,96 @@ +// Copyright (c) Meta Platforms, Inc. and affiliates. + +'use client'; + +/** + * @file presentationHold.ts + * @input Whether a layer is open, and the presentation an opening would + * choose now + * @output createPresentationHold (an imperative hold, for code that decides + * at an open event) and usePresentationHold (the same hold taken during + * render): the presentation the opening chose, held until the component + * releases it + * @position The layer runtime's presentation hold (architecture:layer-runtime + * INV14): keeping a layer's chosen presentation from the open until its + * close settles. Internal to core: not exported from the package or any + * barrel (presentationHold.test.tsx locks that). Not the input hold + * (inputHold.ts), which drops input while a presentation animates. + * + * A layer that can present more than one way (a popover that is a sheet on a + * phone, a touch grow or a pointer entrance) chooses as it opens, and its + * close has to play out in the same presentation even when what it would + * choose has changed meanwhile (a phone rotated past the compact width, a + * pointer that changed). One job: take the choice at the opening, hold it, + * and let it go when the component says its close has settled (its + * ``'s `close` event, its exit animation's end). The component + * decides when that is; the hold decides nothing about timing. + * + * SYNC: When modified, update presentationHold.test.tsx. + */ + +import {useCallback, useState} from 'react'; + +/** An imperative presentation hold. */ +export interface PresentationHold

{ + /** Takes the opening's choice, replacing any held one. */ + take(presentation: P): void; + /** The held choice, or undefined when nothing is held. */ + current(): P | undefined; + /** Whether a choice is held. */ + isHeld(): boolean; + /** Lets the held choice go (the close has settled). */ + release(): void; +} + +/** A presentation hold for code that chooses at an open event. */ +export function createPresentationHold

(): PresentationHold

{ + let held: {readonly value: P} | null = null; + return { + take(presentation) { + held = {value: presentation}; + }, + current: () => held?.value, + isHeld: () => held != null, + release() { + held = null; + }, + }; +} + +/** What `usePresentationHold` returns. */ +export interface PresentationHoldState

{ + /** The opening's choice while it is held; null when nothing is held. */ + readonly presentation: P | null; + /** True from the opening render until `release()`. */ + readonly isHeld: boolean; + /** Lets the held choice go (the close has settled). Stable. */ + readonly release: () => void; +} + +/** + * The presentation hold, taken during render: the render in which `isOpen` + * turns on already holds `presentation` as it is then, and it stays held, + * whatever `presentation` becomes, until `release()`. A component calls + * `release()` when its close has settled (it may do so during render, as it + * would set any of its own state). + */ +export function usePresentationHold

({ + isOpen, + presentation, +}: { + isOpen: boolean; + presentation: P; +}): PresentationHoldState

{ + const [held, setHeld] = useState<{readonly value: P} | null>(null); + let current = held; + if (isOpen && held == null) { + current = {value: presentation}; + setHeld(current); + } + const release = useCallback(() => setHeld(null), []); + return { + presentation: current == null ? null : current.value, + isHeld: current != null, + release, + }; +} diff --git a/packages/core/src/Popover/Popover.tsx b/packages/core/src/Popover/Popover.tsx index 440399040f2d5..77f757bbac9c0 100644 --- a/packages/core/src/Popover/Popover.tsx +++ b/packages/core/src/Popover/Popover.tsx @@ -60,6 +60,7 @@ import * as stylex from '@stylexjs/stylex'; import {devWarn} from '../utils/devWarning'; import type {BaseProps} from '../BaseProps'; import {usePopover} from './usePopover'; +import {usePresentationHold} from '../Layer/presentationHold'; import {usePopoverSurfaceMotion} from './PopoverTouch'; import {FOCUSABLE_SELECTOR} from '../hooks/focusableSelector'; import type {LayerAlignment, LayerPlacement} from '../Layer/useLayer'; @@ -590,18 +591,17 @@ export function Popover({ // A controlled popover opens on the surface its presentation resolves to // as it opens, and keeps that surface until it closes: a phone rotated past // the compact width while the sheet is open keeps its sheet. - const [controlledSurface, setControlledSurface] = useState< - 'popover' | 'bottom-sheet' | null - >(null); - // The latch is set during render, as the controlled `isOpen` turns on or - // off, so the open surface is known in the same render that opens it. - if (isControlled) { - if (isOpen !== true && controlledSurface != null) { - setControlledSurface(null); - } else if (isOpen === true && controlledSurface == null) { - setControlledSurface(popover.isOpen ? 'popover' : resolvedPresentation); - } + // The layer runtime's presentation hold (Layer/presentationHold), taken + // during render as the controlled `isOpen` turns on, so the open surface is + // known in the same render that opens it, and let go as it turns off. + const surfaceHold = usePresentationHold<'popover' | 'bottom-sheet'>({ + isOpen: isControlled && isOpen === true, + presentation: popover.isOpen ? 'popover' : resolvedPresentation, + }); + if (isControlled && isOpen !== true && surfaceHold.isHeld) { + surfaceHold.release(); } + const controlledSurface = surfaceHold.presentation; const isSheetOpen = isControlled ? isOpen === true && controlledSurface === 'bottom-sheet' : isSheetOpenState; @@ -615,12 +615,16 @@ export function Popover({ // stopped being compact, it stays through its exit, so the exit plays out // and its dialog hands focus back to the trigger (component:Popover AR4), // and gives way to the anchored layer once its dialog has closed. - const [hasSheetBeenOpen, setHasSheetBeenOpen] = useState(false); + // The sheet's presentation hold: from the render that opens it until its + // dialog has closed. + const sheetHold = usePresentationHold({ + isOpen: isSheetOpen, + presentation: true, + }); + const hasSheetBeenOpen = sheetHold.isHeld; + const releaseSheet = sheetHold.release; // The trigger's text, which names a sheet that has no `label` (DEC-8). const [triggerText, setTriggerText] = useState(''); - if (isSheetOpen && !hasSheetBeenOpen) { - setHasSheetBeenOpen(true); - } const isSheetMounted = isSheetOpen || hasSheetBeenOpen || @@ -697,14 +701,14 @@ export function Popover({ } const dialog = sheetPanelRef.current?.closest('dialog'); if (dialog == null || !dialog.open) { - // eslint-disable-next-line @eslint-react/set-state-in-effect -- syncs with the sheet's

: it is already closed, so its close event has fired and its exit is over; unmount it - setHasSheetBeenOpen(false); + // The sheet's is already closed, so its close event has fired + // and its exit is over: unmount it. + releaseSheet(); return; } - const handleClose = () => setHasSheetBeenOpen(false); - dialog.addEventListener('close', handleClose); - return () => dialog.removeEventListener('close', handleClose); - }, [hasSheetBeenOpen, isSheetOpen]); + dialog.addEventListener('close', releaseSheet); + return () => dialog.removeEventListener('close', releaseSheet); + }, [hasSheetBeenOpen, isSheetOpen, releaseSheet]); // An open on a compact touch screen shows the sheet at once: its module // loads as soon as the sheet can be the surface, with nothing rendered. useEffect(() => { diff --git a/packages/core/src/Popover/usePopoverSurfaceMotion.ts b/packages/core/src/Popover/usePopoverSurfaceMotion.ts index fa32e2acaa77f..764065ef6dbe6 100644 --- a/packages/core/src/Popover/usePopoverSurfaceMotion.ts +++ b/packages/core/src/Popover/usePopoverSurfaceMotion.ts @@ -25,7 +25,8 @@ * - /packages/core/src/Popover/Popover.test.tsx */ -import {useCallback, useEffect, useRef} from 'react'; +import {useCallback, useEffect, useRef, useState} from 'react'; +import {createPresentationHold} from '../Layer/presentationHold'; import {TOUCH_QUERY} from '../utils/touchPresentation'; import { createPopoverSurfaceMotion, @@ -56,35 +57,42 @@ export function usePopoverSurfaceMotion(): ( motion: PopoverSurfaceMotion; detach: () => void; } | null>(null); - // Whether the presentation in progress is the touch one. - const isTouchPresentationRef = useRef(false); + // Whether the presentation in progress is the touch one, held from the + // open until its close begins (Layer/presentationHold). + const [touchHold] = useState(() => createPresentationHold()); - const motionFor = useCallback((element: HTMLElement) => { - const current = motionRef.current; - if (current?.element === element) { - return current.motion; - } - current?.detach(); - const motion = createPopoverSurfaceMotion(element); - const handleBeforeToggle = (event: Event) => { - if ( - (event as ToggleEvent).newState === 'closed' && - isTouchPresentationRef.current - ) { - motion.exit(); + const motionFor = useCallback( + (element: HTMLElement) => { + const current = motionRef.current; + if (current?.element === element) { + return current.motion; } - }; - element.addEventListener('beforetoggle', handleBeforeToggle); - motionRef.current = { - element, - motion, - detach: () => { - element.removeEventListener('beforetoggle', handleBeforeToggle); - motion.dispose(); - }, - }; - return motion; - }, []); + current?.detach(); + const motion = createPopoverSurfaceMotion(element); + const handleBeforeToggle = (event: Event) => { + if ( + (event as ToggleEvent).newState === 'closed' && + touchHold.current() === true + ) { + motion.exit(); + } + if ((event as ToggleEvent).newState === 'closed') { + touchHold.release(); + } + }; + element.addEventListener('beforetoggle', handleBeforeToggle); + motionRef.current = { + element, + motion, + detach: () => { + element.removeEventListener('beforetoggle', handleBeforeToggle); + motion.dispose(); + }, + }; + return motion; + }, + [touchHold], + ); // Releases what the motion holds on the popover element when the // component unmounts: its `beforetoggle` listener, its compositor @@ -107,7 +115,7 @@ export function usePopoverSurfaceMotion(): ( source != null && typeof view?.matchMedia === 'function' && view.matchMedia(TOUCH_QUERY).matches; - isTouchPresentationRef.current = isTouch; + touchHold.take(isTouch); if (!isTouch || source == null) { // A fine pointer animates nothing here, so nothing is created for // it: no `beforetoggle` listener that outlives the open, and no @@ -123,6 +131,6 @@ export function usePopoverSurfaceMotion(): ( source: boxOf(source), })); }, - [motionFor], + [motionFor, touchHold], ); }