fix(web): stop a closed dialog from leaving the page click-dead (#2056)

2026-07-30 23:24:10 +08:00
parent 9b6d976f3b
commit 2f84e8f3b1
7 changed files with 328 additions and 14 deletions
@@ -7,22 +7,42 @@ import { cn } from "./lib/utils";
 import { Button } from "./button";
 import { IconX } from "@tabler/icons-react";
 import { handleDialogDefaultActionKeyDown } from "./lib/dialog-default-action";
+import {
+  DIALOG_CLOSE_RECOVERY_MS,
+  finishClosingDialogAnimations,
+  subscribeDialogCloseRecovery,
+} from "./lib/dialog-body-lock";

-function Dialog({ ...props }: React.ComponentProps<typeof DialogPrimitive.Root>) {
-  React.useEffect(() => {
-    return () => {
-      // Safety cleanup: Radix Dialog sets pointer-events: none on body when
-      // modal. If the dialog unmounts mid-close (e.g. onOpenChange(false)
-      // followed immediately by router.push), Radix never finishes its
-      // animation-based cleanup. Check the actual body state rather than
-      // tracking the open prop, which may already be false by unmount time.
-      if (document.body.style.pointerEvents === "none") {
-        document.body.style.removeProperty("pointer-events");
-      }
-    };
-  }, []);
+function Dialog({ onOpenChange, ...props }: React.ComponentProps<typeof DialogPrimitive.Root>) {
+  const finishClosingAnimations = React.useCallback(
+    () => finishClosingDialogAnimations(document),
+    [],
+  );

-  return <DialogPrimitive.Root data-slot="dialog" {...props} />;
+  // Unmount is one way to strand the lock — Radix's animation-based cleanup
+  // never runs when the dialog disappears mid-close.
+  React.useEffect(
+    () => () => {
+      finishClosingAnimations();
+    },
+    [finishClosingAnimations],
+  );
+
+  // Becoming visible again is the other. Dialog roots share one document-level
+  // listener because the body lock and closing portals are document-global.
+  React.useEffect(() => subscribeDialogCloseRecovery(document), []);
+
+  // And the general case: once a close has been requested, the page must be
+  // usable shortly afterwards whether or not animationend ever arrives.
+  const handleOpenChange = React.useCallback(
+    (open: boolean) => {
+      onOpenChange?.(open);
+      if (!open) window.setTimeout(finishClosingAnimations, DIALOG_CLOSE_RECOVERY_MS);
+    },
+    [finishClosingAnimations, onOpenChange],
+  );
+
+  return <DialogPrimitive.Root data-slot="dialog" onOpenChange={handleOpenChange} {...props} />;
 }

 function DialogTrigger({ ...props }: React.ComponentProps<typeof DialogPrimitive.Trigger>) {
@@ -0,0 +1,73 @@
+/**
+ * Recovery for a stuck modal body lock.
+ *
+ * Radix sets `pointer-events: none` on <body> while a modal dialog is open and
+ * releases it when the content unmounts. Presence defers that unmount until the
+ * exit animation fires `animationend` — so anything that stops that event from
+ * arriving leaves every control on the page dead, with no visible dialog to
+ * close. Two ways in, both observed:
+ *
+ *   - the dialog unmounts mid-close (onOpenChange(false) then router.push), so
+ *     Radix's own cleanup never runs;
+ *   - the document is not being rendered when the dialog closes, which freezes
+ *     CSS animation timelines. `document.timeline.currentTime` stays at 0 and a
+ *     100ms exit animation reports playState "running" indefinitely.
+ *
+ * Recovery finishes only closing dialog animations. The resulting
+ * `animationend` event lets Radix Presence unmount the closing owner and release
+ * its own pointer and scroll-lock bookkeeping.
+ */
+
+/** Grace period after a close before sweeping. Must exceed the exit duration. */
+export const DIALOG_CLOSE_RECOVERY_MS = 400;
+
+const CLOSING_DIALOG_PARTS = [
+  '[data-state="closed"][data-slot="dialog-content"]',
+  '[data-state="closed"][data-slot="dialog-overlay"]',
+].join(",");
+
+type VisibilitySubscription = {
+  subscribers: number;
+  onVisibilityChange: () => void;
+};
+
+const visibilitySubscriptions = new WeakMap<Document, VisibilitySubscription>();
+
+/**
+ * Ask Radix Presence to finish closing dialog owners through its normal
+ * `animationend` path. Returns true when at least one animation was finished.
+ */
+export function finishClosingDialogAnimations(doc: Document): boolean {
+  let finished = false;
+  for (const element of doc.querySelectorAll<HTMLElement>(CLOSING_DIALOG_PARTS)) {
+    for (const animation of element.getAnimations()) {
+      animation.finish();
+      finished = true;
+    }
+  }
+  return finished;
+}
+
+/** Share one document-level foreground recovery listener across dialog roots. */
+export function subscribeDialogCloseRecovery(doc: Document): () => void {
+  let subscription = visibilitySubscriptions.get(doc);
+  if (!subscription) {
+    const onVisibilityChange = () => {
+      if (doc.visibilityState === "visible") finishClosingDialogAnimations(doc);
+    };
+    subscription = { subscribers: 0, onVisibilityChange };
+    visibilitySubscriptions.set(doc, subscription);
+    doc.addEventListener("visibilitychange", onVisibilityChange);
+  }
+  subscription.subscribers += 1;
+
+  let subscribed = true;
+  return () => {
+    if (!subscribed) return;
+    subscribed = false;
+    subscription.subscribers -= 1;
+    if (subscription.subscribers > 0) return;
+    doc.removeEventListener("visibilitychange", subscription.onVisibilityChange);
+    visibilitySubscriptions.delete(doc);
+  };
+}
@@ -0,0 +1,68 @@
+import { expect, type Page } from "@playwright/test";
+import { KanbanPage } from "../../pages/kanban-page";
+
+async function bodyLockState(page: Page) {
+  return page.evaluate(() => ({
+    pointerEvents: document.body.style.pointerEvents,
+    scrollLocked: document.body.hasAttribute("data-scroll-locked"),
+  }));
+}
+
+export async function verifyStalledDialogCloseRecovery(page: Page, touch: boolean) {
+  const kanban = new KanbanPage(page);
+  await kanban.goto();
+  await page.addStyleTag({
+    content: `
+      [data-state="closed"][data-slot="dialog-content"],
+      [data-state="closed"][data-slot="dialog-overlay"] {
+        animation-duration: 60s !important;
+      }
+    `,
+  });
+  await page.evaluate(() => {
+    const accordion = document.createElement("div");
+    accordion.dataset.slot = "accordion-content";
+    accordion.dataset.state = "open";
+    accordion.dataset.testid = "body-lock-open-accordion";
+    document.body.appendChild(accordion);
+  });
+
+  const createTask = touch
+    ? page.getByTestId("mobile-fab")
+    : kanban.createTaskButton.filter({ visible: true }).first();
+  if (touch) await createTask.tap();
+  else await createTask.click();
+
+  const dialog = page.getByTestId("create-task-dialog");
+  await expect(dialog).toBeVisible();
+  await expect
+    .poll(() => bodyLockState(page))
+    .toEqual({
+      pointerEvents: "none",
+      scrollLocked: true,
+    });
+
+  const close = dialog.getByRole("button", { name: "Close" });
+  if (touch) await close.tap();
+  else await close.click();
+
+  await expect(dialog).not.toBeAttached();
+  await expect(page.locator('[data-slot="dialog-overlay"]')).not.toBeAttached();
+  await expect
+    .poll(() => bodyLockState(page))
+    .toEqual({
+      pointerEvents: "",
+      scrollLocked: false,
+    });
+
+  if (touch) await createTask.tap();
+  else await createTask.click();
+
+  await expect(dialog).toBeVisible();
+  await expect
+    .poll(() => bodyLockState(page))
+    .toEqual({
+      pointerEvents: "none",
+      scrollLocked: true,
+    });
+}
@@ -0,0 +1,6 @@
+import { test } from "../../fixtures/test-base";
+import { verifyStalledDialogCloseRecovery } from "./dialog-body-lock-helpers";
+
+test("a stalled dialog close releases and can reacquire the body lock", async ({ testPage }) => {
+  await verifyStalledDialogCloseRecovery(testPage, false);
+});
@@ -0,0 +1,6 @@
+import { test } from "../../fixtures/test-base";
+import { verifyStalledDialogCloseRecovery } from "./dialog-body-lock-helpers";
+
+test("a stalled dialog close releases and can reacquire the body lock", async ({ testPage }) => {
+  await verifyStalledDialogCloseRecovery(testPage, true);
+});
@@ -0,0 +1,31 @@
+import { afterEach, describe, expect, it, vi } from "vitest";
+import { render } from "@testing-library/react";
+import { Dialog } from "@kandev/ui/dialog";
+
+afterEach(() => {
+  vi.restoreAllMocks();
+});
+
+describe("Dialog body-lock recovery subscription", () => {
+  it("shares one visibility listener across mounted dialog roots", () => {
+    const addEventListener = vi.spyOn(document, "addEventListener");
+    const removeEventListener = vi.spyOn(document, "removeEventListener");
+
+    const view = render(
+      <>
+        <Dialog />
+        <Dialog />
+      </>,
+    );
+
+    expect(
+      addEventListener.mock.calls.filter(([type]) => type === "visibilitychange"),
+    ).toHaveLength(1);
+
+    view.unmount();
+
+    expect(
+      removeEventListener.mock.calls.filter(([type]) => type === "visibilitychange"),
+    ).toHaveLength(1);
+  });
+});
@@ -0,0 +1,110 @@
+import { describe, it, expect, beforeEach, vi } from "vitest";
+import { finishClosingDialogAnimations } from "@kandev/ui/lib/dialog-body-lock";
+
+/**
+ * Unit coverage for the modal body-lock recovery used by the base Dialog.
+ *
+ * Radix releases `pointer-events: none` on <body> when the dialog content
+ * unmounts, and Presence defers that unmount until the exit animation fires
+ * `animationend`. When that event never arrives — the document was hidden while
+ * closing, so animation timelines were frozen, or the dialog unmounted mid-close
+ * — the lock outlives its dialog and every control on the page stops responding
+ * with nothing visible to close. These tests pin the two halves of the contract:
+ * recovery terminates only closing modal animations and leaves Radix to release
+ * its own body-lock bookkeeping when those modal owners unmount.
+ */
+const SCROLL_LOCKED = "data-scroll-locked";
+const DIALOG_CONTENT = "dialog-content";
+
+function lockBody() {
+  document.body.style.pointerEvents = "none";
+  document.body.setAttribute(SCROLL_LOCKED, "1");
+}
+
+function isLocked() {
+  return document.body.style.pointerEvents === "none" || document.body.hasAttribute(SCROLL_LOCKED);
+}
+
+function animation() {
+  return { finish: vi.fn() } as unknown as Animation;
+}
+
+/** Mirrors modal/content primitives so selectors see realistic markup. */
+function mountContent(
+  state: "open" | "closed",
+  slot = DIALOG_CONTENT,
+  animations: Animation[] = [],
+) {
+  const el = document.createElement("div");
+  el.setAttribute("role", "dialog");
+  el.setAttribute("data-slot", slot);
+  el.setAttribute("data-state", state);
+  el.getAnimations = () => animations;
+  document.body.appendChild(el);
+  return el;
+}
+
+beforeEach(() => {
+  document.body.innerHTML = "";
+  document.body.removeAttribute(SCROLL_LOCKED);
+  document.body.style.removeProperty("pointer-events");
+});
+
+describe("finishClosingDialogAnimations", () => {
+  it("finishes a closing dialog animation without mutating its owner's body lock", () => {
+    const exit = animation();
+    mountContent("closed", DIALOG_CONTENT, [exit]);
+    lockBody();
+
+    expect(finishClosingDialogAnimations(document)).toBe(true);
+    expect(exit.finish).toHaveBeenCalledOnce();
+    expect(isLocked()).toBe(true);
+  });
+
+  it("finishes a closing overlay so an invisible portal cannot intercept clicks", () => {
+    const exit = animation();
+    mountContent("closed", "dialog-overlay", [exit]);
+
+    expect(finishClosingDialogAnimations(document)).toBe(true);
+    expect(exit.finish).toHaveBeenCalledOnce();
+  });
+
+  it("ignores open dialog animations", () => {
+    const open = animation();
+    mountContent("open", DIALOG_CONTENT, [open]);
+    lockBody();
+
+    expect(finishClosingDialogAnimations(document)).toBe(false);
+    expect(open.finish).not.toHaveBeenCalled();
+    expect(isLocked()).toBe(true);
+  });
+
+  it("finishes the closing owner while preserving a second open modal's lock", () => {
+    const closing = animation();
+    const open = animation();
+    mountContent("closed", DIALOG_CONTENT, [closing]);
+    mountContent("open", "alert-dialog-content", [open]);
+    lockBody();
+
+    expect(finishClosingDialogAnimations(document)).toBe(true);
+    expect(closing.finish).toHaveBeenCalledOnce();
+    expect(open.finish).not.toHaveBeenCalled();
+    expect(isLocked()).toBe(true);
+  });
+
+  it("does not let unrelated open content suppress dialog recovery", () => {
+    const closing = animation();
+    mountContent("closed", DIALOG_CONTENT, [closing]);
+    mountContent("open", "accordion-content");
+    mountContent("open", "collapsible-content");
+    lockBody();
+
+    expect(finishClosingDialogAnimations(document)).toBe(true);
+    expect(closing.finish).toHaveBeenCalledOnce();
+    expect(isLocked()).toBe(true);
+  });
+
+  it("reports nothing to do without a closing dialog animation", () => {
+    expect(finishClosingDialogAnimations(document)).toBe(false);
+  });
+});