diff --git a/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx b/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx index a381add1..32263e9a 100644 --- a/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx +++ b/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx @@ -2,7 +2,11 @@ import { useState } from "react"; import { describe, it, expect, vi } from "vitest"; import userEvent from "@testing-library/user-event"; import type { InspectorFormSchema } from "../../../utils/jsonUtils"; -import { renderWithMantine, screen } from "../../../test/renderWithMantine"; +import { + fireEvent, + renderWithMantine, + screen, +} from "../../../test/renderWithMantine"; import { SchemaForm } from "./SchemaForm"; describe("SchemaForm", () => { @@ -332,6 +336,76 @@ describe("SchemaForm", () => { expect(onChange).toHaveBeenLastCalledWith({ count: 15 }); }); + it("drops in-progress text when resetKey moves the form to another entity", async () => { + const user = userEvent.setup(); + // Both "tools" expose the same-named number field with no default, so the + // value is `undefined` before and after the switch. The value comparison + // sees no divergence, and only resetKey can tell the field to start over. + const schema: InspectorFormSchema = { + type: "object", + properties: { divisor: { type: "number", title: "Divisor" } }, + }; + function Harness() { + const [tool, setTool] = useState("tool-a"); + const [values, setValues] = useState>({}); + return ( + <> + + + + ); + } + renderWithMantine(); + const input = () => screen.getByLabelText(/Divisor/) as HTMLInputElement; + await user.type(input(), "-"); + expect(input().value).toBe("-"); + // fireEvent, not user.click: a real click also blurs the input, and + // Mantine sanitizes an incomplete value on blur — which would mask + // whether the switch itself cleared the draft. This drives the state + // change without the blur, isolating the reset to resetKey. + fireEvent.click(screen.getByRole("button", { name: "Switch tool" })); + expect(input().value).toBe(""); + }); + + it("keeps in-progress text across re-renders of the same entity", async () => { + const user = userEvent.setup(); + // The counterpart to the test above: a stable resetKey must NOT remount + // the field, or every keystroke would wipe the draft and reinstate #1888. + const schema: InspectorFormSchema = { + type: "object", + properties: { divisor: { type: "number", title: "Divisor" } }, + }; + function Harness() { + const [values, setValues] = useState>({}); + return ( + + ); + } + renderWithMantine(); + const input = screen.getByLabelText(/Divisor/) as HTMLInputElement; + await user.type(input, "1.5"); + expect(input.value).toBe("1.5"); + }); + it("re-syncs the displayed text when the value changes externally", async () => { const user = userEvent.setup(); const onChange = vi.fn(); diff --git a/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx b/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx index 0e67d854..efcb32e3 100644 --- a/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx +++ b/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx @@ -98,6 +98,11 @@ interface SchemaNumberInputProps { * parses to, which leaves an external reset (a cleared form, a loaded example) * working while an in-progress `"1."` — whose parse is `1`, matching the value we * just emitted — is left alone. + * + * That value comparison cannot see a reset to an *equal* value, so the caller is + * additionally expected to vary this component's React key via `SchemaForm`'s + * `resetKey` when it switches which entity the form edits. See the note on that + * prop for the case it covers. */ function SchemaNumberInput({ value, @@ -129,6 +134,25 @@ export interface SchemaFormProps { values: Record; onChange: (values: Record) => void; disabled?: boolean; + /** + * Stable identity of whatever this form is editing — a tool name, a request + * id. Pass it whenever the same mounted form is reused for a *different* + * entity, which is the case for the Tools tab: `ToolDetailPanel` is not keyed + * by tool, so selecting another tool re-renders the same field components. + * + * It exists because the number field's draft/value re-sync compares parsed + * numbers, and so cannot detect a reset to an equal value. Type `-` (draft + * `"-"`, value `undefined`), then switch to a tool with a same-named number + * field and no default: the value is `undefined` on both sides, no divergence + * is seen, and the stale `-` would otherwise be left in the box for the new + * tool to continue from. Varying `resetKey` remounts the field instead, so no + * in-progress text can outlive the entity it was typed into. + * + * Omit it when the form is mounted fresh per entity (the elicitation panels), + * where unmounting already discards the draft. The schema object itself is no + * substitute — callers rebuild it every render, so its identity is unstable. + */ + resetKey?: string; } function getDefaultValue(fieldSchema: InspectorFormSchema): unknown { @@ -153,6 +177,7 @@ export function SchemaForm({ values, onChange, disabled = false, + resetKey, }: SchemaFormProps) { const properties = schema.properties ?? {}; const requiredFields = schema.required ?? []; @@ -232,7 +257,9 @@ export function SchemaForm({ if (fieldSchema.type === "number" || fieldSchema.type === "integer") { return ( diff --git a/clients/web/src/components/groups/ToolDetailPanel/ToolDetailPanel.tsx b/clients/web/src/components/groups/ToolDetailPanel/ToolDetailPanel.tsx index a74c63a9..fbb6d26d 100644 --- a/clients/web/src/components/groups/ToolDetailPanel/ToolDetailPanel.tsx +++ b/clients/web/src/components/groups/ToolDetailPanel/ToolDetailPanel.tsx @@ -317,6 +317,10 @@ export function ToolDetailPanel({ values={formValues} onChange={onFormChange} disabled={isExecuting} + // This panel is reused across tool selections rather than remounted, + // so the form needs the tool name to drop another tool's + // in-progress field text. See SchemaFormProps.resetKey. + resetKey={name} /> {progress && }