[popups] Look for animations when open is set to false externally (…

…#878)

packages/react/src/alert-dialog/root/AlertDialogRoot.tsx

@@ -16,11 +16,12 @@ import { useDialogRoot } from '../../dialog/root/useDialogRoot';
  * - [AlertDialogRoot API](https://base-ui.com/components/react-alert-dialog/#api-reference-AlertDialogRoot)
  */
 const AlertDialogRoot: React.FC<AlertDialogRoot.Props> = function AlertDialogRoot(props) {
-  const { animated = true, children, defaultOpen, onOpenChange, open } = props;
+  const { animated = true, children, defaultOpen = false, onOpenChange, open } = props;
 
   const parentDialogRootContext = React.useContext(AlertDialogRootContext);
 
   const dialogRoot = useDialogRoot({
+    animated,
     open,
     defaultOpen,
     onOpenChange,

packages/react/src/dialog/popup/useDialogPopup.tsx

@@ -74,7 +74,6 @@ export function useDialogPopup(parameters: useDialogPopup.Parameters): useDialog
     mergeReactProps<'div'>(externalProps, {
       'aria-labelledby': titleElementId ?? undefined,
       'aria-describedby': descriptionElementId ?? undefined,
-      'aria-hidden': !open || undefined,
       'aria-modal': open && modal ? true : undefined,
       role: 'dialog',
       tabIndex: -1,

packages/react/src/dialog/root/DialogRoot.test.tsx

@@ -1,7 +1,7 @@
 import * as React from 'react';
 import { expect } from 'chai';
 import { spy } from 'sinon';
-import { act, describeSkipIf, fireEvent } from '@mui/internal-test-utils';
+import { act, describeSkipIf, fireEvent, screen, waitFor } from '@mui/internal-test-utils';
 import { Dialog } from '@base-ui-components/react/dialog';
 import { createRenderer } from '#test-utils';
 
@@ -44,43 +44,93 @@ describe('<Dialog.Root />', () => {
       setProps({ open: false });
       expect(queryByRole('dialog')).to.equal(null);
     });
-  });
 
-  // toWarnDev doesn't work reliably with async rendering. To re-eanble after it's fixed in the test-utils.
-  // eslint-disable-next-line mocha/no-skipped-tests
-  describe.skip('prop: modal', () => {
-    it('warns when the dialog is modal but no backdrop is present', async () => {
-      await expect(() =>
-        render(
-          <Dialog.Root modal animated={false}>
-            <Dialog.Popup />
-          </Dialog.Root>,
-        ),
-      ).toWarnDev([
-        'Base UI: The Dialog is modal but no backdrop is present. Add the backdrop component to prevent interacting with the rest of the page.',
-        'Base UI: The Dialog is modal but no backdrop is present. Add the backdrop component to prevent interacting with the rest of the page.',
-      ]);
-    });
+    it('should remove the popup when animated=true and there is no exit animation defined', async function test(t = {}) {
+      if (/jsdom/.test(window.navigator.userAgent)) {
+        // @ts-expect-error to support mocha and vitest
+        // eslint-disable-next-line @typescript-eslint/no-unused-expressions
+        this?.skip?.() || t?.skip();
+      }
+
+      function Test() {
+        const [open, setOpen] = React.useState(true);
+
+        return (
+          <div>
+            <button onClick={() => setOpen(false)}>Close</button>
+            <Dialog.Root open={open}>
+              <Dialog.Popup />
+            </Dialog.Root>
+          </div>
+        );
+      }
+
+      const { user } = await render(<Test />);
+
+      const closeButton = screen.getByText('Close');
+      await user.click(closeButton);
 
-    it('does not warn when the dialog is not modal and no backdrop is present', () => {
-      expect(() =>
-        render(
-          <Dialog.Root modal={false} animated={false}>
-            <Dialog.Popup />
-          </Dialog.Root>,
-        ),
-      ).not.toWarnDev();
+      await waitFor(() => {
+        expect(screen.queryByRole('dialog')).to.equal(null);
+      });
     });
 
-    it('does not warn when the dialog is modal and backdrop is present', () => {
-      expect(() =>
-        render(
-          <Dialog.Root modal animated={false}>
-            <Dialog.Backdrop />
-            <Dialog.Popup />
-          </Dialog.Root>,
-        ),
-      ).not.toWarnDev();
+    it('should remove the popup when animated=true and the animation finishes', async function test(t = {}) {
+      if (/jsdom/.test(window.navigator.userAgent)) {
+        // @ts-expect-error to support mocha and vitest
+        // eslint-disable-next-line @typescript-eslint/no-unused-expressions
+        this?.skip?.() || t?.skip();
+      }
+
+      let animationFinished = false;
+      const notifyAnimationFinished = () => {
+        animationFinished = true;
+      };
+
+      function Test() {
+        const style = `
+          @keyframes test-anim {
+            to {
+              opacity: 0;
+            }
+          }
+
+          .animation-test-popup[data-open] {
+            opacity: 1;
+          }
+
+          .animation-test-popup[data-exiting] {
+            animation: test-anim 50ms;
+          }
+        `;
+
+        const [open, setOpen] = React.useState(true);
+
+        return (
+          <div>
+            {/* eslint-disable-next-line react/no-danger */}
+            <style dangerouslySetInnerHTML={{ __html: style }} />
+            <button onClick={() => setOpen(false)}>Close</button>
+            <Dialog.Root open={open}>
+              <Dialog.Popup
+                className="animation-test-popup"
+                onAnimationEnd={notifyAnimationFinished}
+              />
+            </Dialog.Root>
+          </div>
+        );
+      }
+
+      const { user } = await render(<Test />);
+
+      const closeButton = screen.getByText('Close');
+      await user.click(closeButton);
+
+      await waitFor(() => {
+        expect(screen.queryByRole('dialog')).to.equal(null);
+      });
+
+      expect(animationFinished).to.equal(true);
     });
   });
 
@@ -136,16 +186,24 @@ describe('<Dialog.Root />', () => {
   `;
 
     it('when `true`, waits for the exit transition to finish before unmounting', async () => {
+      const notifyTransitionEnd = spy();
+
       const { setProps, queryByRole } = await render(
         <Dialog.Root open modal={false} animated>
           {/* eslint-disable-next-line react/no-danger */}
           <style dangerouslySetInnerHTML={{ __html: css }} />
-          <Dialog.Popup className="dialog" />
+          <Dialog.Popup className="dialog" onTransitionEnd={notifyTransitionEnd} />
         </Dialog.Root>,
       );
 
       setProps({ open: false });
-      expect(queryByRole('dialog', { hidden: true })).not.to.equal(null);
+      expect(queryByRole('dialog')).not.to.equal(null);
+
+      await waitFor(() => {
+        expect(queryByRole('dialog')).to.equal(null);
+      });
+
+      expect(notifyTransitionEnd.callCount).to.equal(1);
     });
 
     it('when `false`, unmounts the popup immediately', async () => {

packages/react/src/dialog/root/DialogRoot.tsx

@@ -29,6 +29,7 @@ const DialogRoot = function DialogRoot(props: DialogRoot.Props) {
   const parentDialogRootContext = React.useContext(DialogRootContext);
 
   const dialogRoot = useDialogRoot({
+    animated,
     open,
     defaultOpen,
     onOpenChange,

packages/react/src/dialog/root/useDialogRoot.ts

@@ -10,20 +10,18 @@ import {
 import { useControlled } from '../../utils/useControlled';
 import { useEventCallback } from '../../utils/useEventCallback';
 import { useTransitionStatus, type TransitionStatus } from '../../utils/useTransitionStatus';
-import { useAnimationsFinished } from '../../utils/useAnimationsFinished';
 import { type InteractionType } from '../../utils/useEnhancedClickHandler';
-import { type GenericHTMLProps } from '../../utils/types';
+import type { RequiredExcept, GenericHTMLProps } from '../../utils/types';
 import { useOpenInteractionType } from '../../utils/useOpenInteractionType';
 import { mergeReactProps } from '../../utils/mergeReactProps';
-import { useLatestRef } from '../../utils/useLatestRef';
+import { useUnmountAfterExitAnimation } from '../../utils/useUnmountAfterCloseAnimation';
 
 export function useDialogRoot(parameters: useDialogRoot.Parameters): useDialogRoot.ReturnValue {
   const {
-    animated = true,
-    defaultOpen = false,
-    dismissible = true,
-    keepMounted = false,
-    modal = true,
+    animated,
+    defaultOpen,
+    dismissible,
+    modal,
     onNestedDialogClose,
     onNestedDialogOpen,
     onOpenChange,
@@ -49,25 +47,16 @@ export function useDialogRoot(parameters: useDialogRoot.Parameters): useDialogRo
 
   const { mounted, setMounted, transitionStatus } = useTransitionStatus(open);
 
-  const runOnceAnimationsFinish = useAnimationsFinished(popupRef);
-
-  const openRef = useLatestRef(open);
-
   const setOpen = useEventCallback((nextOpen: boolean, event?: Event) => {
     onOpenChange?.(nextOpen, event);
     setOpenUnwrapped(nextOpen);
+  });
 
-    if (!keepMounted && !nextOpen) {
-      if (animated) {
-        runOnceAnimationsFinish(() => {
-          if (!openRef.current) {
-            setMounted(false);
-          }
-        });
-      } else {
-        setMounted(false);
-      }
-    }
+  useUnmountAfterExitAnimation({
+    open,
+    animated,
+    animatedElementRef: popupRef,
+    setMounted,
   });
 
   const context = useFloatingRootContext({
@@ -198,15 +187,10 @@ export interface CommonParameters {
    * @default true
    */
   dismissible?: boolean;
-  /**
-   * Whether the dialog element stays mounted in the DOM when closed.
-   * @default false
-   */
-  keepMounted?: boolean;
 }
 
 export namespace useDialogRoot {
-  export interface Parameters extends CommonParameters {
+  export interface Parameters extends RequiredExcept<CommonParameters, 'open' | 'onOpenChange'> {
     /**
      * Callback to invoke when a nested dialog is opened.
      */

packages/react/src/menu/root/MenuRoot.test.tsx

@@ -1,6 +1,6 @@
 import * as React from 'react';
 import { expect } from 'chai';
-import { act, flushMicrotasks, waitFor } from '@mui/internal-test-utils';
+import { act, flushMicrotasks, waitFor, screen } from '@mui/internal-test-utils';
 import { Menu } from '@base-ui-components/react/menu';
 import userEvent from '@testing-library/user-event';
 import { spy } from 'sinon';
@@ -685,4 +685,98 @@ describe('<Menu.Root />', () => {
       expect(menus[0].id).to.equal('parent-menu');
     });
   });
+
+  describe('controlled mode', () => {
+    it('should remove the popup when animated=true and there is no exit animation defined', async function test(t = {}) {
+      if (/jsdom/.test(window.navigator.userAgent)) {
+        // @ts-expect-error to support mocha and vitest
+        // eslint-disable-next-line @typescript-eslint/no-unused-expressions
+        this?.skip?.() || t?.skip();
+      }
+
+      function Test() {
+        const [open, setOpen] = React.useState(true);
+
+        return (
+          <div>
+            <button onClick={() => setOpen(false)}>Close</button>
+            <Menu.Root open={open}>
+              <Menu.Positioner>
+                <Menu.Popup />
+              </Menu.Positioner>
+            </Menu.Root>
+          </div>
+        );
+      }
+
+      await render(<Test />);
+
+      const closeButton = screen.getByText('Close');
+      await user.click(closeButton);
+
+      await waitFor(() => {
+        expect(screen.queryByRole('menu')).to.equal(null);
+      });
+    });
+
+    it('should remove the popup when animated=true and the animation finishes', async function test(t = {}) {
+      if (/jsdom/.test(window.navigator.userAgent)) {
+        // @ts-expect-error to support mocha and vitest
+        // eslint-disable-next-line @typescript-eslint/no-unused-expressions
+        this?.skip?.() || t?.skip();
+      }
+
+      let animationFinished = false;
+      const notifyAnimationFinished = () => {
+        animationFinished = true;
+      };
+
+      function Test() {
+        const style = `
+          @keyframes test-anim {
+            to {
+              opacity: 0;
+            }
+          }
+
+          .animation-test-popup[data-open] {
+            opacity: 1;
+          }
+
+          .animation-test-popup[data-exiting] {
+            animation: test-anim 50ms;
+          }
+        `;
+
+        const [open, setOpen] = React.useState(true);
+
+        return (
+          <div>
+            {/* eslint-disable-next-line react/no-danger */}
+            <style dangerouslySetInnerHTML={{ __html: style }} />
+            <button onClick={() => setOpen(false)}>Close</button>
+            <Menu.Root open={open}>
+              <Menu.Positioner>
+                <Menu.Popup
+                  className="animation-test-popup"
+                  onAnimationEnd={notifyAnimationFinished}
+                />
+              </Menu.Positioner>
+            </Menu.Root>
+          </div>
+        );
+      }
+
+      await render(<Test />);
+
+      const closeButton = screen.getByText('Close');
+      await user.click(closeButton);
+
+      await waitFor(() => {
+        expect(screen.queryByRole('menu')).to.equal(null);
+      });
+
+      expect(animationFinished).to.equal(true);
+    });
+  });
 });