From 5340283fb49904a5e0739268ddb97a835f8dbb10 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Mon, 24 Aug 2026 09:05:31 -0400 Subject: [PATCH] fix(web): let tool string arguments be typed as multiline (#2042) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A string argument renders as a single-line ``, which swallows Enter — so a value containing newlines could not be typed at all. Each string field now carries an enlarge button (ImEnlarge2) to the left of its clear button. Clicking it swaps the input for an autosizing text area that keeps the field's value, label, and length constraints. It is deliberately one-way: shrinking back would have to either discard newlines the user typed or keep a value the field can no longer show, and both are worse than staying enlarged. Enlarging is per-field and resets with `resetKey`, so it cannot carry across to another tool's same-named field. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01JTHVxSu8AUgHRLvo1ntZ8H Signed-off-by: cliffhall --- .../EnlargeButton/EnlargeButton.stories.tsx | 33 ++++++ .../EnlargeButton/EnlargeButton.test.tsx | 36 ++++++ .../elements/EnlargeButton/EnlargeButton.tsx | 45 ++++++++ .../groups/SchemaForm/SchemaForm.test.tsx | 103 ++++++++++++++++++ .../groups/SchemaForm/SchemaForm.tsx | 98 ++++++++++++++--- 5 files changed, 301 insertions(+), 14 deletions(-) create mode 100644 clients/web/src/components/elements/EnlargeButton/EnlargeButton.stories.tsx create mode 100644 clients/web/src/components/elements/EnlargeButton/EnlargeButton.test.tsx create mode 100644 clients/web/src/components/elements/EnlargeButton/EnlargeButton.tsx diff --git a/clients/web/src/components/elements/EnlargeButton/EnlargeButton.stories.tsx b/clients/web/src/components/elements/EnlargeButton/EnlargeButton.stories.tsx new file mode 100644 index 00000000..9d77efd7 --- /dev/null +++ b/clients/web/src/components/elements/EnlargeButton/EnlargeButton.stories.tsx @@ -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 = { + title: "Elements/EnlargeButton", + component: EnlargeButton, + args: { onClick: () => {} }, +}; + +export default meta; +type Story = StoryObj; + +// 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(); + }, +}; diff --git a/clients/web/src/components/elements/EnlargeButton/EnlargeButton.test.tsx b/clients/web/src/components/elements/EnlargeButton/EnlargeButton.test.tsx new file mode 100644 index 00000000..ccb37dc5 --- /dev/null +++ b/clients/web/src/components/elements/EnlargeButton/EnlargeButton.test.tsx @@ -0,0 +1,36 @@ +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(); + expect(screen.getByRole("button", { name: "Enlarge" })).toBeInTheDocument(); + }); + + it("stays in the keyboard tab order", () => { + renderWithMantine(); + 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(); + await user.click(screen.getByRole("button", { name: "Enlarge" })); + expect(onClick).toHaveBeenCalledTimes(1); + }); + + it("allows the accessible name to be overridden", () => { + renderWithMantine( + , + ); + expect( + screen.getByRole("button", { name: "Enlarge Query" }), + ).toBeInTheDocument(); + }); +}); diff --git a/clients/web/src/components/elements/EnlargeButton/EnlargeButton.tsx b/clients/web/src/components/elements/EnlargeButton/EnlargeButton.tsx new file mode 100644 index 00000000..193c0de2 --- /dev/null +++ b/clients/web/src/components/elements/EnlargeButton/EnlargeButton.tsx @@ -0,0 +1,45 @@ +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; +} + +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 `` 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 }: EnlargeButtonProps) { + return ( + + + + + + ); +} diff --git a/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx b/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx index b1d3182f..722ebf5b 100644 --- a/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx +++ b/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx @@ -1790,3 +1790,106 @@ describe("SchemaForm defaulted fields (#2026)", () => { expect(jsonInput.value).toBe('[\n "z"\n]'); }); }); + +// A plain `` 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>({}); + return ( + + ); + } + + 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(); + }); + + 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" })); + + const textarea = screen.getByLabelText(/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(); + await user.click(screen.getByRole("button", { name: "Enlarge" })); + expect(screen.getByLabelText(/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" })); + expect( + screen.queryByRole("button", { name: "Enlarge" }), + ).not.toBeInTheDocument(); + }); + + it("keeps the clear button working after enlarging", async () => { + 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; + 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(); + expect( + screen.queryByRole("button", { name: "Clear" }), + ).not.toBeInTheDocument(); + await user.type(screen.getByLabelText(/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(); + + await user.click(screen.getByRole("button", { name: "Enlarge" })); + expect(screen.getByLabelText(/Note/).tagName).toBe("TEXTAREA"); + + rerender(); + expect(screen.getByLabelText(/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" })); + rerender(); + expect(screen.getByLabelText(/Note/).tagName).toBe("TEXTAREA"); + }); +}); diff --git a/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx b/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx index 152fe40f..6edb9795 100644 --- a/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx +++ b/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx @@ -1,15 +1,24 @@ import { Checkbox, + Group, JsonInput, MultiSelect, NumberInput, Select, Stack, Text, + Textarea, TextInput, } from "@mantine/core"; -import { useCallback, useEffect, useRef, useState } from "react"; +import { + useCallback, + useEffect, + useRef, + useState, + type ChangeEvent, +} from "react"; import { ClearButton } from "../../elements/ClearButton/ClearButton"; +import { EnlargeButton } from "../../elements/EnlargeButton/EnlargeButton"; import { useValueChange } from "../../../hooks/useValueChange"; import type { InspectorFormSchema, @@ -38,6 +47,26 @@ const SchemaJsonInput = JsonInput.withProps({ 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", +}); + +// 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" }); + +// Widths of the string field's right section, which has to be declared rather +// than measured. One button's worth, and two. +const ONE_ACTION_WIDTH = 34; +const TWO_ACTION_WIDTH = 62; + function serializeJson(value: unknown): string { return JSON.stringify(value, null, 2); } @@ -507,6 +536,19 @@ export function SchemaForm({ () => 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>( + () => 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 // updater returns the previous set unchanged when nothing moved, which is // what makes a nested form's inline callback safe to call repeatedly. @@ -604,24 +646,52 @@ export function SchemaForm({ // plain string if (fieldSchema.type === "string") { + const clearButton = rawValue ? ( + handleFieldChange(fieldName, "")} /> + ) : null; + const sharedProps = { + label, + description, + withAsterisk: isRequired, + disabled, + value: (rawValue as string) ?? "", + minLength: fieldSchema.minLength, + maxLength: fieldSchema.maxLength, + onChange: ( + event: ChangeEvent, + ) => 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 ( + + ); + } + return ( - handleFieldChange(fieldName, event.currentTarget.value) - } + {...sharedProps} rightSectionPointerEvents="auto" + rightSectionWidth={clearButton ? TWO_ACTION_WIDTH : ONE_ACTION_WIDTH} rightSection={ - rawValue ? ( - handleFieldChange(fieldName, "")} /> - ) : null + + + setEnlargedFields( + (previous) => new Set([...previous, fieldName]), + ) + } + /> + {clearButton} + } /> );