From d42fc053ddda8c7e548fad749893cf6b5e1c014f Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Thu, 29 Oct 2026 15:35:36 +0100 Subject: [PATCH] fix: let Escape close a dialog right after its form closed --- hooks/__tests__/use-focus-trap.test.tsx | 20 ++++++++++++++++++++ hooks/use-focus-trap.ts | 7 +++++-- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/hooks/__tests__/use-focus-trap.test.tsx b/hooks/__tests__/use-focus-trap.test.tsx index f820f8820..d9243c2e7 100644 --- a/hooks/__tests__/use-focus-trap.test.tsx +++ b/hooks/__tests__/use-focus-trap.test.tsx @@ -157,3 +157,23 @@ describe('useFocusTrap and the page behind', () => { } }); }); + +// Presses Escape in its layout effect: after the DOM of a render is in +// place, before that render's passive effects have run. +function EscapeOnCommit({ press }: { press: number }) { + React.useLayoutEffect(() => { + if (press) fireEvent.keyDown(document.body, { key: 'Escape' }); + }, [press]); + return null; +} + +describe('useFocusTrap handler timing', () => { + it('answers an Escape right after a render with that render\'s handler', () => { + const before = vi.fn(); + const after = vi.fn(); + const { rerender } = render(<>); + rerender(<>); + expect(after).toHaveBeenCalledTimes(1); + expect(before).not.toHaveBeenCalled(); + }); +}); diff --git a/hooks/use-focus-trap.ts b/hooks/use-focus-trap.ts index e80339591..5a37d266b 100644 --- a/hooks/use-focus-trap.ts +++ b/hooks/use-focus-trap.ts @@ -1,4 +1,4 @@ -import { useEffect, useRef } from 'react'; +import { useEffect, useLayoutEffect, useRef } from 'react'; // The traps that are active, the one opened last at the end. Only that one // answers Escape, so a dialog over a dialog closes alone. @@ -24,9 +24,12 @@ export function useFocusTrap({ // one from here instead of starting over for it: each new start moved the // focus back to the opener and then to the first element, out of the field // the user was typing in whenever the page behind rendered again. + // Updated during the commit, not after it: an Escape pressed once the + // DOM has changed must reach the handler of that render, not the one + // before (a form that just closed would ask about its input again). const onEscapeRef = useRef(onEscape); const initialFocusRef = useRef(initialFocus); - useEffect(() => { + useLayoutEffect(() => { onEscapeRef.current = onEscape; initialFocusRef.current = initialFocus; });