Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/dismissable-escape-sync.md
Original file line number Diff line number Diff line change
@@ -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.
25 changes: 18 additions & 7 deletions packages/utilities/dismissable/src/dismissable-layer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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?.())
}
Expand Down
133 changes: 133 additions & 0 deletions packages/utilities/dismissable/tests/dismissable-layer.test.ts
Original file line number Diff line number Diff line change
@@ -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<void>((resolve) => {
requestAnimationFrame(() => resolve())
})
}

/** Drain the double-rAF used by `nextTick` in `layerStack.remove` */
function drainRemoval() {
return new Promise<void>((resolve) => {
requestAnimationFrame(() => {
requestAnimationFrame(() => resolve())
})
})
}

describe("trackDismissableElement", () => {
const cleanups: VoidFunction[] = []

function track(...args: Parameters<typeof trackDismissableElement>) {
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)
})
})