From 925ff4de2fe953a24d7bd7588df9b21cd4e00d47 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Mon, 24 Aug 2026 09:25:50 -0400 Subject: [PATCH] fix(web): hand focus to the enlarged text area, and name each button per field Copilot review round 1 on #2109. - Activating Enlarge from the keyboard unmounted the focused button and left focus on the document body, so the next Tab restarted at the top of the page. The text area now takes focus on mount, caret after the text already typed. - Every field's enlarge button carried the same "Enlarge" name, so a form with several string arguments exposed indistinguishable controls. Each now reads "Enlarge ". Queries that matched a field by its label alone are scoped to the textbox role, since the button's name now contains it too. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01JTHVxSu8AUgHRLvo1ntZ8H Signed-off-by: cliffhall --- .../groups/SchemaForm/SchemaForm.test.tsx | 114 ++++++++++++++---- .../groups/SchemaForm/SchemaForm.tsx | 31 ++++- .../screens/ToolsScreen/ToolsScreen.test.tsx | 4 +- 3 files changed, 120 insertions(+), 29 deletions(-) diff --git a/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx b/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx index 722ebf5b..da10bfc6 100644 --- a/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx +++ b/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx @@ -24,7 +24,7 @@ describe("SchemaForm", () => { renderWithMantine( , ); - const input = screen.getByLabelText(/Name/); + const input = screen.getByRole("textbox", { name: /Name/ }); await user.type(input, "a"); expect(onChange).toHaveBeenCalledWith({ name: "a" }); }); @@ -669,7 +669,7 @@ describe("SchemaForm", () => { ); expect(screen.getByText("Address")).toBeInTheDocument(); expect(screen.getByText("Street and city")).toBeInTheDocument(); - expect(screen.getByLabelText(/Street/)).toBeInTheDocument(); + expect(screen.getByRole("textbox", { name: /Street/ })).toBeInTheDocument(); }); it("propagates nested object changes back to top-level onChange", async () => { @@ -690,7 +690,7 @@ describe("SchemaForm", () => { renderWithMantine( , ); - await user.type(screen.getByLabelText(/Street/), "1"); + await user.type(screen.getByRole("textbox", { name: /Street/ }), "1"); expect(onChange).toHaveBeenCalled(); const lastCall = onChange.mock.calls[onChange.mock.calls.length - 1][0]; expect(lastCall.address).toEqual({ street: "1" }); @@ -881,7 +881,7 @@ describe("SchemaForm", () => { disabled />, ); - expect(screen.getByLabelText(/Name/)).toBeDisabled(); + expect(screen.getByRole("textbox", { name: /Name/ })).toBeDisabled(); expect(screen.getByLabelText("Active")).toBeDisabled(); }); @@ -895,7 +895,9 @@ describe("SchemaForm", () => { renderWithMantine( , ); - expect(screen.getByLabelText(/rawField/)).toBeInTheDocument(); + expect( + screen.getByRole("textbox", { name: /rawField/ }), + ).toBeInTheDocument(); }); it("renders nothing inside the form when properties are missing", () => { @@ -1027,7 +1029,7 @@ describe("SchemaForm nullable unions", () => { renderWithMantine( , ); - await user.type(screen.getByLabelText(/Note/), "a"); + await user.type(screen.getByRole("textbox", { name: /Note/ }), "a"); expect(onChange).toHaveBeenCalledWith({ note: "a" }); }); @@ -1096,7 +1098,7 @@ describe("SchemaForm nullable unions", () => { renderWithMantine( , ); - await user.type(screen.getByLabelText(/Nick/), "z"); + await user.type(screen.getByRole("textbox", { name: /Nick/ }), "z"); expect(onChange).toHaveBeenCalledWith({ profile: { nick: "z" } }); }); @@ -1202,7 +1204,7 @@ describe("SchemaForm nullable unions", () => { renderWithMantine( , ); - expect(screen.getByLabelText(/Mode/).tagName).toBe("INPUT"); + expect(screen.getByRole("textbox", { name: /Mode/ }).tagName).toBe("INPUT"); }); it("still renders a MultiSelect when every anyOf branch has a distinct const", () => { @@ -1541,7 +1543,9 @@ describe("JSON editor escaping (#1853, #1856, #1885)", () => { , ); - const jsonInput = screen.getByLabelText(/Note/) as HTMLTextAreaElement; + const jsonInput = screen.getByRole("textbox", { + name: /Note/, + }) as HTMLTextAreaElement; await user.click(jsonInput); await user.keyboard("{End}a"); @@ -1816,17 +1820,21 @@ describe("SchemaForm multiline strings (#2042)", () => { it("renders a single-line input with an enlarge button by default", () => { renderWithMantine(); - expect(screen.getByLabelText(/Note/).tagName).toBe("INPUT"); - expect(screen.getByRole("button", { name: "Enlarge" })).toBeInTheDocument(); + expect(screen.getByRole("textbox", { name: /Note/ }).tagName).toBe("INPUT"); + expect( + screen.getByRole("button", { name: "Enlarge Note" }), + ).toBeInTheDocument(); }); it("swaps the input for a text area that accepts newlines", async () => { const user = userEvent.setup(); renderWithMantine(); - await user.click(screen.getByRole("button", { name: "Enlarge" })); + await user.click(screen.getByRole("button", { name: "Enlarge Note" })); - const textarea = screen.getByLabelText(/Note/) as HTMLTextAreaElement; + const textarea = screen.getByRole("textbox", { + name: /Note/, + }) as HTMLTextAreaElement; expect(textarea.tagName).toBe("TEXTAREA"); await user.type(textarea, "one{Enter}two"); expect(textarea.value).toBe("one\ntwo"); @@ -1835,16 +1843,19 @@ describe("SchemaForm multiline strings (#2042)", () => { it("carries the constraints of the field it replaced", async () => { const user = userEvent.setup(); renderWithMantine(); - await user.click(screen.getByRole("button", { name: "Enlarge" })); - expect(screen.getByLabelText(/Note/)).toHaveAttribute("maxlength", "40"); + await user.click(screen.getByRole("button", { name: "Enlarge Note" })); + expect(screen.getByRole("textbox", { name: /Note/ })).toHaveAttribute( + "maxlength", + "40", + ); }); it("is one-way — the enlarge button is gone once used", async () => { const user = userEvent.setup(); renderWithMantine(); - await user.click(screen.getByRole("button", { name: "Enlarge" })); + await user.click(screen.getByRole("button", { name: "Enlarge Note" })); expect( - screen.queryByRole("button", { name: "Enlarge" }), + screen.queryByRole("button", { name: "Enlarge Note" }), ).not.toBeInTheDocument(); }); @@ -1852,9 +1863,11 @@ describe("SchemaForm multiline strings (#2042)", () => { const user = userEvent.setup(); renderWithMantine(); - await user.type(screen.getByLabelText(/Note/), "typed"); - await user.click(screen.getByRole("button", { name: "Enlarge" })); - const textarea = screen.getByLabelText(/Note/) as HTMLTextAreaElement; + await user.type(screen.getByRole("textbox", { name: /Note/ }), "typed"); + await user.click(screen.getByRole("button", { name: "Enlarge Note" })); + const textarea = screen.getByRole("textbox", { + name: /Note/, + }) as HTMLTextAreaElement; expect(textarea.value).toBe("typed"); await user.click(screen.getByRole("button", { name: "Clear" })); @@ -1867,7 +1880,7 @@ describe("SchemaForm multiline strings (#2042)", () => { expect( screen.queryByRole("button", { name: "Clear" }), ).not.toBeInTheDocument(); - await user.type(screen.getByLabelText(/Note/), "x"); + await user.type(screen.getByRole("textbox", { name: /Note/ }), "x"); expect(screen.getByRole("button", { name: "Clear" })).toBeInTheDocument(); }); @@ -1877,19 +1890,68 @@ describe("SchemaForm multiline strings (#2042)", () => { const user = userEvent.setup(); const { rerender } = renderWithMantine(); - await user.click(screen.getByRole("button", { name: "Enlarge" })); - expect(screen.getByLabelText(/Note/).tagName).toBe("TEXTAREA"); + await user.click(screen.getByRole("button", { name: "Enlarge Note" })); + expect(screen.getByRole("textbox", { name: /Note/ }).tagName).toBe( + "TEXTAREA", + ); rerender(); - expect(screen.getByLabelText(/Note/).tagName).toBe("INPUT"); + expect(screen.getByRole("textbox", { name: /Note/ }).tagName).toBe("INPUT"); }); it("stays enlarged while resetKey is unchanged", async () => { const user = userEvent.setup(); const { rerender } = renderWithMantine(); - await user.click(screen.getByRole("button", { name: "Enlarge" })); + await user.click(screen.getByRole("button", { name: "Enlarge Note" })); rerender(); - expect(screen.getByLabelText(/Note/).tagName).toBe("TEXTAREA"); + expect(screen.getByRole("textbox", { name: /Note/ }).tagName).toBe( + "TEXTAREA", + ); + }); + + it("gives each field's enlarge button a distinct accessible name", () => { + renderWithMantine( + , + ); + expect( + screen.getByRole("button", { name: "Enlarge Note" }), + ).toBeInTheDocument(); + expect( + screen.getByRole("button", { name: "Enlarge Summary" }), + ).toBeInTheDocument(); + }); + + // The button unmounts in the same commit that mounts the text area, so + // without an explicit hand-off a keyboard user is left focused on nothing. + it("moves focus into the text area, caret last, when activated", async () => { + const user = userEvent.setup(); + renderWithMantine(); + + await user.type(screen.getByRole("textbox", { name: /Note/ }), "typed"); + await user.tab(); + expect(screen.getByRole("button", { name: "Enlarge Note" })).toHaveFocus(); + await user.keyboard("{Enter}"); + + const textarea = screen.getByRole("textbox", { + name: /Note/, + }) as HTMLTextAreaElement; + expect(textarea.tagName).toBe("TEXTAREA"); + expect(textarea).toHaveFocus(); + expect(textarea.selectionStart).toBe("typed".length); + + // And the caret really is at the end: typing appends rather than prepends. + await user.keyboard("{Enter}more"); + expect(textarea.value).toBe("typed\nmore"); }); }); diff --git a/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx b/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx index 6edb9795..2b7e4258 100644 --- a/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx +++ b/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx @@ -8,6 +8,7 @@ import { Stack, Text, Textarea, + type TextareaProps, TextInput, } from "@mantine/core"; import { @@ -58,6 +59,33 @@ const MultilineStringInput = Textarea.withProps({ rightSectionPointerEvents: "auto", }); +/** + * A string field that has just been enlarged (#2042). + * + * Exists only to take focus on mount. This component mounts as a direct + * consequence of the user activating the enlarge button — which unmounts in the + * same commit, taking the focused element with it. Without this, a keyboard user + * who activates it is left with focus on the document body, and the next Tab + * restarts from the top of the page rather than continuing through the form. + * + * The caret is placed at the end of whatever was already typed, since focusing a + * pre-filled text control does not agree across browsers on where it lands, and + * the one answer that is never right is "before the text the user just wrote". + */ +function EnlargedStringField(props: TextareaProps) { + const inputRef = useRef(null); + + useEffect(() => { + const node = inputRef.current; + /* v8 ignore next -- an effect runs after mount, so the ref is always set */ + if (!node) return; + node.focus(); + node.setSelectionRange(node.value.length, node.value.length); + }, []); + + return ; +} + // Holds the buttons a string field stacks in its `rightSection`. `nowrap` keeps // them on one line inside a section only wide enough for the two. const FieldActions = Group.withProps({ gap: 2, wrap: "nowrap" }); @@ -666,7 +694,7 @@ export function SchemaForm({ // gone rather than disabled — there is nothing left for it to do. if (enlargedFields.has(fieldName)) { return ( - setEnlargedFields( (previous) => new Set([...previous, fieldName]), diff --git a/clients/web/src/components/screens/ToolsScreen/ToolsScreen.test.tsx b/clients/web/src/components/screens/ToolsScreen/ToolsScreen.test.tsx index da3ed278..bdf9f978 100644 --- a/clients/web/src/components/screens/ToolsScreen/ToolsScreen.test.tsx +++ b/clients/web/src/components/screens/ToolsScreen/ToolsScreen.test.tsx @@ -111,7 +111,7 @@ describe("ToolsScreen", () => { const rows = screen.getAllByText("get_weather"); await user.click(rows[1] as HTMLElement); // The second copy's own field renders — the first copy's does not. - expect(screen.getByLabelText(/zip/i)).toBeInTheDocument(); + expect(screen.getByRole("textbox", { name: /zip/i })).toBeInTheDocument(); expect(screen.queryByLabelText(/city/i)).toBeNull(); await user.click(screen.getByRole("button", { name: /Execute/ })); @@ -137,7 +137,7 @@ describe("ToolsScreen", () => { renderWithMantine(); await user.click(screen.getByText("gamma")); // gamma seeds { mode: "fast" }; editing the field flows through onUiChange. - const field = screen.getByLabelText(/mode/i); + const field = screen.getByRole("textbox", { name: /mode/i }); await user.clear(field); await user.type(field, "slow"); await user.click(screen.getByRole("button", { name: /Execute/ }));