diff --git a/.changeset/flow-height-transitioning.md b/.changeset/flow-height-transitioning.md new file mode 100644 index 00000000000..a845151cc84 --- /dev/null +++ b/.changeset/flow-height-transitioning.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/packages/mosaic/src/primitives/flow/README.md b/packages/mosaic/src/primitives/flow/README.md index 00d3481e83c..dc03168af09 100644 --- a/packages/mosaic/src/primitives/flow/README.md +++ b/packages/mosaic/src/primitives/flow/README.md @@ -80,7 +80,9 @@ The initially active step does not receive `data-starting-style`. An exiting ste `Flow.Root` carries `data-initial` through the first measured frame. Styled adapters can use it to disable viewport transitions so the initial step never animates. -`Flow.Root` carries `data-transitioning` while any step is exiting. Styled adapters should only transition the viewport height under it, so a resize inside the active step (for example a field message animating open) is tracked as-is instead of being animated a second time. +`Flow.Root` carries `data-transitioning` from a step change until its own `height` transition finishes, independent of how long the steps take to enter or exit. If the height does not transition, it clears at once, and other animations on the root are ignored. Styled adapters should only transition the viewport height under it, so a resize inside the active step (for example a field message animating open) is tracked as-is instead of being animated a second time. + +`Flow.Root` carries `data-height-change` set to `grow` or `shrink` after the measured step height changes, so the height transition can use a different curve in each direction. ## CSS variable diff --git a/packages/mosaic/src/primitives/flow/flow-context.ts b/packages/mosaic/src/primitives/flow/flow-context.ts index 0f98c6a5236..e01ce527be8 100644 --- a/packages/mosaic/src/primitives/flow/flow-context.ts +++ b/packages/mosaic/src/primitives/flow/flow-context.ts @@ -8,8 +8,6 @@ export interface FlowContextValue { rootRef: RefObject; registerActiveStep: (element: HTMLElement) => void; unregisterActiveStep: (element: HTMLElement) => void; - registerExitingStep: (element: HTMLElement) => void; - unregisterExitingStep: (element: HTMLElement) => void; } export const FlowContext = createContext(null); diff --git a/packages/mosaic/src/primitives/flow/flow-root.tsx b/packages/mosaic/src/primitives/flow/flow-root.tsx index 4e9dc07d9d3..b9caa9be812 100644 --- a/packages/mosaic/src/primitives/flow/flow-root.tsx +++ b/packages/mosaic/src/primitives/flow/flow-root.tsx @@ -7,6 +7,10 @@ import { type ComponentProps, mergeProps, useRender } from '../utils'; import { autoUpdate, getDimensions } from '../utils/dom'; import { FlowContext, type FlowContextValue, type FlowDirection } from './flow-context'; +function isHeightTransition(animation: Animation): boolean { + return 'transitionProperty' in animation && animation.transitionProperty === 'height'; +} + export interface FlowRootProps extends ComponentProps<'div'> { value: string; direction?: FlowDirection; @@ -18,13 +22,16 @@ export const FlowRoot = React.forwardRef(function const [activeStep, setActiveStep] = useState(null); const [measured, setMeasured] = useState(false); const [initial, setInitial] = useState(true); - const [exitingSteps, setExitingSteps] = useState>(() => new Set()); - const [settling, setSettling] = useState(false); + const [transitioning, setTransitioning] = useState(false); + const lastActiveStepRef = useRef(null); const stepHeightRef = useRef(null); - const runOnRootAnimationsFinished = useAnimationsFinished(rootRef, false); - const exiting = exitingSteps.size > 0; + const runOnHeightTransitionFinished = useAnimationsFinished(rootRef, false, isHeightTransition); const registerActiveStep = useCallback((element: HTMLElement) => { + if (lastActiveStepRef.current && lastActiveStepRef.current !== element) { + setTransitioning(true); + } + lastActiveStepRef.current = element; setActiveStep(element); }, []); @@ -32,21 +39,6 @@ export const FlowRoot = React.forwardRef(function setActiveStep(current => (current === element ? null : current)); }, []); - const registerExitingStep = useCallback((element: HTMLElement) => { - setExitingSteps(current => new Set(current).add(element)); - }, []); - - const unregisterExitingStep = useCallback((element: HTMLElement) => { - setExitingSteps(current => { - if (!current.has(element)) { - return current; - } - const next = new Set(current); - next.delete(element); - return next; - }); - }, []); - useLayoutEffect(() => { if (!activeStep) { return; @@ -65,16 +57,12 @@ export const FlowRoot = React.forwardRef(function }, [activeStep]); useLayoutEffect(() => { - if (exiting) { - setSettling(true); - return; - } - if (!settling) { + if (!transitioning) { return; } - return runOnRootAnimationsFinished(() => setSettling(false)); - }, [exiting, settling, runOnRootAnimationsFinished]); + return runOnHeightTransitionFinished(() => setTransitioning(false)); + }, [transitioning, activeStep, runOnHeightTransitionFinished]); useLayoutEffect(() => { if (!measured || !initial) { @@ -92,10 +80,8 @@ export const FlowRoot = React.forwardRef(function rootRef, registerActiveStep, unregisterActiveStep, - registerExitingStep, - unregisterExitingStep, }), - [value, direction, registerActiveStep, unregisterActiveStep, registerExitingStep, unregisterExitingStep], + [value, direction, registerActiveStep, unregisterActiveStep], ); const element = useRender({ @@ -105,7 +91,7 @@ export const FlowRoot = React.forwardRef(function props: mergeProps<'div'>( { 'data-initial': initial ? '' : undefined, - 'data-transitioning': exiting || settling ? '' : undefined, + 'data-transitioning': transitioning ? '' : undefined, }, otherProps, ), diff --git a/packages/mosaic/src/primitives/flow/flow-step.tsx b/packages/mosaic/src/primitives/flow/flow-step.tsx index bbe6c145361..d837c6c8758 100644 --- a/packages/mosaic/src/primitives/flow/flow-step.tsx +++ b/packages/mosaic/src/primitives/flow/flow-step.tsx @@ -29,15 +29,7 @@ function firstInDocumentOrder(elements: Iterable): HTMLElement | nu export const FlowStep = React.forwardRef(function FlowStep(props, forwardedRef) { const { render, ids, children, ...otherProps } = props; - const { - value, - direction, - rootRef, - registerActiveStep, - unregisterActiveStep, - registerExitingStep, - unregisterExitingStep, - } = useFlowContext(); + const { value, direction, rootRef, registerActiveStep, unregisterActiveStep } = useFlowContext(); const open = ids.includes(value); const stepRef = useRef(null); const activeChildrenRef = useRef(children); @@ -63,18 +55,6 @@ export const FlowStep = React.forwardRef(function return () => unregisterActiveStep(element); }, [open, registerActiveStep, unregisterActiveStep]); - const exiting = mounted && !open; - - useLayoutEffect(() => { - const element = stepRef.current; - if (!exiting || !element) { - return; - } - - registerExitingStep(element); - return () => unregisterExitingStep(element); - }, [exiting, registerExitingStep, unregisterExitingStep]); - // A layout effect, so focus moves in the commit that makes the leaving step inert: field to field, // which iOS does without dropping the keyboard. A passive effect would leave a frame with none. useLayoutEffect(() => { diff --git a/packages/mosaic/src/primitives/flow/flow.test.tsx b/packages/mosaic/src/primitives/flow/flow.test.tsx index 4f853be613d..913543d74fb 100644 --- a/packages/mosaic/src/primitives/flow/flow.test.tsx +++ b/packages/mosaic/src/primitives/flow/flow.test.tsx @@ -239,40 +239,101 @@ describe('Flow', () => { expect(root).not.toHaveAttribute('data-initial'); + expect(root).not.toHaveAttribute('data-height-change'); + rerender(); expect(root.style.getPropertyValue('--cl-flow-step-height')).toBe('240px'); + expect(root).toHaveAttribute('data-height-change', 'grow'); expect(root).not.toHaveAttribute('data-initial'); + + rerender(); + + expect(root).toHaveAttribute('data-height-change', 'shrink'); offsetHeight.mockRestore(); }); - it('marks the root as transitioning only while a step is exiting', async () => { - let finishAnimation!: () => void; - const animationFinished = new Promise(resolve => { - finishAnimation = resolve; + describe('data-transitioning', () => { + function deferred() { + let resolve!: () => void; + const promise = new Promise(r => { + resolve = r; + }); + return { promise, resolve }; + } + + function mockRootAnimations(root: HTMLElement, animations: Array>) { + root.getAnimations = vi.fn(() => animations as unknown as Animation[]); + } + + it('lasts from a step change until the root height transition finishes', async () => { + const height = deferred(); + const { rerender } = render(); + const root = screen.getByTestId('flow-root'); + mockRootAnimations(root, [{ transitionProperty: 'height', finished: height.promise }]); + + rerender(); + + expect(root).not.toHaveAttribute('data-transitioning'); + + rerender(); + + expect(screen.queryByTestId('password-step')).not.toBeInTheDocument(); + expect(root).toHaveAttribute('data-transitioning'); + + mockRootAnimations(root, []); + await act(async () => { + height.resolve(); + await height.promise; + }); + + expect(root).not.toHaveAttribute('data-transitioning'); }); - const { rerender } = render(); - const root = screen.getByTestId('flow-root'); - const outgoingStep = screen.getByTestId('password-step'); - outgoingStep.getAnimations = vi.fn(() => [{ finished: animationFinished }] as unknown as Animation[]); - expect(root).not.toHaveAttribute('data-transitioning'); + it('clears at once when the root height does not transition', () => { + const { rerender } = render(); + const root = screen.getByTestId('flow-root'); + mockRootAnimations(root, []); - rerender(); + rerender(); - expect(root).not.toHaveAttribute('data-transitioning'); + expect(root).not.toHaveAttribute('data-transitioning'); + }); - rerender(); + it('is not held by other animations on the root', () => { + const { rerender } = render(); + const root = screen.getByTestId('flow-root'); + mockRootAnimations(root, [{ animationName: 'pulse', finished: new Promise(() => {}) }]); - expect(root).toHaveAttribute('data-transitioning'); + rerender(); - outgoingStep.getAnimations = vi.fn(() => []); - await act(async () => { - finishAnimation(); - await animationFinished; + expect(root).not.toHaveAttribute('data-transitioning'); }); - expect(screen.queryByTestId('password-step')).not.toBeInTheDocument(); - expect(root).not.toHaveAttribute('data-transitioning'); + it('waits for the latest height transition when the step changes again', async () => { + const first = deferred(); + const second = deferred(); + const { rerender } = render(); + const root = screen.getByTestId('flow-root'); + mockRootAnimations(root, [{ transitionProperty: 'height', finished: first.promise }]); + + rerender(); + mockRootAnimations(root, [{ transitionProperty: 'height', finished: second.promise }]); + rerender(); + await act(async () => { + first.resolve(); + await first.promise; + }); + + expect(root).toHaveAttribute('data-transitioning'); + + mockRootAnimations(root, []); + await act(async () => { + second.resolve(); + await second.promise; + }); + + expect(root).not.toHaveAttribute('data-transitioning'); + }); }); describe('useFlowAutoFocus', () => { diff --git a/packages/mosaic/src/primitives/hooks/__tests__/use-animations-finished.test.ts b/packages/mosaic/src/primitives/hooks/__tests__/use-animations-finished.test.ts index 043a1de12ef..e19a57a23d4 100644 --- a/packages/mosaic/src/primitives/hooks/__tests__/use-animations-finished.test.ts +++ b/packages/mosaic/src/primitives/hooks/__tests__/use-animations-finished.test.ts @@ -5,7 +5,7 @@ import { describe, expect, it, vi } from 'vitest'; import { useAnimationsFinished } from '../use-animations-finished'; function createMockElement( - animations: Array<{ finished: Promise }> = [], + animations: Array<{ finished: Promise; transitionProperty?: string; animationName?: string }> = [], attributes: Record = {}, ): HTMLElement { const el = document.createElement('div'); @@ -120,6 +120,66 @@ describe('useAnimationsFinished', () => { expect(callback).toHaveBeenCalledTimes(1); }); + describe('with a filter', () => { + const neverFinishes = new Promise(() => {}); + const isHeightTransition = (animation: Animation) => + 'transitionProperty' in animation && animation.transitionProperty === 'height'; + + it('fires callback immediately when only unmatched animations are running', () => { + const el = createMockElement([{ animationName: 'pulse', finished: neverFinishes }]); + const ref = { current: el } as RefObject; + + const { result } = renderHook(() => useAnimationsFinished(ref, false, isHeightTransition)); + + const callback = vi.fn(); + act(() => result.current(callback)); + expect(callback).toHaveBeenCalledTimes(1); + }); + + it('waits only for matched animations', async () => { + let resolveHeight!: () => void; + const heightFinished = new Promise(r => { + resolveHeight = r; + }); + const el = createMockElement([ + { transitionProperty: 'height', finished: heightFinished }, + { animationName: 'pulse', finished: neverFinishes }, + ]); + const ref = { current: el } as RefObject; + + const { result } = renderHook(() => useAnimationsFinished(ref, false, isHeightTransition)); + + const callback = vi.fn(); + act(() => result.current(callback)); + expect(callback).not.toHaveBeenCalled(); + + await act(() => resolveHeight()); + expect(callback).toHaveBeenCalledTimes(1); + }); + + it('re-checks only matched animations when one is cancelled', async () => { + let rejectHeight!: () => void; + const cancelledHeight = new Promise((_, reject) => { + rejectHeight = reject; + }); + const el = createMockElement([{ transitionProperty: 'height', finished: cancelledHeight }]); + const ref = { current: el } as RefObject; + + const { result } = renderHook(() => useAnimationsFinished(ref, false, isHeightTransition)); + + const callback = vi.fn(); + act(() => result.current(callback)); + + el.getAnimations = vi.fn(() => [{ animationName: 'pulse', finished: neverFinishes }] as unknown as Animation[]); + await act(async () => { + rejectHeight(); + await new Promise(r => setTimeout(r, 0)); + }); + + expect(callback).toHaveBeenCalledTimes(1); + }); + }); + it('waits for starting-style attribute removal when open=true', async () => { const el = createMockElement([], { 'data-starting-style': '' }); const ref = { current: el } as RefObject; diff --git a/packages/mosaic/src/primitives/hooks/use-animations-finished.ts b/packages/mosaic/src/primitives/hooks/use-animations-finished.ts index 517b975f5f9..3603e428270 100644 --- a/packages/mosaic/src/primitives/hooks/use-animations-finished.ts +++ b/packages/mosaic/src/primitives/hooks/use-animations-finished.ts @@ -14,11 +14,18 @@ import { flushSync } from 'react-dom'; * before polling animations. This avoids a race where `getAnimations()` returns * an empty array before the enter transition has been registered. * + * When `filter` is given, only animations it accepts are waited on, so an + * unrelated (or infinite) animation on the element can't hold the callback. + * * Each call aborts any pending wait from a previous call, so rapid open/close * toggles don't leak stale callbacks, and returns a cancel function for callers * that need to abandon a wait before it resolves. */ -export function useAnimationsFinished(ref: RefObject, open: boolean) { +export function useAnimationsFinished( + ref: RefObject, + open: boolean, + filter?: (animation: Animation) => boolean, +) { const abortRef = useRef(null); useEffect(() => { @@ -42,11 +49,13 @@ export function useAnimationsFinished(ref: RefObject, open: return cancel; } + const getAnimations = () => (filter ? element.getAnimations().filter(filter) : element.getAnimations()); + const runCheck = () => { if (signal.aborted) { return; } - const animations = element.getAnimations(); + const animations = getAnimations(); if (animations.length === 0) { // Called synchronously (from useEffect or MutationObserver) — // plain callback is fine, React will batch the state update. @@ -68,7 +77,7 @@ export function useAnimationsFinished(ref: RefObject, open: } // An animation was cancelled. If new animations are running, wait // for those instead; otherwise we're done. - const current = element.getAnimations(); + const current = getAnimations(); if (current.length > 0) { runCheck(); } else { @@ -95,6 +104,6 @@ export function useAnimationsFinished(ref: RefObject, open: runCheck(); return cancel; }, - [ref, open], + [ref, open, filter], ); }