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.
This commit is contained in:
@@ -116,6 +116,28 @@ describe("Modal initial focus by purpose (C4)", () => {
|
|||||||
expect(document.activeElement).toBe(cancel);
|
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) => (
|
||||||
|
<Modal initialFocus="field" title="t" onClose={() => {}}>
|
||||||
|
<Late ready={ready} />
|
||||||
|
{hint && <p>hint</p>}
|
||||||
|
<ModalFooter>
|
||||||
|
<button>cancel</button>
|
||||||
|
</ModalFooter>
|
||||||
|
</Modal>
|
||||||
|
);
|
||||||
|
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", () => {
|
it("close: the ✕, even with a field in the body", () => {
|
||||||
render(
|
render(
|
||||||
<Modal initialFocus="close" title="t" onClose={() => {}}>
|
<Modal initialFocus="close" title="t" onClose={() => {}}>
|
||||||
|
|||||||
@@ -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
|
// 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
|
// 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.
|
// 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 form takes it ONCE: arrived while the focus was elsewhere, it disarms all the same, or a later
|
||||||
// the spinner again): the focus fell to <body>, so it waits on the ✕ for the form (E32 review).
|
// mutation would pull the focus off a ✕ the user went back to (E32 review round 2). The watch
|
||||||
// Only <body>: focus in a dialog stacked on top is not this one's to take.
|
// 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 <body> — it then waits on the ✕ for the form (E32 review). Only <body>: focus
|
||||||
|
// in a dialog stacked on top is not this one's to take.
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
const body = bodyRef.current;
|
const body = bodyRef.current;
|
||||||
if (initialFocus !== "field" || !body || focusablesIn(body).length > 0) return;
|
if (initialFocus !== "field" || !body || focusablesIn(body).length > 0) return;
|
||||||
@@ -119,9 +122,13 @@ export default function Modal({
|
|||||||
armed = true;
|
armed = true;
|
||||||
}
|
}
|
||||||
const first = focusablesIn(body)[0];
|
const first = focusablesIn(body)[0];
|
||||||
if (!armed || !first || document.activeElement !== closeRef.current) return;
|
if (!first) {
|
||||||
|
armed = true;
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
if (!armed) return;
|
||||||
armed = false;
|
armed = false;
|
||||||
first.focus();
|
if (document.activeElement === closeRef.current) first.focus();
|
||||||
});
|
});
|
||||||
watch.observe(body, { childList: true, subtree: true });
|
watch.observe(body, { childList: true, subtree: true });
|
||||||
return () => watch.disconnect();
|
return () => watch.disconnect();
|
||||||
|
|||||||
Reference in New Issue
Block a user