From 4191189b4c8c5b1675637f5dd0fb2c245e39140e Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sun, 4 Oct 2026 04:16:52 +0200 Subject: [PATCH] fix(ui): a late form takes a dialog's focus once, even if it arrived elsewhere The watch stayed armed when the form arrived while the focus was off the close button, so a later change in the body pulled the focus off a close button the user went back to. It now disarms on arrival and re-arms only when the focus is taken away. --- .../components/ui/Modal.purpose.dom.test.tsx | 22 +++++++++++++++++++ frontend/src/components/ui/Modal.tsx | 17 +++++++++----- 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/frontend/src/components/ui/Modal.purpose.dom.test.tsx b/frontend/src/components/ui/Modal.purpose.dom.test.tsx index 4afb444d..0ec7e197 100644 --- a/frontend/src/components/ui/Modal.purpose.dom.test.tsx +++ b/frontend/src/components/ui/Modal.purpose.dom.test.tsx @@ -116,6 +116,28 @@ describe("Modal initial focus by purpose (C4)", () => { expect(document.activeElement).toBe(cancel); }); + // …and it stays the user's: the form disarms the watch when it arrives, so a later change in the + // body (a hint appearing as they type) cannot pull the focus off a ✕ they went back to. + it("field that loads late: once arrived, it never takes the focus off the ✕ again", async () => { + const view = (ready: boolean, hint = false) => ( + {}}> + + {hint &&

hint

} + + + +
+ ); + const { rerender } = render(view(false)); + act(() => screen.getByRole("button", { name: "cancel" }).focus()); + rerender(view(true)); + await act(async () => {}); + act(() => closeBtn().focus()); + rerender(view(true, true)); + await act(async () => {}); + expect(document.activeElement).toBe(closeBtn()); + }); + it("close: the ✕, even with a field in the body", () => { render( {}}> diff --git a/frontend/src/components/ui/Modal.tsx b/frontend/src/components/ui/Modal.tsx index 68a86364..b4a0b955 100644 --- a/frontend/src/components/ui/Modal.tsx +++ b/frontend/src/components/ui/Modal.tsx @@ -106,9 +106,12 @@ export default function Modal({ // A form that loads after the dialog opens (the issue composer shows a spinner first) has no field // to take the focus at open, so it lands on the ✕. When the first control arrives the focus moves // there — but only while it still rests on the ✕: a user who has moved it keeps it where it is. - // The watch re-arms when the focused control is taken away (a failed load's Retry giving way to - // the spinner again): the focus fell to , so it waits on the ✕ for the form (E32 review). - // Only : focus in a dialog stacked on top is not this one's to take. + // The form takes it ONCE: arrived while the focus was elsewhere, it disarms all the same, or a later + // mutation would pull the focus off a ✕ the user went back to (E32 review round 2). The watch + // re-arms only when the focus is taken away, not moved: the body has no control again, or the + // focused one went (a failed load's Retry giving way to the spinner, or straight to the form) and + // the focus fell to — it then waits on the ✕ for the form (E32 review). Only : focus + // in a dialog stacked on top is not this one's to take. useEffect(() => { const body = bodyRef.current; if (initialFocus !== "field" || !body || focusablesIn(body).length > 0) return; @@ -119,9 +122,13 @@ export default function Modal({ armed = true; } const first = focusablesIn(body)[0]; - if (!armed || !first || document.activeElement !== closeRef.current) return; + if (!first) { + armed = true; + return; + } + if (!armed) return; armed = false; - first.focus(); + if (document.activeElement === closeRef.current) first.focus(); }); watch.observe(body, { childList: true, subtree: true }); return () => watch.disconnect();