From eef23e0d90007744cbe00c18b1813d0b458876c6 Mon Sep 17 00:00:00 2001 From: Pete Steyert-Woods Date: Thu, 15 Oct 2026 15:25:36 +0100 Subject: [PATCH] fix: unmount overlays when their exit animation is cancelled Cancelling an animation rejects its finished promise, so Promise.all in useAnimation rejected and onEnd never ran, leaving exiting overlays mounted. Wait for every promise to settle instead, as #9772 did. Settling on cancellation means an interrupted run (e.g. an overlay reopened while exiting) could end a later one, which the effect cleanup removed in #10674 used to prevent. Track the current run in a ref instead of restoring that cleanup, since StrictMode re-runs the effect on mount and the isActiveRef guard would skip restarting it. Closes #10772 Co-Authored-By: Claude Opus 5.5 --- .../react-aria-components/test/Dialog.test.js | 19 ++++ packages/react-aria/src/utils/animation.ts | 19 +++- .../react-aria/test/utils/animation.test.tsx | 87 +++++++++++++++++++ 3 files changed, 121 insertions(+), 4 deletions(-) create mode 100644 packages/react-aria/test/utils/animation.test.tsx diff --git a/packages/react-aria-components/test/Dialog.test.js b/packages/react-aria-components/test/Dialog.test.js index 3c28c938367..a4b730c7194 100644 --- a/packages/react-aria-components/test/Dialog.test.js +++ b/packages/react-aria-components/test/Dialog.test.js @@ -525,6 +525,25 @@ describe('Dialog', () => { expect(overlay).not.toBeInTheDocument(); }); + it('unmounts when the exit animation is cancelled', async () => { + let {getByRole, getByTestId} = render(); + + await user.click(getByRole('button')); + let overlay = getByTestId('overlay'); + + let reject; + let finished = new Promise((_, r) => (reject = r)); + Element.prototype.getAnimations = () => [{playState: 'running', finished}]; + + await user.click(getByRole('button', {name: 'Close'})); + expect(overlay).toBeInTheDocument(); + expect(overlay).toHaveAttribute('data-exiting'); + + // Cancelling an animation rejects its finished promise. + await act(async () => reject(new DOMException('The animation was cancelled', 'AbortError'))); + expect(overlay).not.toBeInTheDocument(); + }); + it('calls the latest onEnter and onExit callbacks', async () => { let enterPromise = deferred(); let exitPromise = deferred(); diff --git a/packages/react-aria/src/utils/animation.ts b/packages/react-aria/src/utils/animation.ts index 174c105a0ed..2439ab1d7dc 100644 --- a/packages/react-aria/src/utils/animation.ts +++ b/packages/react-aria/src/utils/animation.ts @@ -111,6 +111,7 @@ function useAnimation( onEnd: () => void ): void { let isActiveRef = useRef(null); + let currentRun = useRef(null); let start = useEffectEvent(onStart); useLayoutEffect(() => { if (isActive && ref.current && isActiveRef.current !== isActive) { @@ -135,15 +136,25 @@ function useAnimation( return; } - Promise.all([startPromise, ...animations.map(a => a.finished)]) - .then(() => { + // Cancelled animations reject their finished promise, so wait for every promise to settle + // rather than resolve. Otherwise onEnd never runs and an exiting overlay never unmounts. + // Ignore a run that was interrupted (e.g. an overlay reopened while exiting) so it can't end a + // later one. This can't be an effect cleanup: StrictMode re-runs the effect on mount, and the + // isActiveRef guard above would skip restarting it. + let run = {}; + currentRun.current = run; + Promise.allSettled([startPromise, ...animations.map(a => a.finished)]).then(() => { + if (currentRun.current === run) { flushSync(() => { onEnd(); }); - }) - .catch(() => {}); + } + }); } + if (!isActive) { + currentRun.current = null; + } isActiveRef.current = isActive; }, [ref, isActive, onEnd]); } diff --git a/packages/react-aria/test/utils/animation.test.tsx b/packages/react-aria/test/utils/animation.test.tsx new file mode 100644 index 00000000000..bfe49cf6ade --- /dev/null +++ b/packages/react-aria/test/utils/animation.test.tsx @@ -0,0 +1,87 @@ +/* + * Copyright 2026 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ + +import {act, render} from '@react-spectrum/test-utils-internal'; +import React, {StrictMode, useRef} from 'react'; +import {render as renderWithoutWrapper} from '@testing-library/react'; +import {useEnterAnimation, useExitAnimation} from '../../src/utils/animation'; + +function deferred() { + let resolve!: () => void; + let promise = new Promise(r => (resolve = r)); + return {promise, resolve}; +} + +describe('animation', () => { + beforeEach(() => { + // jsdom skips animations unless getAnimations is implemented. + Element.prototype.getAnimations = () => []; + }); + + afterEach(() => { + // @ts-expect-error + delete Element.prototype.getAnimations; + }); + + it('finishes an enter animation that starts on mount in StrictMode', async () => { + let enter = deferred(); + let onEnter = jest.fn(() => enter.promise); + function Test() { + let ref = useRef(null); + let isEntering = useEnterAnimation(ref, true, onEnter); + return
; + } + + // StrictMode only re-runs effects on mount when it is the root element, which the + // STRICT_MODE wrapper used by render is not. + let {getByTestId} = renderWithoutWrapper( + + + + ); + let el = getByTestId('el'); + expect(onEnter).toHaveBeenCalledTimes(1); + expect(el).toHaveAttribute('data-entering'); + + await act(async () => enter.resolve()); + expect(el).not.toHaveAttribute('data-entering'); + }); + + it('ignores an interrupted exit animation that settles during a later exit', async () => { + let first = deferred(); + let second = deferred(); + let onExit = jest.fn().mockReturnValueOnce(first.promise).mockReturnValueOnce(second.promise); + function Test({isOpen}: {isOpen: boolean}) { + let ref = useRef(null); + let isExiting = useExitAnimation(ref, isOpen, onExit); + if (!isOpen && !isExiting) { + return null; + } + return
; + } + + let {getByTestId, rerender} = render(); + let el = getByTestId('el'); + + rerender(); + rerender(); + rerender(); + expect(onExit).toHaveBeenCalledTimes(2); + + await act(async () => first.resolve()); + expect(el).toBeInTheDocument(); + expect(el).toHaveAttribute('data-exiting'); + + await act(async () => second.resolve()); + expect(el).not.toBeInTheDocument(); + }); +});