diff --git a/.changeset/dismissable-escape-sync.md b/.changeset/dismissable-escape-sync.md new file mode 100644 index 0000000000..b6a3bc4e32 --- /dev/null +++ b/.changeset/dismissable-escape-sync.md @@ -0,0 +1,7 @@ +--- +"@zag-js/dismissable": patch +--- + +Fix issue where the escape key handler and layer registration were deferred by a frame when `defer: true`, leaving a just-opened dialog undismissable via the escape key until the next animation frame. + +`trackDismissableElement` now resolves the node eagerly and registers handlers synchronously when the node is available. The deferred pass remains only as a fallback for nodes that have not rendered yet. Interact-outside behavior is unchanged, since it applies its own deferral internally. diff --git a/packages/utilities/dismissable/src/dismissable-layer.ts b/packages/utilities/dismissable/src/dismissable-layer.ts index cd56c03c7e..031df729f0 100644 --- a/packages/utilities/dismissable/src/dismissable-layer.ts +++ b/packages/utilities/dismissable/src/dismissable-layer.ts @@ -165,14 +165,25 @@ function trackDismissableElementImpl(node: MaybeElement, options: DismissableEle export function trackDismissableElement(nodeOrFn: NodeOrFn, options: DismissableElementOptions) { const { defer } = options - const func = defer ? raf : (v: any) => v() const cleanups: (VoidFunction | undefined)[] = [] - cleanups.push( - func(() => { - const node = isFunction(nodeOrFn) ? nodeOrFn() : nodeOrFn - cleanups.push(trackDismissableElementImpl(node, options)) - }), - ) + + // Resolve the node eagerly so the layer (and its escape keydown handler) is registered + // synchronously whenever possible. `defer` only needs to gate node resolution; the + // interact-outside logic applies its own deferral internally (via `options.defer`), + // so registering early cannot cause the opening click to dismiss the layer. + const node = isFunction(nodeOrFn) ? nodeOrFn() : nodeOrFn + + if (!defer || node) { + cleanups.push(trackDismissableElementImpl(node, options)) + } else { + cleanups.push( + raf(() => { + const deferredNode = isFunction(nodeOrFn) ? nodeOrFn() : nodeOrFn + cleanups.push(trackDismissableElementImpl(deferredNode, options)) + }), + ) + } + return () => { cleanups.forEach((fn) => fn?.()) } diff --git a/packages/utilities/dismissable/tests/dismissable-layer.test.ts b/packages/utilities/dismissable/tests/dismissable-layer.test.ts new file mode 100644 index 0000000000..019db30da5 --- /dev/null +++ b/packages/utilities/dismissable/tests/dismissable-layer.test.ts @@ -0,0 +1,133 @@ +// @vitest-environment jsdom + +import { afterEach, describe, expect, test, vi } from "vitest" +import { trackDismissableElement } from "../src/dismissable-layer" +import { layerStack } from "../src/layer-stack" + +function dispatchEscape(target: EventTarget = document) { + const event = new KeyboardEvent("keydown", { key: "Escape", bubbles: true, cancelable: true }) + target.dispatchEvent(event) + return event +} + +function nextFrame() { + return new Promise((resolve) => { + requestAnimationFrame(() => resolve()) + }) +} + +/** Drain the double-rAF used by `nextTick` in `layerStack.remove` */ +function drainRemoval() { + return new Promise((resolve) => { + requestAnimationFrame(() => { + requestAnimationFrame(() => resolve()) + }) + }) +} + +describe("trackDismissableElement", () => { + const cleanups: VoidFunction[] = [] + + function track(...args: Parameters) { + const cleanup = trackDismissableElement(...args) + cleanups.push(cleanup) + return cleanup + } + + afterEach(async () => { + cleanups.splice(0).forEach((fn) => fn()) + document.body.innerHTML = "" + for (let i = 0; i < 3; i++) { + await drainRemoval() + } + }) + + test("escape key dismisses immediately when `defer: true` and node is available", () => { + const node = document.createElement("div") + document.body.appendChild(node) + + const onDismiss = vi.fn() + track(() => node, { defer: true, onDismiss }) + + // no frame has elapsed yet; escape must already be wired up + dispatchEscape() + expect(onDismiss).toHaveBeenCalledTimes(1) + }) + + test("registers the layer on the stack synchronously when node is available", () => { + const node = document.createElement("div") + document.body.appendChild(node) + + track(() => node, { defer: true, onDismiss: vi.fn() }) + expect(layerStack.isTopMost(node)).toBe(true) + }) + + test("falls back to deferred registration when node is not yet available", async () => { + let node: HTMLElement | null = null + const onDismiss = vi.fn() + track(() => node, { defer: true, onDismiss }) + + // node commits after the effect ran (the case `defer` exists for) + node = document.createElement("div") + document.body.appendChild(node) + + await nextFrame() + + dispatchEscape() + expect(onDismiss).toHaveBeenCalledTimes(1) + }) + + test("escape only dismisses the topmost layer", () => { + const parent = document.createElement("div") + const child = document.createElement("div") + document.body.append(parent, child) + + const dismissParent = vi.fn() + const dismissChild = vi.fn() + + track(() => parent, { defer: true, onDismiss: dismissParent }) + track(() => child, { defer: true, onDismiss: dismissChild }) + + dispatchEscape() + expect(dismissChild).toHaveBeenCalledTimes(1) + expect(dismissParent).not.toHaveBeenCalled() + }) + + test("cleanup removes the escape handler", () => { + const node = document.createElement("div") + document.body.appendChild(node) + + const onDismiss = vi.fn() + const cleanup = track(() => node, { defer: true, onDismiss }) + + cleanup() + dispatchEscape() + expect(onDismiss).not.toHaveBeenCalled() + }) + + test("cleanup before the deferred frame cancels registration", async () => { + let node: HTMLElement | null = null + const onDismiss = vi.fn() + const cleanup = track(() => node, { defer: true, onDismiss }) + + cleanup() + + node = document.createElement("div") + document.body.appendChild(node) + await nextFrame() + + dispatchEscape() + expect(onDismiss).not.toHaveBeenCalled() + }) + + test("without `defer`, registration is synchronous (unchanged behavior)", () => { + const node = document.createElement("div") + document.body.appendChild(node) + + const onDismiss = vi.fn() + track(node, { onDismiss }) + + dispatchEscape() + expect(onDismiss).toHaveBeenCalledTimes(1) + }) +})