Use <dialog> for binder EditOrRun modal (#2681)

fonsp · Oct 27, 2023 · 925f243 · 925f243
1 parent effe84d
commit 925f243
Show file tree

Hide file tree

Showing 4 changed files with 55 additions and 42 deletions.
diff --git a/frontend/binder.css b/frontend/binder.css
@@ -133,13 +133,11 @@ body.wiggle_binder .edit_or_run > button {
 .binder_help_text {
     --width: min(85vw, 570px);
     position: fixed;
-    top: 5rem;
     max-height: calc(100vh - 4rem);
     overflow: auto;
     width: var(--width);
     padding: 16px;
     border-radius: 8px;
-    left: calc(50vw - var(--width) / 2);
     background-color: white;
     color: black;
     color-scheme: light;

diff --git a/frontend/common/useDialog.js b/frontend/common/useDialog.js
@@ -0,0 +1,41 @@
+//@ts-ignore
+import dialogPolyfill from "https://cdn.jsdelivr.net/npm/[email protected]/dist/dialog-polyfill.esm.min.js"
+
+import { useEffect, useLayoutEffect, useRef } from "../imports/Preact.js"
+
+/**
+ * @returns {[import("../imports/Preact.js").Ref<HTMLDialogElement?>, () => void, () => void, () => void]}
+ */
+export const useDialog = ({ light_dismiss = false } = {}) => {
+    const dialog_ref = useRef(/** @type {HTMLDialogElement?} */ (null))
+
+    useLayoutEffect(() => {
+        if (dialog_ref.current != null) dialogPolyfill.registerDialog(dialog_ref.current)
+    }, [dialog_ref.current])
+
+    //@ts-ignore
+    const open = () => dialog_ref.current.showModal()
+    //@ts-ignore
+    const close = () => dialog_ref.current.close()
+    //@ts-ignore
+    const toggle = () => (dialog_ref.current.open ? dialog_ref.current?.close() : dialog_ref.current?.showModal())
+
+    useEffect(() => {
+        if (light_dismiss) {
+            const handleclick = (e) => {
+                if (dialog_ref.current?.open && dialog_ref.current.contains(e.target)) {
+                    close()
+                    // Avoid activating whatever was below
+                    e.stopPropagation()
+                    e.preventDefault()
+                }
+            }
+            document.body.addEventListener("click", handleclick)
+            return () => {
+                document.body.removeEventListener("click", handleclick)
+            }
+        }
+    }, [dialog_ref.current])
+
+    return [dialog_ref, open, close, toggle]
+}
diff --git a/frontend/components/EditOrRunButton.js b/frontend/components/EditOrRunButton.js
@@ -1,6 +1,9 @@
 import _ from "../imports/lodash.js"
 import { BackendLaunchPhase } from "../common/Binder.js"
-import { html, useEffect, useState, useRef } from "../imports/Preact.js"
+import { html, useEffect, useState, useRef, useLayoutEffect } from "../imports/Preact.js"
+
+import { has_ctrl_or_cmd_pressed } from "../common/KeyboardShortcuts.js"
+import { useDialog } from "../common/useDialog.js"
 
 export const RunLocalButton = ({ show, start_local }) => {
     //@ts-ignore
@@ -30,36 +33,14 @@ export const RunLocalButton = ({ show, start_local }) => {
  * }} props
  * */
 export const BinderButton = ({ offer_binder, start_binder, notebookfile, notebook }) => {
-    const [popupOpen, setPopupOpen] = useState(false)
+    const [dialog_ref, openModal, closeModal, toggleModal] = useDialog({ light_dismiss: true })
+
     const [showCopyPopup, setShowCopyPopup] = useState(false)
     const notebookfile_ref = useRef("")
     notebookfile_ref.current = notebookfile ?? ""
 
     //@ts-ignore
-    window.open_edit_or_run_popup = () => {
-        setPopupOpen(true)
-    }
-
-    useEffect(() => {
-        const handlekeyup = (e) => {
-            e.key === "Escape" && setPopupOpen(false)
-        }
-        const handleclick = (e) => {
-            if (popupOpen && !e.target?.closest(".binder_help_text")) {
-                setPopupOpen(false)
-                // Avoid activating whatever was below
-                e.stopPropagation()
-                e.preventDefault()
-            }
-        }
-        document.body.addEventListener("keyup", handlekeyup)
-        document.body.addEventListener("click", handleclick)
-
-        return () => {
-            document.body.removeEventListener("keyup", handlekeyup)
-            document.body.removeEventListener("click", handleclick)
-        }
-    }, [popupOpen])
+    window.open_edit_or_run_popup = openModal
 
     useEffect(() => {
         //@ts-ignore
@@ -73,19 +54,19 @@ export const BinderButton = ({ offer_binder, start_binder, notebookfile, noteboo
 
     const recommend_download = notebookfile_ref.current.startsWith("data:")
     const runtime_str = expected_runtime_str(notebook)
+
     return html`<div class="edit_or_run">
         <button
             onClick=${(e) => {
+                toggleModal()
                 e.stopPropagation()
                 e.preventDefault()
-                setPopupOpen(!popupOpen)
             }}
         >
             <b>Edit</b> or <b>run</b> this notebook
         </button>
-        ${popupOpen &&
-        html`<div class="binder_help_text">
-            <span onClick=${() => setPopupOpen(false)} class="close"></span>
+        <dialog ref=${dialog_ref} class="binder_help_text">
+            <span onClick=${closeModal} class="close"></span>
             ${offer_binder
                 ? html`
                       <p style="text-align: center;">
@@ -162,7 +143,7 @@ export const BinderButton = ({ offer_binder, start_binder, notebookfile, noteboo
                           `}
                 </li>
             </ol>
-        </div>`}
+        </dialog>
     </div>`
 }
 

diff --git a/frontend/components/FrontmatterInput.js b/frontend/components/FrontmatterInput.js
@@ -7,6 +7,7 @@ import "https://cdn.jsdelivr.net/gh/fonsp/[email protected]/lib/rebel-tag-in
 //@ts-ignore
 import dialogPolyfill from "https://cdn.jsdelivr.net/npm/[email protected]/dist/dialog-polyfill.esm.min.js"
 import immer from "../imports/immer.js"
+import { useDialog } from "../common/useDialog.js"
 
 /**
  * @param {{
@@ -32,15 +33,7 @@ export const FrontMatterInput = ({ remote_frontmatter, set_remote_frontmatter })
             })
         )
 
-    const dialog_ref = useRef(/** @type {HTMLDialogElement?} */ (null))
-    useLayoutEffect(() => {
-        dialogPolyfill.registerDialog(dialog_ref.current)
-    })
-
-    //@ts-ignore
-    const open = () => dialog_ref.current.showModal()
-    //@ts-ignore
-    const close = () => dialog_ref.current.close()
+    const [dialog_ref, open, close, _toggle] = useDialog({ light_dismiss: false })
 
     const cancel = () => {
         set_frontmatter(remote_frontmatter ?? {})