Extract logic to dynamically render a native dialog

src/annotator/components/ModalDialog.tsx

+import classnames from 'classnames';
+import type { ComponentChildren } from 'preact';
+import { useEffect, useMemo, useRef } from 'preact/hooks';
+
+type DialogProps = {
+  closed: boolean;
+  children: ComponentChildren;
+  onClose: () => void;
+};
+
+const NativeDialog = ({ closed, children, onClose }: DialogProps) => {
+  const dialogRef = useRef<HTMLDialogElement | null>(null);
+
+  useEffect(() => {
+    if (closed) {
+      dialogRef.current?.close();
+    } else {
+      dialogRef.current?.showModal();
+    }
+  }, [closed]);
+
+  useEffect(() => {
+    const dialogElement = dialogRef.current;
+
+    dialogElement?.addEventListener('cancel', onClose);
+    return () => {
+      dialogElement?.removeEventListener('cancel', onClose);
+    };
+  }, [onClose]);
+
+  return (
+    <dialog
+      ref={dialogRef}
+      className="relative w-full h-full backdrop:bg-black/50"
+      data-testid="notebook-outer"
+    >
+      {children}
+    </dialog>
+  );
+};
+
+/**
+ * Temporary fallback used in browsers not supporting `dialog` element.
+ * It can be removed once all browsers we support can use it.
+ */
+const FallbackDialog = ({ closed, children }: DialogProps) => {
+  return (
+    <div
+      className={classnames(
+        'fixed z-max top-0 left-0 right-0 bottom-0 p-3 bg-black/50',
+        { hidden: closed },
+      )}
+      data-testid="notebook-outer"
+    >
+      <div className="relative w-full h-full" data-testid="notebook-inner">
+        {children}
+      </div>
+    </div>
+  );
+};
+
+/** Checks if the browser supports native modal dialogs */
+function isModalDialogSupported(document: Document) {
+  const dialog = document.createElement('dialog');
+  return typeof dialog.showModal === 'function';
+}
+
+export type ModalDialogProps = DialogProps & {
+  document_?: Document;
+};
+
+export default function ModalDialog({
+  /* istanbul ignore next - test seam */
+  document_ = document,
+  ...rest
+}: ModalDialogProps) {
+  const Dialog = useMemo(
+    () => (isModalDialogSupported(document_) ? NativeDialog : FallbackDialog),
+    [document_],
+  );
+
+  return <Dialog {...rest} />;
+}

src/annotator/components/NotebookModal.tsx

 import { IconButton, CancelIcon } from '@hypothesis/frontend-shared';
 import classnames from 'classnames';
-import type { ComponentChildren } from 'preact';
-import {
-  useCallback,
-  useEffect,
-  useMemo,
-  useRef,
-  useState,
-} from 'preact/hooks';
+import { useCallback, useEffect, useRef, useState } from 'preact/hooks';
 
 import { addConfigFragment } from '../../shared/config-fragment';
 import { createAppConfig } from '../config/app';
 import type { EventBus, Emitter } from '../util/emitter';
