Allow closed state to be passed to Dialog for proper transition handling

src/components/feedback/Dialog.tsx

@@ -26,12 +26,20 @@ type ComponentProps = {
   closeOnClickAway?: boolean;
   closeOnEscape?: boolean;
   closeOnFocusAway?: boolean;
+
   /**
    * Dialog _should_ be provided with a close handler. We have a few edge use
    * cases, however, in which we need to render a "non-closeable" modal dialog.
    */
   onClose?: () => void;
 
+  /**
+   * This prop allows to control closing state from consuming code.
+   * If a TransitionComponent is also passed, changing this will trigger the
+   * corresponding `open`/`close` transition.
+   */
+  closed?: boolean;
+
   /**
    * Element that should take focus when the Dialog is first rendered. When not
    * provided ("auto"), the dialog's outer element will take focus. Setting this
@@ -74,6 +82,7 @@ const Dialog = function Dialog({
   restoreFocus = false,
   transitionComponent: TransitionComponent,
   variant = 'panel',
+  closed = false,
 
   classes,
   elementRef,
@@ -89,9 +98,13 @@ const Dialog = function Dialog({
   ...htmlAttributes
 }: DialogProps) {
   const modalRef = useSyncedRef(elementRef);
-  const restoreFocusEl = useRef<HTMLElement | null>(
-    document.activeElement as HTMLElement | null
-  );
+  const restoreFocusEl = useRef<HTMLElement | null>(null);
+  const isClosableDialog = !!onClose;
+
+  // TODO To properly handle closing/opening with a TransitionComponent, these
+  //  two pieces of state need to be synced with the `closed` prop.
+  //  That makes the logic hard to follow. It would be good to revisit eventually
+  const [isClosed, setIsClosed] = useState(closed);
   const [transitionComponentVisible, setTransitionComponentVisible] =
     useState(false);
 
@@ -130,10 +143,17 @@ const Dialog = function Dialog({
     }
   }, [initialFocus, modalRef]);
 
+  const doRestoreFocus = useCallback(() => {
+    if (restoreFocus) {
+      restoreFocusEl.current?.focus();
+    }
+  }, [restoreFocus]);
+
   const onTransitionEnd = (direction: 'in' | 'out') => {
     if (direction === 'in') {
       initializeDialog();
     } else {
+      setIsClosed(true);
       onClose?.();
     }
   };
@@ -157,31 +177,40 @@ const Dialog = function Dialog({
   );
 
   useEffect(() => {
-    setTransitionComponentVisible(true);
-    if (!TransitionComponent) {
+    if (closed) {
+      if (TransitionComponent) {
+        setTransitionComponentVisible(false);
+      } else {
+        setIsClosed(true);
+      }
+    } else {
+      setIsClosed(false);
+    }
+
+    if (!closed && !TransitionComponent) {
       initializeDialog();
     }
 
-    // We only want to run this effect once when the dialog is mounted.
+    // We only want to run this effect when opened or closed.
     // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, []);
+  }, [closed]);
 
-  useLayoutEffect(
-    /**
-     * Restore focus when component is unmounted, if `restoreFocus` is set.
-     */
-    () => {
-      const restoreFocusTo = restoreFocusEl.current;
-      return () => {
-        if (restoreFocus && restoreFocusTo) {
-          restoreFocusTo.focus();
-        }
-      };
-    },
-    // We only want to run this effect once when the dialog is mounted.
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-    []
-  );
+  useEffect(() => {
+    if (!isClosed) {
+      setTransitionComponentVisible(true);
+    }
+  }, [isClosed]);
+
+  useLayoutEffect(() => {
+    if (!isClosed) {
+      // Determine active element when component is "opened"
+      restoreFocusEl.current = document.activeElement as HTMLElement | null;
+    } else {
+      doRestoreFocus();
+    }
+
+    return doRestoreFocus;
+  }, [isClosed, doRestoreFocus]);
 
   useLayoutEffect(
     /**
@@ -201,11 +230,14 @@ const Dialog = function Dialog({
     [dialogDescriptionId, modalRef]
   );
 
-  // Provide a close handler to descendant components
   const closeableContext: CloseableInfo = {
     onClose: onClose ? closeHandler : undefined,
   };
 
+  if (isClosed) {
+    return null;
+  }
+
   return (
     <CloseableContext.Provider value={closeableContext}>
       <Wrapper

src/components/feedback/test/Dialog-test.js

@@ -1,5 +1,6 @@
 import { mount } from 'enzyme';
 import { createRef } from 'preact';
+import { useEffect } from 'preact/hooks';
 
 import { delay } from '../../../test-util/wait.js';
 import { testPresentationalComponent } from '../../test/common-tests';
@@ -19,7 +20,14 @@ const createComponent = (Component, props = {}) => {
  */
 const ComponentWithTransition = ({ children, direction, onTransitionEnd }) => {
   // Fake a 50ms transition time
-  setTimeout(() => onTransitionEnd?.(direction), 50);
+  useEffect(() => {
+    const timeout = setTimeout(() => onTransitionEnd?.(direction), 50);
+
+    return () => {
+      clearTimeout(timeout);
+    };
+  }, [direction, onTransitionEnd]);
+
   return <div>{children}</div>;
 };
 
@@ -178,26 +186,37 @@ describe('Dialog', () => {
       assert.equal(document.activeElement, document.body);
     });
 
-    it('should restore focus when unmounted when `restoreFocus` set', () => {
-      const button = document.getElementById('focus-button');
+    [
+      {
+        action: 'closed',
+        updateWrapper: wrapper => wrapper.setProps({ closed: true }),
+      },
+      {
+        action: 'unmounted',
+        updateWrapper: wrapper => wrapper.unmount(),
+      },
+    ].forEach(({ action, updateWrapper }) => {
+      it(`should restore focus when ${action} when \`restoreFocus\` set`, () => {
+        const button = document.getElementById('focus-button');
 
-      // Start with focus on a button
-      button.focus();
-      assert.equal(document.activeElement, button);
+        // Start with focus on a button
+        button.focus();
+        assert.equal(document.activeElement, button);
 
-      const wrapper = mount(
-        <Dialog id="focus-dialog" title="My dialog" restoreFocus />,
-        {
-          attachTo: container,
-        }
-      );
-      const dialogElement = document.getElementById('focus-dialog');
-      // Focus moves to dialog by default when mounted
-      assert.equal(document.activeElement, dialogElement);
+        const wrapper = mount(
+          <Dialog id="focus-dialog" title="My dialog" restoreFocus />,
+          {
+            attachTo: container,
+          }
+        );
+        const dialogElement = document.getElementById('focus-dialog');
+        // Focus moves to dialog by default when mounted
+        assert.equal(document.activeElement, dialogElement);
 
-      // Unmount cleanup should restore focus to the button
-      wrapper.unmount();
-      assert.equal(document.activeElement, button);
+        // Updating should restore focus to the button
+        updateWrapper(wrapper);
+        assert.equal(document.activeElement, button);
+      });
     });
   });
 
@@ -267,9 +286,7 @@ describe('Dialog', () => {
             onClose={onClose}
             transitionComponent={ComponentWithTransition}
           />,
-          {
-            attachTo: container,
-          }
+          { attachTo: container }
         );
 
         // We simulate closing the Dialog's Panel
@@ -281,7 +298,7 @@ describe('Dialog', () => {
         // The onClose callback is not immediately invoked
         assert.notCalled(onClose);
         // Once the transition has ended, the callback should have been called
-        await delay(60); // Transition finishes after 50ms
+        await delay(70); // Transition finishes after 50ms
         assert.called(onClose);
       });
     });
@@ -292,6 +309,32 @@ describe('Dialog', () => {
         assert.isFalse(wrapper.find('CancelIcon').exists());
       });
     });
+
+    context('when `closed` prop is true', () => {
+      it('sets component as closed without TransitionComponent', () => {
+        const wrapper = mount(<Dialog closed title="My dialog" />);
+        assert.isFalse(wrapper.exists('div[data-component="Dialog"]'));
+      });
+
+      it('triggers `out` transition with transition component', async () => {
+        const wrapper = mount(
+          <Dialog
+            title="My dialog"
+            transitionComponent={ComponentWithTransition}
+          />
+        );
+
+        wrapper.setProps({ closed: true });
+
+        // The dialog is still rendered, until the transition is finished
+        assert.isTrue(wrapper.exists('div[data-component="Dialog"]'));
+
+        await delay(60); // Transition finishes after 50ms
+        wrapper.update();
+
+        assert.isFalse(wrapper.exists('div[data-component="Dialog"]'));
+      });
+    });
   });
 
   describe('dialog layout', () => {