Close field-free dialogs on outside click by default

e2e/scenarios/dialog-outside-dismiss.test.ts

@@ -0,0 +1,107 @@
+// Cross-target (browser): a dialog decides from its contents whether a click
+// outside closes it. While it holds a form field — including the one-time key
+// reveal's read-only inputs — a stray click on the page behind it must not
+// discard what the user typed or was shown. A field-free confirm dialog has
+// nothing to lose, so the same click dismisses it without firing its action.
+// The API keys page exercises both kinds in one flow.
+import { Effect } from "effect";
+import { expect } from "@effect/vitest";
+import { AccountApi, addGroup } from "@executor-js/api";
+
+import { scenario } from "../src/scenario";
+import { Api, Browser, Target } from "../src/services";
+import { visit } from "../src/surfaces/browser";
+
+const accountApi = addGroup(AccountApi);
+
+const KEY_NAME = "outside-click-check";
+
+scenario(
+  "Dialogs (UI) · Outside click keeps a form open, closes a field-free confirm",
+  {},
+  Effect.gen(function* () {
+    const target = yield* Target;
+    const browser = yield* Browser;
+    const { client: makeClient } = yield* Api;
+    const identity = yield* target.newIdentity();
+    const client = yield* makeClient(accountApi, identity);
+
+    const session = browser.session(identity, async ({ page, step }) => {
+      const createDialog = page.getByRole("dialog").filter({ hasText: "Create API key" });
+      const revokeDialog = page.getByRole("dialog").filter({ hasText: "Revoke API key" });
+      const nameInput = page.locator("#create-key-name");
+      const keyRow = page.getByText(KEY_NAME, { exact: true });
+      // The overlay covers the whole viewport and the dialog panel is centered,
+      // so the top-left corner is always outside the panel.
+      const clickOutside = async () => {
+        await page.mouse.click(8, 8);
+        // Dismissal is synchronous with the pointer event; this beat only gives
+        // a wrongly-dismissed dialog time to actually unmount before the
+        // "still open" assertions run.
+        await page.waitForTimeout(400);
+      };
+
+      await step("Open the create dialog and name the key", async () => {
+        await visit(page, "/api-keys");
+        await page.getByRole("button", { name: "New key" }).click();
+        await nameInput.waitFor();
+        await nameInput.fill(KEY_NAME);
+      });
+
+      await step("A stray outside click does not discard the form", async () => {
+        await clickOutside();
+        await createDialog.waitFor({ state: "visible" });
+        expect(await nameInput.inputValue(), "the typed name survives the stray click").toBe(
+          KEY_NAME,
+        );
+      });
+
+      await step("The one-time key reveal also survives an outside click", async () => {
+        await createDialog.getByRole("button", { name: "Create key" }).click();
+        await createDialog.getByText("It is only shown once.").waitFor();
+        await clickOutside();
+        await createDialog.getByText("It is only shown once.").waitFor({ state: "visible" });
+      });
+
+      await step("Closing the reveal deliberately lands the key in the list", async () => {
+        // "Close" also names the corner X; the footer's ghost button is the
+        // deliberate path under test.
+        await createDialog
+          .locator("[data-slot='dialog-footer']")
+          .getByRole("button", { name: "Close" })
+          .click();
+        await createDialog.waitFor({ state: "detached" });
+        await keyRow.waitFor();
+      });
+
+      await step("An outside click dismisses the field-free revoke confirm", async () => {
+        await page.getByRole("button", { name: `Revoke ${KEY_NAME}` }).click();
+        await revokeDialog.waitFor({ state: "visible" });
+        await clickOutside();
+        await revokeDialog.waitFor({ state: "detached" });
+        // Dismissal closed the dialog without firing the revoke.
+        await keyRow.waitFor({ state: "visible" });
+      });
+
+      await step("Confirming the revoke still removes the key", async () => {
+        await page.getByRole("button", { name: `Revoke ${KEY_NAME}` }).click();
+        await revokeDialog.getByRole("button", { name: "Revoke key" }).click();
+        await revokeDialog.waitFor({ state: "detached" });
+        await keyRow.waitFor({ state: "detached" });
+      });
+    });
+
+    // The final step revokes the key through the UI; this sweep only matters
+    // when a mid-test failure aborts the session before that step runs.
+    yield* Effect.ensuring(
+      session,
+      Effect.gen(function* () {
+        const { apiKeys } = yield* client.account.listApiKeys();
+        yield* Effect.forEach(
+          apiKeys.filter((key) => key.name === KEY_NAME),
+          (key) => client.account.revokeApiKey({ params: { apiKeyId: key.id } }),
+        );
+      }).pipe(Effect.ignore),
+    );
+  }),
+);

packages/react/src/components/dialog.tsx