+import ModalDialog from './ModalDialog';
 
 /**
  * Configuration used to launch the notebook application.
@@ -48,75 +42,9 @@ function NotebookIframe({ config, groupId }: NotebookIframeProps) {
   );
 }
 
-/** Checks if the browser supports native modal dialogs */
-function isModalDialogSupported(document: Document) {
-  const dialog = document.createElement('dialog');
-  return typeof dialog.showModal === 'function';
-}
-
 export type NotebookModalProps = {
   eventBus: EventBus;
   config: NotebookConfig;
-
-  /** Test seam */
-  document_?: Document;
-};
-
-type DialogProps = {
-  isHidden: boolean;
-  children: ComponentChildren;
-  onClose: () => void;
-};
-
-const NativeDialog = ({ isHidden, children, onClose }: DialogProps) => {
-  const dialogRef = useRef<HTMLDialogElement | null>(null);
-
-  useEffect(() => {
-    if (isHidden) {
-      dialogRef.current?.close();
-    } else {
-      dialogRef.current?.showModal();
-    }
-  }, [isHidden]);
-
-  useEffect(() => {
-    const dialogElement = dialogRef.current;
-
-    dialogElement?.addEventListener('cancel', onClose);
-    return () => {
-      dialogElement?.removeEventListener('cancel', onClose);
-    };
-  }, [onClose]);
-
-  return (
-    <dialog
-      ref={dialogRef}
-      className="relative w-full h-full backdrop:bg-black/50"
-      data-testid="notebook-outer"
-    >
-      {children}
-    </dialog>
-  );
-};
-
-/**
- * Temporary fallback used in browsers not supporting `dialog` element.
- * It can be removed once all browsers we support can use it.
- */
-const FallbackDialog = ({ isHidden, children }: DialogProps) => {
-  return (
-    <div
-      className={classnames(
-        'fixed z-max top-0 left-0 right-0 bottom-0 p-3 bg-black/50',
-        { hidden: isHidden },
-      )}
-      data-testid="notebook-outer"
-    >
-      <div className="relative w-full h-full" data-testid="notebook-inner">
-        {children}
-      </div>
-    </div>
-  );
 };
 
 /**
@@ -125,8 +53,6 @@ const FallbackDialog = ({ isHidden, children }: DialogProps) => {
 export default function NotebookModal({
   eventBus,
   config,
-  /* istanbul ignore next - test seam */
-  document_ = document,
 }: NotebookModalProps) {
   // Temporary solution: while there is no mechanism to sync new annotations in
   // the notebook, we force re-rendering of the iframe on every 'openNotebook'
@@ -138,11 +64,6 @@ export default function NotebookModal({
   const originalDocumentOverflowStyle = useRef('');
   const emitterRef = useRef<Emitter | null>(null);
 
-  const Dialog = useMemo(
-    () => (isModalDialogSupported(document_) ? NativeDialog : FallbackDialog),
-    [document_],
-  );
-
   // Stores the original overflow CSS property of document.body and reset it
   // when the component is destroyed
   useEffect(() => {
@@ -187,7 +108,7 @@ export default function NotebookModal({
   }
 
   return (
-    <Dialog isHidden={isHidden} onClose={onClose}>
+    <ModalDialog closed={isHidden} onClose={onClose}>
       <div className="absolute right-0 m-3">
         <IconButton
           title="Close the Notebook"
@@ -206,6 +127,6 @@ export default function NotebookModal({
         </IconButton>
       </div>
       <NotebookIframe key={iframeKey} config={config} groupId={groupId} />
-    </Dialog>
+    </ModalDialog>
   );
 }

src/annotator/components/test/ModalDialog-test.js

+import { mount } from 'enzyme';
+
+import ModalDialog from '../ModalDialog';
+
+describe('ModalDialog', () => {
+  let components;
+
+  const createComponent = props => {
+    const attachTo = document.createElement('div');
+    document.body.appendChild(attachTo);
+
+    const component = mount(<ModalDialog open {...props} />, {
+      attachTo,
+    });
+    components.push([component, attachTo]);
+    return component;
+  };
+
+  beforeEach(() => {
+    components = [];
+  });
+
+  afterEach(() => {
+    components.forEach(([component, container]) => {
+      component.unmount();
+      container.remove();
+    });
+  });
+
+  context('when native modal dialog is not supported', () => {
+    let fakeDocument;
+
+    beforeEach(() => {
+      fakeDocument = {
+        createElement: sinon.stub().returns({}),
+      };
+    });
+
+    it('does not render a dialog element', () => {
+      const wrapper = createComponent({ document_: fakeDocument });
+      assert.isFalse(wrapper.exists('dialog'));
+    });
+  });
+
+  context('when native modal dialog is supported', () => {
+    it('renders a dialog element', () => {
+      const wrapper = createComponent();
+      assert.isTrue(wrapper.exists('dialog'));
+    });
+
+    it('closes native dialog on cancel', () => {
+      const onClose = sinon.stub();
+      const wrapper = createComponent({ onClose });
+
+      wrapper.find('dialog').getDOMNode().dispatchEvent(new Event('cancel'));
+      wrapper.update();
+      assert.called(onClose);
+    });
+  });
+});

src/annotator/components/test/NotebookModal-test.js

@@ -14,7 +14,7 @@ describe('NotebookModal', () => {
 
   const outerSelector = '[data-testid="notebook-outer"]';
 
-  const createComponent = (config, fakeDocument) => {
+  const createComponent = config => {
     const attachTo = document.createElement('div');
     document.body.appendChild(attachTo);
 
@@ -22,7 +22,6 @@ describe('NotebookModal', () => {
       <NotebookModal
         eventBus={eventBus}
         config={{ notebookAppUrl: notebookURL, ...config }}
-        document_={fakeDocument}
       />,
       { attachTo },
     );
@@ -125,74 +124,25 @@ describe('NotebookModal', () => {
     assert.equal(document.body.style.overflow, 'hidden');
   });
 
-  context('when native modal dialog is not supported', () => {
-    let fakeDocument;
+  [
+    // Close via clicking close button
+    wrapper => getCloseButton(wrapper).props().onClick(),
 
-    beforeEach(() => {
-      fakeDocument = {
-        createElement: sinon.stub().returns({}),
-      };
-    });
-
-    it('does not render a dialog element', () => {
-      const wrapper = createComponent({}, fakeDocument);
-
-      emitter.publish('openNotebook', 'myGroup');
-      wrapper.update();
-
-      assert.isFalse(wrapper.exists('dialog'));
-    });
-
-    it('hides modal on closing', () => {
-      const wrapper = createComponent({}, fakeDocument);
-
-      emitter.publish('openNotebook', 'myGroup');
-      wrapper.update();
-
-      let outer = wrapper.find(outerSelector);
-      assert.isFalse(outer.hasClass('hidden'));
-
-      act(() => {
-        getCloseButton(wrapper).prop('onClick')();
-      });
-      wrapper.update();
-
-      outer = wrapper.find(outerSelector);
-
-      assert.isTrue(outer.hasClass('hidden'));
-    });
-  });
-
-  context('when native modal dialog is supported', () => {
-    it('renders a dialog element', () => {
+    // Close via "cancel" event, like pressing `Esc` key
+    wrapper =>
+      wrapper.find('dialog').getDOMNode().dispatchEvent(new Event('cancel')),
+  ].forEach(closeDialog => {
+    it('opens and closes native dialog', () => {
       const wrapper = createComponent({});
+      const isDialogOpen = () => wrapper.find('dialog').getDOMNode().open;
 
-      emitter.publish('openNotebook', 'myGroup');
+      act(() => emitter.publish('openNotebook', 'myGroup'));
       wrapper.update();
+      assert.isTrue(isDialogOpen());
 
-      assert.isTrue(wrapper.exists('dialog'));
-    });
-
-    [
-      // Close via clicking close button
-      wrapper => getCloseButton(wrapper).props().onClick(),
-
-      // Close via "cancel" event, like pressing `Esc` key
-      wrapper =>
-        wrapper.find('dialog').getDOMNode().dispatchEvent(new Event('cancel')),
-    ].forEach(closeDialog => {
-      it('opens and closes native dialog', () => {
-        const wrapper = createComponent({});
-        const isDialogOpen = () => wrapper.find('dialog').getDOMNode().open;
-
-        act(() => emitter.publish('openNotebook', 'myGroup'));
-        wrapper.update();
-        assert.isTrue(isDialogOpen());
-
-        act(() => closeDialog(wrapper));
-        wrapper.update();
-        assert.isFalse(isDialogOpen());
-      });
+      act(() => closeDialog(wrapper));
+      wrapper.update();
+      assert.isFalse(isDialogOpen());
     });
   });