From 4cccd40e2ab8462fda7d9b24abe696b2dca706ec Mon Sep 17 00:00:00 2001 From: Blomios Date: Tue, 7 Jul 2026 08:28:08 +0200 Subject: [PATCH] =?UTF-8?q?fix(tickets):=20corrige=20la=20perte=20de=20foc?= =?UTF-8?q?us=20dans=20l'=C3=A9dition=20d'un=20ticket=20(#17)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Le focus-trap de FloatingWindow avait un useEffect dépendant de `[onClose]`. `onClose` (ex. TicketDetail.requestClose) est une closure recréée à chaque render : chaque frappe ré-armait l'effet et renvoyait le focus au premier élément focusable, ne laissant saisir qu'une lettre à la fois dans le formulaire d'édition. Correctif : conserver le dernier `onClose` dans un `onCloseRef` et passer l'effet focus-trap en mount-only (`[]`). Le handler Escape lit la valeur courante via la ref. Ajout d'un test de régression prouvé rouge-sans / vert-avec le fix. Co-Authored-By: Claude Opus 4.8 --- .../src/shared/ui/FloatingWindow.test.tsx | 33 +++++++++++++++++++ frontend/src/shared/ui/FloatingWindow.tsx | 16 +++++++-- 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/frontend/src/shared/ui/FloatingWindow.test.tsx b/frontend/src/shared/ui/FloatingWindow.test.tsx index d50af5b..bd58778 100644 --- a/frontend/src/shared/ui/FloatingWindow.test.tsx +++ b/frontend/src/shared/ui/FloatingWindow.test.tsx @@ -128,6 +128,39 @@ describe("FloatingWindow (#16, G1)", () => { expect(onClose).not.toHaveBeenCalled(); }); + it("keeps focus on a controlled input across renders that change onClose identity (#17 focus-loss bug)", () => { + // Reproduces the reported bug: a caller re-creates its `onClose` closure on + // every render (as TicketDetail's `requestClose` does). If the focus-trap + // effect re-ran on that identity change, focus would jump back to the first + // focusable after each keystroke — only one character could be typed. + function Harness() { + const [value, setValue] = useState(""); + // New closure identity on every render, like an inline/rebuilt handler. + const onClose = () => {}; + return ( + + setValue(e.target.value)} + /> + + ); + } + render(); + const field = screen.getByRole("textbox", { name: "field" }); + field.focus(); + expect(document.activeElement).toBe(field); + + fireEvent.change(field, { target: { value: "a" } }); + expect(document.activeElement).toBe(field); + fireEvent.change(field, { target: { value: "ab" } }); + fireEvent.change(field, { target: { value: "abc" } }); + // Focus never left the field, so multi-character input works. + expect(document.activeElement).toBe(field); + expect((field as HTMLInputElement).value).toBe("abc"); + }); + it("restores focus to the opener when closed", () => { function Harness() { const [open, setOpen] = useState(false); diff --git a/frontend/src/shared/ui/FloatingWindow.tsx b/frontend/src/shared/ui/FloatingWindow.tsx index 39dee57..bb4df13 100644 --- a/frontend/src/shared/ui/FloatingWindow.tsx +++ b/frontend/src/shared/ui/FloatingWindow.tsx @@ -80,6 +80,15 @@ function FloatingWindowBody({ }: Omit) { const dialogRef = useRef(null); + // Keep the latest `onClose` in a ref so the mount-only effect below never + // re-runs when the caller passes an unstable `onClose` closure (a new + // identity on each parent render). Re-running the effect would move focus + // back to the first focusable element on every keystroke, so a controlled + // input inside the window could only accept one character before losing + // focus. The Escape handler reads the current value via this ref. + const onCloseRef = useRef(onClose); + onCloseRef.current = onClose; + useEffect(() => { const previouslyFocused = document.activeElement as HTMLElement | null; const node = dialogRef.current; @@ -96,7 +105,7 @@ function FloatingWindowBody({ if (!isTopmost()) return; if (e.key === "Escape") { e.preventDefault(); - onClose(); + onCloseRef.current(); return; } if (e.key !== "Tab" || !node) return; @@ -128,7 +137,10 @@ function FloatingWindowBody({ if (i >= 0) windowStack.splice(i, 1); previouslyFocused?.focus?.(); }; - }, [onClose]); + // Mount-only: focus save/restore and listener wiring must not re-run when + // `onClose` changes identity (see `onCloseRef` above). + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); return (