@@ -2,6 +2,7 @@ import * as React from "react";
 import { XIcon } from "lucide-react";
 import { Dialog as DialogPrimitive } from "radix-ui";
 
+import { useComposedRef } from "../lib/compose-refs";
 import { applyOutsideDismissPolicy } from "../lib/outside-dismiss";
 import { cn } from "../lib/utils";
 import { Button } from "./button";
@@ -42,24 +43,28 @@ function DialogContent({
   className,
   children,
   showCloseButton = true,
-  dismissOnOutsideClick = false,
+  dismissOnOutsideClick,
   onInteractOutside,
   onPointerDownOutside,
   forceOverlay = false,
+  ref,
   ...props
 }: React.ComponentProps<typeof DialogPrimitive.Content> & {
   showCloseButton?: boolean;
-  /** Let a click outside the dialog close it. Off by default: a stray click on
-   *  the page behind a form must not discard what the user typed. Turn it on
-   *  for a dialog with nothing to lose — a confirmation, a picker, a read-only
-   *  panel. Escape and the close button close either way. */
+  /** Whether a click outside the dialog closes it. Unset, the dialog decides
+   *  from its contents: it stays open while it holds a form field — a stray
+   *  click on the page behind a form must not discard what the user typed —
+   *  and closes when it has nothing to lose. Pass a boolean to override the
+   *  detection. Escape and the close button close either way. */
   dismissOnOutsideClick?: boolean;
   /** Pair with `<Dialog modal={false}>`: Radix renders no overlay in non-modal
    *  mode, so this renders a plain dim layer instead. It still eats outside
    *  clicks, so the dialog keeps its modal look while the wheel stays free for
    *  portaled popups. */
   forceOverlay?: boolean;
 }) {
+  const contentRef = React.useRef<React.ComponentRef<typeof DialogPrimitive.Content> | null>(null);
+  const composedRef = useComposedRef(ref, contentRef);
   return (
     <DialogPortal data-slot="dialog-portal">
       <DialogOverlay />
@@ -68,13 +73,14 @@ function DialogContent({
       ) : null}
       <DialogPrimitive.Content
         data-slot="dialog-content"
+        ref={composedRef}
         onPointerDownOutside={(event) => {
           onPointerDownOutside?.(event);
-          applyOutsideDismissPolicy(event, dismissOnOutsideClick);
+          applyOutsideDismissPolicy(event, dismissOnOutsideClick, contentRef.current);
         }}
         onInteractOutside={(event) => {
           onInteractOutside?.(event);
-          applyOutsideDismissPolicy(event, dismissOnOutsideClick);
+          applyOutsideDismissPolicy(event, dismissOnOutsideClick, contentRef.current);
         }}
         className={cn(
           "fixed top-[50%] left-[50%] z-50 grid w-full max-w-[calc(100%-2rem)] translate-x-[-50%] translate-y-[-50%] gap-4 rounded-lg border bg-background p-6 shadow-lg duration-200 outline-none data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=closed]:zoom-out-95 data-[state=open]:animate-in data-[state=open]:fade-in-0 data-[state=open]:zoom-in-95 sm:max-w-lg",

packages/react/src/components/sheet.tsx

@@ -4,6 +4,7 @@ import * as React from "react";
 import { XIcon } from "lucide-react";
 import { Dialog as SheetPrimitive } from "radix-ui";
 
+import { useComposedRef } from "../lib/compose-refs";
 import { applyOutsideDismissPolicy } from "../lib/outside-dismiss";
 import { cn } from "../lib/utils";
 
@@ -44,31 +45,36 @@ function SheetContent({
   children,
   side = "right",
   showCloseButton = true,
-  dismissOnOutsideClick = false,
+  dismissOnOutsideClick,
   onInteractOutside,
   onPointerDownOutside,
+  ref,
   ...props
 }: React.ComponentProps<typeof SheetPrimitive.Content> & {
   side?: "top" | "right" | "bottom" | "left";
   showCloseButton?: boolean;
-  /** Let a click outside the sheet close it. Off by default: a stray click on
-   *  the page behind a form must not discard what the user typed. Turn it on
-   *  for a sheet with nothing to lose — navigation, a picker, a read-only
-   *  panel. Escape and the close button close either way. */
+  /** Whether a click outside the sheet closes it. Unset, the sheet decides
+   *  from its contents: it stays open while it holds a form field — a stray
+   *  click on the page behind a form must not discard what the user typed —
+   *  and closes when it has nothing to lose. Pass a boolean to override the
+   *  detection. Escape and the close button close either way. */
   dismissOnOutsideClick?: boolean;
 }) {
+  const contentRef = React.useRef<React.ComponentRef<typeof SheetPrimitive.Content> | null>(null);
+  const composedRef = useComposedRef(ref, contentRef);
   return (
     <SheetPortal>
       <SheetOverlay />
       <SheetPrimitive.Content
         data-slot="sheet-content"
+        ref={composedRef}
         onPointerDownOutside={(event) => {
           onPointerDownOutside?.(event);
-          applyOutsideDismissPolicy(event, dismissOnOutsideClick);
+          applyOutsideDismissPolicy(event, dismissOnOutsideClick, contentRef.current);
         }}
         onInteractOutside={(event) => {
           onInteractOutside?.(event);
-          applyOutsideDismissPolicy(event, dismissOnOutsideClick);
+          applyOutsideDismissPolicy(event, dismissOnOutsideClick, contentRef.current);
         }}
         className={cn(
           "fixed z-50 flex flex-col gap-4 bg-background shadow-lg transition ease-in-out data-[state=closed]:animate-out data-[state=closed]:duration-300 data-[state=open]:animate-in data-[state=open]:duration-500",

packages/react/src/lib/compose-refs.test.ts

@@ -0,0 +1,68 @@
+import { describe, expect, it } from "@effect/vitest";
+
+import { composedRefCallback } from "./compose-refs";
+
+/**
+ * The contract under test: both refs always see the same node, across the
+ * three shapes an incoming ref can take (object, plain callback, React 19
+ * callback with cleanup). The cleanup case matters most — React never calls
+ * the callback with `null` when a cleanup was returned, so clearing the local
+ * ref has to happen inside the merged cleanup or it dangles forever.
+ */
+describe("composedRefCallback", () => {
+  const node = { nodeName: "DIV" };
+
+  it("tracks attach and detach in both an object ref and the local ref", () => {
+    const outer: { current: typeof node | null } = { current: null };
+    const local: { current: typeof node | null } = { current: null };
+    const callback = composedRefCallback(outer, local);
+
+    expect(callback(node)).toBeUndefined();
+    expect(outer.current).toBe(node);
+    expect(local.current).toBe(node);
+
+    callback(null);
+    expect(outer.current).toBeNull();
+    expect(local.current).toBeNull();
+  });
+
+  it("forwards attach and detach to a plain callback ref", () => {
+    const seen: Array<typeof node | null> = [];
+    const local: { current: typeof node | null } = { current: null };
+    const callback = composedRefCallback((value: typeof node | null) => {
+      seen.push(value);
+    }, local);
+
+    expect(callback(node)).toBeUndefined();
+    callback(null);
+    expect(seen).toEqual([node, null]);
+    expect(local.current).toBeNull();
+  });
+
+  it("preserves a React 19 cleanup and clears the local ref inside it", () => {
+    let cleanedUp = false;
+    const local: { current: typeof node | null } = { current: null };
+    const callback = composedRefCallback(() => {
+      return () => {
+        cleanedUp = true;
+      };
+    }, local);
+
+    const cleanup = callback(node);
+    expect(local.current).toBe(node);
+    expect(typeof cleanup).toBe("function");
+
+    (cleanup as () => void)();
+    expect(cleanedUp).toBe(true);
+    expect(local.current).toBeNull();
+  });
+
+  it("still tracks the local ref when no outer ref is given", () => {
+    const local: { current: typeof node | null } = { current: null };
+    const callback = composedRefCallback(undefined, local);
+    callback(node);
+    expect(local.current).toBe(node);
+    callback(null);
+    expect(local.current).toBeNull();
+  });
+});

packages/react/src/lib/compose-refs.ts

@@ -0,0 +1,33 @@
+import * as React from "react";
+
+/**
+ * One callback ref that keeps an incoming ref and a local ref on the same node.
+ *
+ * A React 19 callback ref may return a cleanup function; when the incoming ref
+ * does, React runs that cleanup instead of calling the callback again with
+ * `null`, so the local ref must be cleared inside the cleanup too.
+ */
+export const composedRefCallback =
+  <T>(outer: React.Ref<T> | undefined, local: React.RefObject<T | null>): React.RefCallback<T> =>
+  (node) => {
+    local.current = node;
+    if (typeof outer === "function") {
+      const cleanup = outer(node);
+      if (typeof cleanup === "function") {
+        return () => {
+          local.current = null;
+          cleanup();
+        };
+      }
+      return undefined;
+    }
+    if (outer) outer.current = node;
+    return undefined;
+  };
+
+/** `composedRefCallback`, kept stable across renders for stable inputs so the
+ *  incoming ref is not detached and reattached on every render. */
+export const useComposedRef = <T>(
+  outer: React.Ref<T> | undefined,
+  local: React.RefObject<T | null>,
+): React.RefCallback<T> => React.useMemo(() => composedRefCallback(outer, local), [outer, local]);