Merge pull request #2109 from modelcontextprotocol/v2/fix/2042-multiline-string-input
fix(web): let tool string arguments be typed as multiline
This commit is contained in:
commit
9d0448b644
8 changed files with 510 additions and 29 deletions
|
|
@ -0,0 +1,33 @@
|
||||||
|
import type { Meta, StoryObj } from "@storybook/react-vite";
|
||||||
|
import { expect, userEvent, within } from "storybook/test";
|
||||||
|
import { EnlargeButton } from "./EnlargeButton";
|
||||||
|
|
||||||
|
const meta: Meta<typeof EnlargeButton> = {
|
||||||
|
title: "Elements/EnlargeButton",
|
||||||
|
component: EnlargeButton,
|
||||||
|
args: { onClick: () => {} },
|
||||||
|
};
|
||||||
|
|
||||||
|
export default meta;
|
||||||
|
type Story = StoryObj<typeof EnlargeButton>;
|
||||||
|
|
||||||
|
// Default: labelled "Enlarge", reachable by keyboard, and clickable.
|
||||||
|
export const Default: Story = {
|
||||||
|
play: async ({ canvasElement }) => {
|
||||||
|
const canvas = within(canvasElement);
|
||||||
|
const button = await canvas.findByRole("button", { name: "Enlarge" });
|
||||||
|
await expect(button).not.toHaveAttribute("tabindex", "-1");
|
||||||
|
await userEvent.click(button);
|
||||||
|
},
|
||||||
|
};
|
||||||
|
|
||||||
|
// A per-field accessible name, for a form where several fields each carry one.
|
||||||
|
export const CustomLabel: Story = {
|
||||||
|
args: { ariaLabel: "Enlarge Query" },
|
||||||
|
play: async ({ canvasElement }) => {
|
||||||
|
const canvas = within(canvasElement);
|
||||||
|
await expect(
|
||||||
|
await canvas.findByRole("button", { name: "Enlarge Query" }),
|
||||||
|
).toBeVisible();
|
||||||
|
},
|
||||||
|
};
|
||||||
|
|
@ -0,0 +1,46 @@
|
||||||
|
import { describe, it, expect, vi } from "vitest";
|
||||||
|
import userEvent from "@testing-library/user-event";
|
||||||
|
import { renderWithMantine, screen } from "../../../test/renderWithMantine";
|
||||||
|
import { EnlargeButton } from "./EnlargeButton";
|
||||||
|
|
||||||
|
describe("EnlargeButton", () => {
|
||||||
|
it('renders with the "Enlarge" accessible name', () => {
|
||||||
|
renderWithMantine(<EnlargeButton onClick={vi.fn()} />);
|
||||||
|
expect(screen.getByRole("button", { name: "Enlarge" })).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("stays in the keyboard tab order", () => {
|
||||||
|
renderWithMantine(<EnlargeButton onClick={vi.fn()} />);
|
||||||
|
expect(screen.getByRole("button", { name: "Enlarge" })).not.toHaveAttribute(
|
||||||
|
"tabindex",
|
||||||
|
"-1",
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("invokes onClick when clicked", async () => {
|
||||||
|
const user = userEvent.setup();
|
||||||
|
const onClick = vi.fn();
|
||||||
|
renderWithMantine(<EnlargeButton onClick={onClick} />);
|
||||||
|
await user.click(screen.getByRole("button", { name: "Enlarge" }));
|
||||||
|
expect(onClick).toHaveBeenCalledTimes(1);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("allows the accessible name to be overridden", () => {
|
||||||
|
renderWithMantine(
|
||||||
|
<EnlargeButton onClick={vi.fn()} ariaLabel="Enlarge Query" />,
|
||||||
|
);
|
||||||
|
expect(
|
||||||
|
screen.getByRole("button", { name: "Enlarge Query" }),
|
||||||
|
).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("is inert when the field it belongs to is disabled", async () => {
|
||||||
|
const user = userEvent.setup();
|
||||||
|
const onClick = vi.fn();
|
||||||
|
renderWithMantine(<EnlargeButton onClick={onClick} disabled />);
|
||||||
|
const button = screen.getByRole("button", { name: "Enlarge" });
|
||||||
|
expect(button).toBeDisabled();
|
||||||
|
await user.click(button);
|
||||||
|
expect(onClick).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
@ -0,0 +1,59 @@
|
||||||
|
import { ActionIcon, Tooltip } from "@mantine/core";
|
||||||
|
import { ImEnlarge2 } from "react-icons/im";
|
||||||
|
|
||||||
|
export interface EnlargeButtonProps {
|
||||||
|
/** Swap the single-line input this sits in for a multiline text area. */
|
||||||
|
onClick: () => void;
|
||||||
|
/**
|
||||||
|
* Overrides the accessible name (aria-label), which defaults to the tooltip
|
||||||
|
* text ("Enlarge"). Pass a per-field name (e.g. including the field's label)
|
||||||
|
* when several inputs in one form each carry a button, so assistive tech can
|
||||||
|
* tell them apart; the visible tooltip stays the plain verb.
|
||||||
|
*/
|
||||||
|
ariaLabel?: string;
|
||||||
|
/**
|
||||||
|
* Mirrors the disabled state of the input this sits in. A live button on a
|
||||||
|
* disabled field would still swap in the text area — which mounts disabled and
|
||||||
|
* cannot take focus, dropping keyboard focus to the document.
|
||||||
|
*/
|
||||||
|
disabled?: boolean;
|
||||||
|
}
|
||||||
|
|
||||||
|
const EnlargeActionIcon = ActionIcon.withProps({
|
||||||
|
variant: "subtle",
|
||||||
|
color: "gray",
|
||||||
|
size: "sm",
|
||||||
|
});
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The enlarge affordance shown to the left of a text input's ClearButton
|
||||||
|
* (#2042). Clicking it replaces the single-line input with a multiline text
|
||||||
|
* area, so a value containing newlines can be typed — a plain `<input>` swallows
|
||||||
|
* Enter, which left string arguments effectively single-line.
|
||||||
|
*
|
||||||
|
* Deliberately not a toggle: shrinking back would have to decide what to do with
|
||||||
|
* the newlines already typed, and either answer (silently discard them, or keep
|
||||||
|
* a value the field can no longer display) is worse than staying enlarged.
|
||||||
|
*
|
||||||
|
* Unlike ClearButton it stays in the keyboard tab order. Clearing a field has a
|
||||||
|
* keyboard equivalent (select-all, delete), so #1487 could take that button out
|
||||||
|
* of the tab order without cost; entering multiline mode has none, so removing
|
||||||
|
* this one would put the feature out of reach of keyboard users entirely.
|
||||||
|
*/
|
||||||
|
export function EnlargeButton({
|
||||||
|
onClick,
|
||||||
|
ariaLabel,
|
||||||
|
disabled,
|
||||||
|
}: EnlargeButtonProps) {
|
||||||
|
return (
|
||||||
|
<Tooltip label="Enlarge">
|
||||||
|
<EnlargeActionIcon
|
||||||
|
aria-label={ariaLabel ?? "Enlarge"}
|
||||||
|
disabled={disabled}
|
||||||
|
onClick={onClick}
|
||||||
|
>
|
||||||
|
<ImEnlarge2 size={12} />
|
||||||
|
</EnlargeActionIcon>
|
||||||
|
</Tooltip>
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
@ -24,7 +24,7 @@ describe("SchemaForm", () => {
|
||||||
renderWithMantine(
|
renderWithMantine(
|
||||||
<SchemaForm schema={schema} values={{}} onChange={onChange} />,
|
<SchemaForm schema={schema} values={{}} onChange={onChange} />,
|
||||||
);
|
);
|
||||||
const input = screen.getByLabelText(/Name/);
|
const input = screen.getByRole("textbox", { name: /Name/ });
|
||||||
await user.type(input, "a");
|
await user.type(input, "a");
|
||||||
expect(onChange).toHaveBeenCalledWith({ name: "a" });
|
expect(onChange).toHaveBeenCalledWith({ name: "a" });
|
||||||
});
|
});
|
||||||
|
|
@ -669,7 +669,7 @@ describe("SchemaForm", () => {
|
||||||
);
|
);
|
||||||
expect(screen.getByText("Address")).toBeInTheDocument();
|
expect(screen.getByText("Address")).toBeInTheDocument();
|
||||||
expect(screen.getByText("Street and city")).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 () => {
|
it("propagates nested object changes back to top-level onChange", async () => {
|
||||||
|
|
@ -690,7 +690,7 @@ describe("SchemaForm", () => {
|
||||||
renderWithMantine(
|
renderWithMantine(
|
||||||
<SchemaForm schema={schema} values={{}} onChange={onChange} />,
|
<SchemaForm schema={schema} values={{}} onChange={onChange} />,
|
||||||
);
|
);
|
||||||
await user.type(screen.getByLabelText(/Street/), "1");
|
await user.type(screen.getByRole("textbox", { name: /Street/ }), "1");
|
||||||
expect(onChange).toHaveBeenCalled();
|
expect(onChange).toHaveBeenCalled();
|
||||||
const lastCall = onChange.mock.calls[onChange.mock.calls.length - 1][0];
|
const lastCall = onChange.mock.calls[onChange.mock.calls.length - 1][0];
|
||||||
expect(lastCall.address).toEqual({ street: "1" });
|
expect(lastCall.address).toEqual({ street: "1" });
|
||||||
|
|
@ -881,7 +881,7 @@ describe("SchemaForm", () => {
|
||||||
disabled
|
disabled
|
||||||
/>,
|
/>,
|
||||||
);
|
);
|
||||||
expect(screen.getByLabelText(/Name/)).toBeDisabled();
|
expect(screen.getByRole("textbox", { name: /Name/ })).toBeDisabled();
|
||||||
expect(screen.getByLabelText("Active")).toBeDisabled();
|
expect(screen.getByLabelText("Active")).toBeDisabled();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|
@ -895,7 +895,9 @@ describe("SchemaForm", () => {
|
||||||
renderWithMantine(
|
renderWithMantine(
|
||||||
<SchemaForm schema={schema} values={{}} onChange={vi.fn()} />,
|
<SchemaForm schema={schema} values={{}} onChange={vi.fn()} />,
|
||||||
);
|
);
|
||||||
expect(screen.getByLabelText(/rawField/)).toBeInTheDocument();
|
expect(
|
||||||
|
screen.getByRole("textbox", { name: /rawField/ }),
|
||||||
|
).toBeInTheDocument();
|
||||||
});
|
});
|
||||||
|
|
||||||
it("renders nothing inside the form when properties are missing", () => {
|
it("renders nothing inside the form when properties are missing", () => {
|
||||||
|
|
@ -1027,7 +1029,7 @@ describe("SchemaForm nullable unions", () => {
|
||||||
renderWithMantine(
|
renderWithMantine(
|
||||||
<SchemaForm schema={schema} values={{}} onChange={onChange} />,
|
<SchemaForm schema={schema} values={{}} onChange={onChange} />,
|
||||||
);
|
);
|
||||||
await user.type(screen.getByLabelText(/Note/), "a");
|
await user.type(screen.getByRole("textbox", { name: /Note/ }), "a");
|
||||||
expect(onChange).toHaveBeenCalledWith({ note: "a" });
|
expect(onChange).toHaveBeenCalledWith({ note: "a" });
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|
@ -1096,7 +1098,7 @@ describe("SchemaForm nullable unions", () => {
|
||||||
renderWithMantine(
|
renderWithMantine(
|
||||||
<SchemaForm schema={schema} values={{}} onChange={onChange} />,
|
<SchemaForm schema={schema} values={{}} onChange={onChange} />,
|
||||||
);
|
);
|
||||||
await user.type(screen.getByLabelText(/Nick/), "z");
|
await user.type(screen.getByRole("textbox", { name: /Nick/ }), "z");
|
||||||
expect(onChange).toHaveBeenCalledWith({ profile: { nick: "z" } });
|
expect(onChange).toHaveBeenCalledWith({ profile: { nick: "z" } });
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|
@ -1202,7 +1204,7 @@ describe("SchemaForm nullable unions", () => {
|
||||||
renderWithMantine(
|
renderWithMantine(
|
||||||
<SchemaForm schema={schema} values={{}} onChange={vi.fn()} />,
|
<SchemaForm schema={schema} values={{}} onChange={vi.fn()} />,
|
||||||
);
|
);
|
||||||
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", () => {
|
it("still renders a MultiSelect when every anyOf branch has a distinct const", () => {
|
||||||
|
|
@ -1541,7 +1543,9 @@ describe("JSON editor escaping (#1853, #1856, #1885)", () => {
|
||||||
<EscapingHarness schema={schema} initial={{ note: "" }} />,
|
<EscapingHarness schema={schema} initial={{ note: "" }} />,
|
||||||
);
|
);
|
||||||
|
|
||||||
const jsonInput = screen.getByLabelText(/Note/) as HTMLTextAreaElement;
|
const jsonInput = screen.getByRole("textbox", {
|
||||||
|
name: /Note/,
|
||||||
|
}) as HTMLTextAreaElement;
|
||||||
await user.click(jsonInput);
|
await user.click(jsonInput);
|
||||||
await user.keyboard("{End}a");
|
await user.keyboard("{End}a");
|
||||||
|
|
||||||
|
|
@ -1790,3 +1794,179 @@ describe("SchemaForm defaulted fields (#2026)", () => {
|
||||||
expect(jsonInput.value).toBe('[\n "z"\n]');
|
expect(jsonInput.value).toBe('[\n "z"\n]');
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// A plain `<input>` swallows Enter, so a string argument could not be given a
|
||||||
|
// value containing newlines (#2042). Each string field carries an enlarge button
|
||||||
|
// that swaps it for a text area, one-way.
|
||||||
|
describe("SchemaForm multiline strings (#2042)", () => {
|
||||||
|
const stringSchema: InspectorFormSchema = {
|
||||||
|
type: "object",
|
||||||
|
properties: {
|
||||||
|
note: { type: "string", title: "Note", maxLength: 40 },
|
||||||
|
},
|
||||||
|
};
|
||||||
|
|
||||||
|
function StringHarness({ resetKey }: { resetKey?: string }) {
|
||||||
|
const [values, setValues] = useState<Record<string, unknown>>({});
|
||||||
|
return (
|
||||||
|
<SchemaForm
|
||||||
|
schema={stringSchema}
|
||||||
|
values={values}
|
||||||
|
onChange={setValues}
|
||||||
|
resetKey={resetKey}
|
||||||
|
/>
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
it("renders a single-line input with an enlarge button by default", () => {
|
||||||
|
renderWithMantine(<StringHarness />);
|
||||||
|
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(<StringHarness />);
|
||||||
|
|
||||||
|
await user.click(screen.getByRole("button", { name: "Enlarge Note" }));
|
||||||
|
|
||||||
|
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");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("carries the constraints of the field it replaced", async () => {
|
||||||
|
const user = userEvent.setup();
|
||||||
|
renderWithMantine(<StringHarness />);
|
||||||
|
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(<StringHarness />);
|
||||||
|
await user.click(screen.getByRole("button", { name: "Enlarge Note" }));
|
||||||
|
expect(
|
||||||
|
screen.queryByRole("button", { name: "Enlarge Note" }),
|
||||||
|
).not.toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("keeps the clear button working after enlarging", async () => {
|
||||||
|
const user = userEvent.setup();
|
||||||
|
renderWithMantine(<StringHarness />);
|
||||||
|
|
||||||
|
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" }));
|
||||||
|
expect(textarea.value).toBe("");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("shows no clear button until the field holds a value", async () => {
|
||||||
|
const user = userEvent.setup();
|
||||||
|
renderWithMantine(<StringHarness />);
|
||||||
|
expect(
|
||||||
|
screen.queryByRole("button", { name: "Clear" }),
|
||||||
|
).not.toBeInTheDocument();
|
||||||
|
await user.type(screen.getByRole("textbox", { name: /Note/ }), "x");
|
||||||
|
expect(screen.getByRole("button", { name: "Clear" })).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
|
// Enlarging belongs to the field the user enlarged, not to its name — the
|
||||||
|
// same reasoning resetKey documents for the number field's draft.
|
||||||
|
it("goes back to a single-line input when resetKey moves to another entity", async () => {
|
||||||
|
const user = userEvent.setup();
|
||||||
|
const { rerender } = renderWithMantine(<StringHarness resetKey="tool-a" />);
|
||||||
|
|
||||||
|
await user.click(screen.getByRole("button", { name: "Enlarge Note" }));
|
||||||
|
expect(screen.getByRole("textbox", { name: /Note/ }).tagName).toBe(
|
||||||
|
"TEXTAREA",
|
||||||
|
);
|
||||||
|
|
||||||
|
rerender(<StringHarness resetKey="tool-b" />);
|
||||||
|
expect(screen.getByRole("textbox", { name: /Note/ }).tagName).toBe("INPUT");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("stays enlarged while resetKey is unchanged", async () => {
|
||||||
|
const user = userEvent.setup();
|
||||||
|
const { rerender } = renderWithMantine(<StringHarness resetKey="tool-a" />);
|
||||||
|
|
||||||
|
await user.click(screen.getByRole("button", { name: "Enlarge Note" }));
|
||||||
|
rerender(<StringHarness resetKey="tool-a" />);
|
||||||
|
expect(screen.getByRole("textbox", { name: /Note/ }).tagName).toBe(
|
||||||
|
"TEXTAREA",
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("gives each field's enlarge button a distinct accessible name", () => {
|
||||||
|
renderWithMantine(
|
||||||
|
<SchemaForm
|
||||||
|
schema={{
|
||||||
|
type: "object",
|
||||||
|
properties: {
|
||||||
|
note: { type: "string", title: "Note" },
|
||||||
|
summary: { type: "string", title: "Summary" },
|
||||||
|
},
|
||||||
|
}}
|
||||||
|
values={{}}
|
||||||
|
onChange={vi.fn()}
|
||||||
|
/>,
|
||||||
|
);
|
||||||
|
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(<StringHarness />);
|
||||||
|
|
||||||
|
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");
|
||||||
|
});
|
||||||
|
|
||||||
|
// A disabled form (a tool call in flight) must be inert as a whole: a live
|
||||||
|
// button there would swap in a text area that mounts disabled, cannot take
|
||||||
|
// focus, and so drops keyboard focus to the document.
|
||||||
|
it("disables the enlarge button along with the field", () => {
|
||||||
|
renderWithMantine(
|
||||||
|
<SchemaForm
|
||||||
|
schema={stringSchema}
|
||||||
|
values={{}}
|
||||||
|
onChange={vi.fn()}
|
||||||
|
disabled
|
||||||
|
/>,
|
||||||
|
);
|
||||||
|
expect(screen.getByRole("button", { name: "Enlarge Note" })).toBeDisabled();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
|
||||||
|
|
@ -1,15 +1,27 @@
|
||||||
import {
|
import {
|
||||||
Checkbox,
|
Checkbox,
|
||||||
|
Group,
|
||||||
JsonInput,
|
JsonInput,
|
||||||
MultiSelect,
|
MultiSelect,
|
||||||
NumberInput,
|
NumberInput,
|
||||||
Select,
|
Select,
|
||||||
Stack,
|
Stack,
|
||||||
Text,
|
Text,
|
||||||
|
rem,
|
||||||
|
Textarea,
|
||||||
|
type TextareaProps,
|
||||||
TextInput,
|
TextInput,
|
||||||
} from "@mantine/core";
|
} from "@mantine/core";
|
||||||
import { useCallback, useEffect, useRef, useState } from "react";
|
import {
|
||||||
|
useCallback,
|
||||||
|
useEffect,
|
||||||
|
useLayoutEffect,
|
||||||
|
useRef,
|
||||||
|
useState,
|
||||||
|
type ChangeEvent,
|
||||||
|
} from "react";
|
||||||
import { ClearButton } from "../../elements/ClearButton/ClearButton";
|
import { ClearButton } from "../../elements/ClearButton/ClearButton";
|
||||||
|
import { EnlargeButton } from "../../elements/EnlargeButton/EnlargeButton";
|
||||||
import { useValueChange } from "../../../hooks/useValueChange";
|
import { useValueChange } from "../../../hooks/useValueChange";
|
||||||
import type {
|
import type {
|
||||||
InspectorFormSchema,
|
InspectorFormSchema,
|
||||||
|
|
@ -38,6 +50,66 @@ const SchemaJsonInput = JsonInput.withProps({
|
||||||
autosize: true,
|
autosize: true,
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// A string field after its enlarge button has been used (#2042). `autosize`
|
||||||
|
// grows it with the text rather than fixing a height the value may not fit, and
|
||||||
|
// `maxRows` caps that growth so a long value scrolls inside the field instead of
|
||||||
|
// pushing the rest of the form off screen.
|
||||||
|
const MultilineStringInput = Textarea.withProps({
|
||||||
|
autosize: true,
|
||||||
|
minRows: 3,
|
||||||
|
maxRows: 12,
|
||||||
|
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<HTMLTextAreaElement>(null);
|
||||||
|
|
||||||
|
// Layout, not passive: a passive effect runs after paint, so the browser would
|
||||||
|
// present at least one frame with focus on the document body — long enough for
|
||||||
|
// a fast Tab to land somewhere else entirely.
|
||||||
|
useLayoutEffect(() => {
|
||||||
|
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 <MultilineStringInput {...props} ref={inputRef} />;
|
||||||
|
}
|
||||||
|
|
||||||
|
// 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" });
|
||||||
|
|
||||||
|
// Width of the string field's right section, which Mantine takes as a number it
|
||||||
|
// cannot derive from its content. Built from the parts rather than written as a
|
||||||
|
// pixel total, so it stays right if a button or the gap changes, and emitted as
|
||||||
|
// `rem` so it tracks a user's root font size rather than pinning to CSS pixels.
|
||||||
|
// The two button widths are what they measure at their current sizes (ActionIcon
|
||||||
|
// `sm` and CloseButton's default); the inset keeps them off the field's edge.
|
||||||
|
const ENLARGE_WIDTH = 22;
|
||||||
|
const CLEAR_WIDTH = 28;
|
||||||
|
const ACTION_GAP = 2;
|
||||||
|
const SECTION_INSET = 6;
|
||||||
|
const ONE_ACTION_WIDTH = rem(CLEAR_WIDTH + SECTION_INSET);
|
||||||
|
const TWO_ACTION_WIDTH = rem(
|
||||||
|
ENLARGE_WIDTH + ACTION_GAP + CLEAR_WIDTH + SECTION_INSET,
|
||||||
|
);
|
||||||
|
|
||||||
function serializeJson(value: unknown): string {
|
function serializeJson(value: unknown): string {
|
||||||
return JSON.stringify(value, null, 2);
|
return JSON.stringify(value, null, 2);
|
||||||
}
|
}
|
||||||
|
|
@ -507,6 +579,19 @@ export function SchemaForm({
|
||||||
() => new Set(),
|
() => new Set(),
|
||||||
);
|
);
|
||||||
|
|
||||||
|
// The names of string fields the user has enlarged into a multiline text area
|
||||||
|
// (#2042). One-way by design — see EnlargeButton — so a name only ever enters
|
||||||
|
// this set.
|
||||||
|
const [enlargedFields, setEnlargedFields] = useState<ReadonlySet<string>>(
|
||||||
|
() => new Set(),
|
||||||
|
);
|
||||||
|
|
||||||
|
// Enlarging is a property of the field the user enlarged, not of the field
|
||||||
|
// *name*, so it must not carry across to another entity's same-named field —
|
||||||
|
// the same reasoning `resetKey` documents for the number field's draft. Reset
|
||||||
|
// during render rather than in an effect so no frame paints the wrong shape.
|
||||||
|
useValueChange(resetKey, () => setEnlargedFields(new Set()));
|
||||||
|
|
||||||
// Stable so a field's reporting effect subscribes once, not per render. The
|
// Stable so a field's reporting effect subscribes once, not per render. The
|
||||||
// updater returns the previous set unchanged when nothing moved, which is
|
// updater returns the previous set unchanged when nothing moved, which is
|
||||||
// what makes a nested form's inline callback safe to call repeatedly.
|
// what makes a nested form's inline callback safe to call repeatedly.
|
||||||
|
|
@ -604,24 +689,54 @@ export function SchemaForm({
|
||||||
|
|
||||||
// plain string
|
// plain string
|
||||||
if (fieldSchema.type === "string") {
|
if (fieldSchema.type === "string") {
|
||||||
|
const clearButton = rawValue ? (
|
||||||
|
<ClearButton onClick={() => handleFieldChange(fieldName, "")} />
|
||||||
|
) : null;
|
||||||
|
const sharedProps = {
|
||||||
|
label,
|
||||||
|
description,
|
||||||
|
withAsterisk: isRequired,
|
||||||
|
disabled,
|
||||||
|
value: (rawValue as string) ?? "",
|
||||||
|
minLength: fieldSchema.minLength,
|
||||||
|
maxLength: fieldSchema.maxLength,
|
||||||
|
onChange: (
|
||||||
|
event: ChangeEvent<HTMLInputElement | HTMLTextAreaElement>,
|
||||||
|
) => handleFieldChange(fieldName, event.currentTarget.value),
|
||||||
|
};
|
||||||
|
|
||||||
|
// Already enlarged: a text area, and no way back. The enlarge button is
|
||||||
|
// gone rather than disabled — there is nothing left for it to do.
|
||||||
|
if (enlargedFields.has(fieldName)) {
|
||||||
|
return (
|
||||||
|
<EnlargedStringField
|
||||||
|
key={fieldName}
|
||||||
|
{...sharedProps}
|
||||||
|
rightSectionWidth={ONE_ACTION_WIDTH}
|
||||||
|
rightSection={clearButton}
|
||||||
|
/>
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<TextInput
|
<TextInput
|
||||||
key={fieldName}
|
key={fieldName}
|
||||||
label={label}
|
{...sharedProps}
|
||||||
description={description}
|
|
||||||
withAsterisk={isRequired}
|
|
||||||
disabled={disabled}
|
|
||||||
value={(rawValue as string) ?? ""}
|
|
||||||
minLength={fieldSchema.minLength}
|
|
||||||
maxLength={fieldSchema.maxLength}
|
|
||||||
onChange={(event) =>
|
|
||||||
handleFieldChange(fieldName, event.currentTarget.value)
|
|
||||||
}
|
|
||||||
rightSectionPointerEvents="auto"
|
rightSectionPointerEvents="auto"
|
||||||
|
rightSectionWidth={clearButton ? TWO_ACTION_WIDTH : ONE_ACTION_WIDTH}
|
||||||
rightSection={
|
rightSection={
|
||||||
rawValue ? (
|
<FieldActions>
|
||||||
<ClearButton onClick={() => handleFieldChange(fieldName, "")} />
|
<EnlargeButton
|
||||||
) : null
|
ariaLabel={`Enlarge ${label}`}
|
||||||
|
disabled={disabled}
|
||||||
|
onClick={() =>
|
||||||
|
setEnlargedFields(
|
||||||
|
(previous) => new Set([...previous, fieldName]),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
/>
|
||||||
|
{clearButton}
|
||||||
|
</FieldActions>
|
||||||
}
|
}
|
||||||
/>
|
/>
|
||||||
);
|
);
|
||||||
|
|
|
||||||
|
|
@ -55,6 +55,15 @@ export interface ToolDetailPanelProps {
|
||||||
/** Receives the effective run-as-task decision for this execution. */
|
/** Receives the effective run-as-task decision for this execution. */
|
||||||
onExecute: (runAsTask: boolean) => void;
|
onExecute: (runAsTask: boolean) => void;
|
||||||
onCancel: () => void;
|
onCancel: () => void;
|
||||||
|
/**
|
||||||
|
* Identity of the selected *row*, which is not the tool name: a server may
|
||||||
|
* repeat a name, and `ToolsScreen` disambiguates those rows with a key of its
|
||||||
|
* own (#1957). Passed straight to `SchemaFormProps.resetKey`, so switching
|
||||||
|
* between two same-named tools drops the first one's in-progress field text
|
||||||
|
* and enlarged fields. Falls back to the name, which is the right answer for
|
||||||
|
* any caller whose list cannot repeat one.
|
||||||
|
*/
|
||||||
|
resetKey?: string;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Outer column: title/annotations pin at top, the Execute footer pins at the
|
// Outer column: title/annotations pin at top, the Execute footer pins at the
|
||||||
|
|
@ -201,6 +210,7 @@ export function ToolDetailPanel({
|
||||||
onFormChange,
|
onFormChange,
|
||||||
onExecute,
|
onExecute,
|
||||||
onCancel,
|
onCancel,
|
||||||
|
resetKey,
|
||||||
}: ToolDetailPanelProps) {
|
}: ToolDetailPanelProps) {
|
||||||
const { name, title, description, icons, annotations, inputSchema } = tool;
|
const { name, title, description, icons, annotations, inputSchema } = tool;
|
||||||
// Narrow the SDK protocol schema to the form renderer's schema type.
|
// Narrow the SDK protocol schema to the form renderer's schema type.
|
||||||
|
|
@ -333,10 +343,10 @@ export function ToolDetailPanel({
|
||||||
values={formValues}
|
values={formValues}
|
||||||
onChange={onFormChange}
|
onChange={onFormChange}
|
||||||
disabled={isExecuting}
|
disabled={isExecuting}
|
||||||
// This panel is reused across tool selections rather than remounted,
|
// This panel is reused across tool selections rather than
|
||||||
// so the form needs the tool name to drop another tool's
|
// remounted, so the form needs a per-selection key to drop another
|
||||||
// in-progress field text. See SchemaFormProps.resetKey.
|
// tool's in-progress field text. See SchemaFormProps.resetKey.
|
||||||
resetKey={name}
|
resetKey={resetKey ?? name}
|
||||||
onValidityChange={setHasInvalidDraft}
|
onValidityChange={setHasInvalidDraft}
|
||||||
/>
|
/>
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -82,6 +82,41 @@ describe("ToolsScreen", () => {
|
||||||
).not.toBeInTheDocument();
|
).not.toBeInTheDocument();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("drops an enlarged field when switching between duplicated tool names", async () => {
|
||||||
|
// The panel is reused across selections, so SchemaForm resets per-field
|
||||||
|
// state on `resetKey`. That key must be the ROW identity: two rows sharing
|
||||||
|
// a name would otherwise look like the same entity, and the first copy's
|
||||||
|
// enlarged field would stay enlarged over the second copy's (#2042/#1957).
|
||||||
|
const user = userEvent.setup();
|
||||||
|
const duplicated: Tool[] = [
|
||||||
|
{
|
||||||
|
name: "get_weather",
|
||||||
|
inputSchema: {
|
||||||
|
type: "object",
|
||||||
|
properties: { city: { type: "string", title: "City" } },
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "get_weather",
|
||||||
|
inputSchema: {
|
||||||
|
type: "object",
|
||||||
|
properties: { city: { type: "string", title: "City" } },
|
||||||
|
},
|
||||||
|
},
|
||||||
|
];
|
||||||
|
renderWithMantine(<ControlledToolsScreen tools={duplicated} />);
|
||||||
|
const rows = screen.getAllByText("get_weather");
|
||||||
|
|
||||||
|
await user.click(rows[0] as HTMLElement);
|
||||||
|
await user.click(screen.getByRole("button", { name: "Enlarge City" }));
|
||||||
|
expect(screen.getByRole("textbox", { name: /City/ }).tagName).toBe(
|
||||||
|
"TEXTAREA",
|
||||||
|
);
|
||||||
|
|
||||||
|
await user.click(rows[1] as HTMLElement);
|
||||||
|
expect(screen.getByRole("textbox", { name: /City/ }).tagName).toBe("INPUT");
|
||||||
|
});
|
||||||
|
|
||||||
it("opens the second copy of a duplicated tool name and calls it by its protocol name", async () => {
|
it("opens the second copy of a duplicated tool name and calls it by its protocol name", async () => {
|
||||||
// A `tools/list` may repeat a name with a different schema. Selection is
|
// A `tools/list` may repeat a name with a different schema. Selection is
|
||||||
// keyed by row identity, so the later copy is reachable and the detail
|
// keyed by row identity, so the later copy is reachable and the detail
|
||||||
|
|
@ -111,7 +146,7 @@ describe("ToolsScreen", () => {
|
||||||
const rows = screen.getAllByText("get_weather");
|
const rows = screen.getAllByText("get_weather");
|
||||||
await user.click(rows[1] as HTMLElement);
|
await user.click(rows[1] as HTMLElement);
|
||||||
// The second copy's own field renders — the first copy's does not.
|
// 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();
|
expect(screen.queryByLabelText(/city/i)).toBeNull();
|
||||||
|
|
||||||
await user.click(screen.getByRole("button", { name: /Execute/ }));
|
await user.click(screen.getByRole("button", { name: /Execute/ }));
|
||||||
|
|
@ -137,7 +172,7 @@ describe("ToolsScreen", () => {
|
||||||
renderWithMantine(<ControlledToolsScreen onCallTool={onCallTool} />);
|
renderWithMantine(<ControlledToolsScreen onCallTool={onCallTool} />);
|
||||||
await user.click(screen.getByText("gamma"));
|
await user.click(screen.getByText("gamma"));
|
||||||
// gamma seeds { mode: "fast" }; editing the field flows through onUiChange.
|
// 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.clear(field);
|
||||||
await user.type(field, "slow");
|
await user.type(field, "slow");
|
||||||
await user.click(screen.getByRole("button", { name: /Execute/ }));
|
await user.click(screen.getByRole("button", { name: /Execute/ }));
|
||||||
|
|
|
||||||
|
|
@ -258,6 +258,9 @@ export function ToolsScreen({
|
||||||
<ContentCard>
|
<ContentCard>
|
||||||
<ToolDetailPanel
|
<ToolDetailPanel
|
||||||
tool={selectedTool}
|
tool={selectedTool}
|
||||||
|
// The row key, not the name — a server may list the same tool
|
||||||
|
// name twice (#1957), and the panel is reused across selections.
|
||||||
|
resetKey={selectedToolKey}
|
||||||
formValues={formValues}
|
formValues={formValues}
|
||||||
isExecuting={isExecuting}
|
isExecuting={isExecuting}
|
||||||
progress={callState?.progress}
|
progress={callState?.progress}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue