fix(tickets): corrige la perte de focus dans l'édition d'un ticket (#17)
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 <noreply@anthropic.com>
This commit is contained in:
@ -128,6 +128,39 @@ describe("FloatingWindow (#16, G1)", () => {
|
|||||||
expect(onClose).not.toHaveBeenCalled();
|
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 (
|
||||||
|
<FloatingWindow open title="Panel" onClose={onClose}>
|
||||||
|
<input
|
||||||
|
aria-label="field"
|
||||||
|
value={value}
|
||||||
|
onChange={(e) => setValue(e.target.value)}
|
||||||
|
/>
|
||||||
|
</FloatingWindow>
|
||||||
|
);
|
||||||
|
}
|
||||||
|
render(<Harness />);
|
||||||
|
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", () => {
|
it("restores focus to the opener when closed", () => {
|
||||||
function Harness() {
|
function Harness() {
|
||||||
const [open, setOpen] = useState(false);
|
const [open, setOpen] = useState(false);
|
||||||
|
|||||||
@ -80,6 +80,15 @@ function FloatingWindowBody({
|
|||||||
}: Omit<FloatingWindowProps, "open">) {
|
}: Omit<FloatingWindowProps, "open">) {
|
||||||
const dialogRef = useRef<HTMLDivElement>(null);
|
const dialogRef = useRef<HTMLDivElement>(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(() => {
|
useEffect(() => {
|
||||||
const previouslyFocused = document.activeElement as HTMLElement | null;
|
const previouslyFocused = document.activeElement as HTMLElement | null;
|
||||||
const node = dialogRef.current;
|
const node = dialogRef.current;
|
||||||
@ -96,7 +105,7 @@ function FloatingWindowBody({
|
|||||||
if (!isTopmost()) return;
|
if (!isTopmost()) return;
|
||||||
if (e.key === "Escape") {
|
if (e.key === "Escape") {
|
||||||
e.preventDefault();
|
e.preventDefault();
|
||||||
onClose();
|
onCloseRef.current();
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
if (e.key !== "Tab" || !node) return;
|
if (e.key !== "Tab" || !node) return;
|
||||||
@ -128,7 +137,10 @@ function FloatingWindowBody({
|
|||||||
if (i >= 0) windowStack.splice(i, 1);
|
if (i >= 0) windowStack.splice(i, 1);
|
||||||
previouslyFocused?.focus?.();
|
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 (
|
return (
|
||||||
<div
|
<div
|
||||||
|
|||||||
Reference in New Issue
Block a user