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; });