Use ModalDialog in the notebook

hypothesis · Feb 1, 2024 · feb020a · feb020a
1 parent aa7aa7a
commit feb020a
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 17 deletions.
diff --git a/package.json b/package.json
@@ -12,7 +12,7 @@
     "@babel/preset-react": "^7.0.0",
     "@babel/preset-typescript": "^7.16.7",
     "@hypothesis/frontend-build": "^3.0.0",
-    "@hypothesis/frontend-shared": "^7.2.0",
+    "@hypothesis/frontend-shared": "^7.3.0",
     "@hypothesis/frontend-testing": "^1.2.0",
     "@npmcli/arborist": "^7.0.0",
     "@octokit/rest": "^20.0.1",

diff --git a/src/annotator/components/NotebookModal.tsx b/src/annotator/components/NotebookModal.tsx
@@ -1,5 +1,10 @@
-import { IconButton, CancelIcon } from '@hypothesis/frontend-shared';
+import {
+  IconButton,
+  CancelIcon,
+  ModalDialog,
+} from '@hypothesis/frontend-shared';
 import classnames from 'classnames';
+import type { Ref } from 'preact';
 import { useEffect, useRef, useState } from 'preact/hooks';
 
 import { addConfigFragment } from '../../shared/config-fragment';
@@ -19,15 +24,16 @@ export type NotebookConfig = {
 type NotebookIframeProps = {
   config: NotebookConfig;
   groupId: string;
+  iframeRef: Ref<HTMLIFrameElement>;
 };
 /**
  * Create the iframe that will load the notebook application.
  */
-function NotebookIframe({ config, groupId }: NotebookIframeProps) {
+function NotebookIframe({ config, groupId, iframeRef }: NotebookIframeProps) {
   const notebookAppSrc = addConfigFragment(config.notebookAppUrl, {
     ...createAppConfig(config.notebookAppUrl, config),
 
-    // Explicity set the "focused" group
+    // Explicitly set the "focused" group
     group: groupId,
   });
 
@@ -37,9 +43,11 @@ function NotebookIframe({ config, groupId }: NotebookIframeProps) {
       className="h-full w-full border-0"
       allow="fullscreen; clipboard-write"
       src={notebookAppSrc}
+      ref={iframeRef}
     />
   );
 }
+
 export type NotebookModalProps = {
   eventBus: EventBus;
   config: NotebookConfig;
@@ -61,6 +69,7 @@ export default function NotebookModal({
   const [groupId, setGroupId] = useState<string | null>(null);
   const originalDocumentOverflowStyle = useRef('');
   const emitterRef = useRef<Emitter | null>(null);
+  const iframeRef = useRef<HTMLIFrameElement | null>(null);
 
   // Stores the original overflow CSS property of document.body and reset it
   // when the component is destroyed
@@ -107,14 +116,18 @@ export default function NotebookModal({
 
   return (
     <div
-      className={classnames(
-        'fixed z-max top-0 left-0 right-0 bottom-0 p-3 bg-black/50',
-        { hidden: isHidden },
-      )}
+      className={classnames({ hidden: isHidden })}
       data-testid="notebook-outer"
     >
-      <div className="relative w-full h-full" data-testid="notebook-inner">
-        <div className="absolute right-0 m-3">
+      <ModalDialog
+        variant="custom"
+        size="custom"
+        classes="p-3 relative w-full h-full"
+        // Initially focusing the iframe means the next element in the tab
+        // sequence will be an element inside the iframe.
+        initialFocus={iframeRef}
+      >
+        <div className="absolute right-6 top-6">
           <IconButton
             title="Close the Notebook"
             onClick={onClose}
@@ -130,8 +143,13 @@ export default function NotebookModal({
             <CancelIcon className="w-4 h-4" />
           </IconButton>
         </div>
-        <NotebookIframe key={iframeKey} config={config} groupId={groupId} />
-      </div>
+        <NotebookIframe
+          key={iframeKey}
+          config={config}
+          groupId={groupId}
+          iframeRef={iframeRef}
+        />
+      </ModalDialog>
     </div>
   );
 }
diff --git a/yarn.lock b/yarn.lock
@@ -1995,15 +1995,15 @@ __metadata:
   languageName: node
   linkType: hard
 
-"@hypothesis/frontend-shared@npm:^7.2.0":
-  version: 7.2.0
-  resolution: "@hypothesis/frontend-shared@npm:7.2.0"
+"@hypothesis/frontend-shared@npm:^7.3.0":
+  version: 7.3.0
+  resolution: "@hypothesis/frontend-shared@npm:7.3.0"
   dependencies:
     highlight.js: ^11.6.0
     wouter-preact: ^2.10.0-alpha.1
   peerDependencies:
     preact: ^10.4.0
-  checksum: fe4b515e0f856dcb6c9876e52aba2a262d1bf6cebb13176b6ee2a676df719e2331106ee1595e2de057843fe7ebe8ac03f3a8f568fb03f50b3ea0622496da2ead
+  checksum: 5b295e0cb949c858d70c96ab24e7ec387d31e4a0e5bde6555ba70843cdceaae030a119e20841fa4924888db618c7fa47bdd4f83a3be65da889ad702506cc75f4
   languageName: node
   linkType: hard
 
@@ -7793,7 +7793,7 @@ __metadata:
     "@babel/preset-react": ^7.0.0
     "@babel/preset-typescript": ^7.16.7
     "@hypothesis/frontend-build": ^3.0.0
-    "@hypothesis/frontend-shared": ^7.2.0
+    "@hypothesis/frontend-shared": ^7.3.0
     "@hypothesis/frontend-testing": ^1.2.0
     "@npmcli/arborist": ^7.0.0
     "@octokit/rest": ^20.0.1