fix(entrykit): improve escape handling (#3623)

Author: frolic

.changeset/kind-carpets-peel.md

@@ -0,0 +1,5 @@
+---
+"@latticexyz/entrykit": patch
+---
+
+Improved escape key handling when account modal is open. And fixed development warnings about missing dialog title/description.

packages/entrykit/src/AccountModal.tsx

@@ -2,7 +2,7 @@ import { Modal } from "./ui/Modal";
 import { useAccountModal } from "./useAccountModal";
 import { twMerge } from "tailwind-merge";
 import { AccountModalContent } from "./AccountModalContent";
-import { DialogClose, DialogTitle } from "@radix-ui/react-dialog";
+import { DialogClose } from "@radix-ui/react-dialog";
 import { CloseIcon } from "./icons/CloseIcon";
 import { Logo } from "./icons/Logo";
 import { ErrorBoundary } from "react-error-boundary";
@@ -13,8 +13,6 @@ export function AccountModal() {
   const { accountModalOpen, toggleAccountModal } = useAccountModal();
   return (
     <Modal open={accountModalOpen} onOpenChange={toggleAccountModal}>
-      {/* TODO: move this into `<Modal>` props? */}
-      <DialogTitle className="sr-only">Connect with EntryKit</DialogTitle>
       {accountModalOpen ? (
         <div
           className={twMerge(
@@ -25,17 +23,6 @@ export function AccountModal() {
             "links:decoration-neutral-500 hover:links:decoration-orange-500",
           )}
         >
-          <div className="absolute top-0 right-0">
-            <DialogClose
-              className={twMerge(
-                "pointer-events-auto leading-none p-2 transition",
-                "text-white/20 hover:text-white/40",
-              )}
-              title="Close"
-            >
-              <CloseIcon className="m-0" />
-            </DialogClose>
-          </div>
           <ErrorBoundary FallbackComponent={ErrorFallback}>
             <AccountModalContent />
             <ErrorsOverlay />
@@ -52,6 +39,18 @@ export function AccountModal() {
             </span>
             <span>Powered by MUD</span>
           </a>
+
+          <div className="absolute top-0 right-0">
+            <DialogClose
+              className={twMerge(
+                "pointer-events-auto leading-none p-2 transition",
+                "text-white/20 hover:text-white/40",
+              )}
+              title="Close"
+            >
+              <CloseIcon className="m-0" />
+            </DialogClose>
+          </div>
         </div>
       ) : null}
     </Modal>

packages/entrykit/src/ui/Modal.tsx

@@ -1,5 +1,11 @@
-import { ReactNode } from "react";
-import { Root as DialogRoot, DialogPortal, DialogContent } from "@radix-ui/react-dialog";
+import { ReactNode, useEffect } from "react";
+import {
+  Root as DialogRoot,
+  DialogPortal,
+  DialogContent,
+  DialogTitle,
+  DialogDescription,
+} from "@radix-ui/react-dialog";
 import { Shadow } from "./Shadow";
 import { twMerge } from "tailwind-merge";
 
@@ -10,6 +16,25 @@ export type Props = {
 };
 
 export function Modal({ open, onOpenChange, children }: Props) {
+  // Focus trapping doesn't seem to completely work with our iframe approach,
+  // so tabbing until you get to the document body means Escape doesn't work.
+  // We'll patch this behavior for now with our own listener.
+  useEffect(() => {
+    function onKeyDown(event: KeyboardEvent) {
+      if (event.defaultPrevented) return;
+
+      if (event.key === "Escape" && open) {
+        event.preventDefault();
+        onOpenChange?.(false);
+      }
+    }
+
+    window.addEventListener("keydown", onKeyDown);
+    return () => {
+      window.removeEventListener("keydown", onKeyDown);
+    };
+  }, [onOpenChange, open]);
+
   return (
     <DialogRoot open={open} onOpenChange={onOpenChange}>
       {/* This intentionally does not use `<DialogTrigger>` because it doesn't play nicely with `<Shadow>` trigger (our primary use case). */}
@@ -28,14 +53,10 @@ export function Modal({ open, onOpenChange, children }: Props) {
             )}
           >
             <div>
-              <DialogContent
-                className="outline-none w-full max-w-[26rem] mx-auto"
-                // TODO description
-                aria-describedby={undefined}
-                onOpenAutoFocus={(event) => {
-                  event.preventDefault();
-                }}
-              >
+              <DialogContent className="outline-none w-full max-w-[26rem] mx-auto">
+                <DialogTitle className="sr-only">EntryKit</DialogTitle>
+                <DialogDescription className="sr-only">Sign in to this app</DialogDescription>
+
                 {children}
               </DialogContent>
             </div>