fix(web): take the enlarge button out of the tab order

Every string field in a tool or elicitation form carries an enlarge
button in its right section, so tabbing through a form stopped on one
between each pair of fields. Give it tabIndex={-1}, as #1487 did for the
clear button.

That button was deliberately left in the tab order by #2042, on the
grounds that clearing has a keyboard equivalent and enlarging has none.
So give it one: Enter on the focused single-line field enlarges it and
enters the newline. Nothing was listening for that key there — no
consumer renders SchemaForm inside a <form> — and it is the key a user
presses trying to type the newline the input swallows, which is the
failure #2042 exists to fix.

Enter enters the newline rather than only reshaping the field, since
consuming the keystroke leaves the next word running on from the last. A
field already at its maxLength is enlarged without one. Shift+Enter
enlarges too; the Ctrl/Cmd/Alt chords are left free for a consumer to
bind to running the tool. Clicking the button still only enlarges.

Signed-off-by: cliffhall <cliff@futurescale.com>

clients/web/src/components/elements/EnlargeButton/EnlargeButton.stories.tsx

@@ -11,12 +11,15 @@ const meta: Meta<typeof EnlargeButton> = {
 export default meta;
 type Story = StoryObj<typeof EnlargeButton>;
 
-// Default: labelled "Enlarge", reachable by keyboard, and clickable.
+// Default: labelled "Enlarge", clickable, and out of the tab order (#2138) —
+// a form's string fields each carry one, so leaving them in doubled the tab
+// stops between adjacent fields. SchemaForm binds Enter on the field itself as
+// the keyboard route in.
 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 expect(button).toHaveAttribute("tabindex", "-1");
     await userEvent.click(button);
   },
 };

clients/web/src/components/elements/EnlargeButton/EnlargeButton.test.tsx

@@ -9,14 +9,33 @@ describe("EnlargeButton", () => {
     expect(screen.getByRole("button", { name: "Enlarge" })).toBeInTheDocument();
   });
 
-  it("stays in the keyboard tab order", () => {
+  // Out of the tab order like ClearButton (#2138) — a form's string fields each
+  // carry one, so leaving them in doubled the stops between adjacent fields.
+  it("is out of the keyboard tab order", () => {
     renderWithMantine(<EnlargeButton onClick={vi.fn()} />);
-    expect(screen.getByRole("button", { name: "Enlarge" })).not.toHaveAttribute(
+    expect(screen.getByRole("button", { name: "Enlarge" })).toHaveAttribute(
       "tabindex",
       "-1",
     );
   });
 
+  // The property that matters is skipped-by-Tab, not the attribute that
+  // implements it — asserted by driving a real Tab past a neighbouring input.
+  it("is skipped when tabbing through the surrounding form", async () => {
+    const user = userEvent.setup();
+    renderWithMantine(
+      <>
+        <input aria-label="Before" />
+        <EnlargeButton onClick={vi.fn()} />
+        <input aria-label="After" />
+      </>,
+    );
+
+    screen.getByRole("textbox", { name: "Before" }).focus();
+    await user.tab();
+    expect(screen.getByRole("textbox", { name: "After" })).toHaveFocus();
+  });
+
   it("invokes onClick when clicked", async () => {
     const user = userEvent.setup();
     const onClick = vi.fn();

clients/web/src/components/elements/EnlargeButton/EnlargeButton.tsx

@@ -23,6 +23,7 @@ const EnlargeActionIcon = ActionIcon.withProps({
   variant: "subtle",
   color: "gray",
   size: "sm",
+  tabIndex: -1,
 });
 
 /**
@@ -35,10 +36,18 @@ const EnlargeActionIcon = ActionIcon.withProps({
  * 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
+ * `tabIndex={-1}` keeps it clickable but out of the keyboard tab order, the same
- * keyboard equivalent (select-all, delete), so #1487 could take that button out
+ * as ClearButton (#1487) — a form's string fields each carry one, so leaving
- * of the tab order without cost; entering multiline mode has none, so removing
+ * them in doubled the tab stops between one field and the next (#2138).
- * this one would put the feature out of reach of keyboard users entirely.
+ *
+ * That is only affordable because the keyboard keeps its own way in: SchemaForm
+ * binds Enter on the single-line field, which enlarges it and enters the
+ * newline in one go. Nothing else is listening for that key there, and it is
+ * the one a user presses trying to type the newline the field cannot hold — so
+ * the gesture that fails is the gesture that fixes it. Do not take this button
+ * out of the tab order anywhere that binding is absent; unlike clearing
+ * (select-all, delete) there is no built-in equivalent, and multiline mode
+ * would simply be unreachable by keyboard.
  */
 export function EnlargeButton({
   onClick,

clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx

@@ -1932,22 +1932,26 @@ describe("SchemaForm multiline strings (#2042)", () => {
     ).toBeInTheDocument();
   });
 
-  // The button unmounts in the same commit that mounts the text area, so
+  // The button unmounts in the same commit that mounts the text area, taking
-  // without an explicit hand-off a keyboard user is left focused on nothing.
+  // the focused element with it, so without an explicit hand-off the user is
+  // left focused on nothing. (The keyboard route hands off from the field
+  // instead — see the #2138 suite.)
   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();
+    await user.click(screen.getByRole("button", { name: "Enlarge Note" }));
-    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();
+    // Clicking asks for a bigger box and nothing else: the value is carried
+    // over untouched, and only Enter — the key that means "new line" — enters
+    // one (#2138).
+    expect(textarea.value).toBe("typed");
     expect(textarea.selectionStart).toBe("typed".length);
 
     // And the caret really is at the end: typing appends rather than prepends.
@@ -1970,3 +1974,202 @@ describe("SchemaForm multiline strings (#2042)", () => {
     expect(screen.getByRole("button", { name: "Enlarge Note" })).toBeDisabled();
   });
 });
+
+// The enlarge button is clickable but out of the tab order, so tabbing runs
+// field to field; Enter in a single-line string field takes its place as the
+// keyboard route into multiline mode (#2138).
+describe("SchemaForm enlarge keyboard access (#2138)", () => {
+  const twoStringSchema: InspectorFormSchema = {
+    type: "object",
+    properties: {
+      note: { type: "string", title: "Note" },
+      summary: { type: "string", title: "Summary" },
+    },
+  };
+
+  function TwoStringHarness({ disabled }: { disabled?: boolean }) {
+    const [values, setValues] = useState<Record<string, unknown>>({});
+    return (
+      <SchemaForm
+        schema={twoStringSchema}
+        values={values}
+        onChange={setValues}
+        disabled={disabled}
+      />
+    );
+  }
+
+  const noteField = () =>
+    screen.getByRole("textbox", { name: /Note/ }) as HTMLTextAreaElement;
+
+  // The defect the issue was filed for: an extra stop per field, on every
+  // string field of every tool form.
+  it("tabs from one string field to the next, skipping the enlarge buttons", async () => {
+    const user = userEvent.setup();
+    renderWithMantine(<TwoStringHarness />);
+
+    noteField().focus();
+    await user.tab();
+
+    expect(screen.getByRole("textbox", { name: /Summary/ })).toHaveFocus();
+  });
+
+  // A populated field renders the clear button too, so this is the widest the
+  // right section ever gets — and still must not add a stop.
+  it("skips both right-section buttons when the field holds a value", async () => {
+    const user = userEvent.setup();
+    renderWithMantine(<TwoStringHarness />);
+
+    await user.type(noteField(), "typed");
+    expect(screen.getByRole("button", { name: "Clear" })).toBeInTheDocument();
+    await user.tab();
+
+    expect(screen.getByRole("textbox", { name: /Summary/ })).toHaveFocus();
+  });
+
+  it("enlarges the focused field when Enter is pressed", async () => {
+    const user = userEvent.setup();
+    renderWithMantine(<TwoStringHarness />);
+
+    expect(noteField().tagName).toBe("INPUT");
+    noteField().focus();
+    await user.keyboard("{Enter}");
+
+    expect(noteField().tagName).toBe("TEXTAREA");
+  });
+
+  // Enlarging is per field: the key must not reach the neighbour.
+  it("enlarges only the focused field", async () => {
+    const user = userEvent.setup();
+    renderWithMantine(<TwoStringHarness />);
+
+    noteField().focus();
+    await user.keyboard("{Enter}");
+
+    expect(screen.getByRole("textbox", { name: /Summary/ }).tagName).toBe(
+      "INPUT",
+    );
+  });
+
+  // The other gesture a user reaches for when an input will not take a newline.
+  it("enlarges on Shift+Enter too", async () => {
+    const user = userEvent.setup();
+    renderWithMantine(<TwoStringHarness />);
+
+    noteField().focus();
+    await user.keyboard("{Shift>}{Enter}{/Shift}");
+
+    expect(noteField().tagName).toBe("TEXTAREA");
+  });
+
+  // Those chords read as "submit" and stay free for a consumer to bind to
+  // running the tool; claiming them would silently enlarge a field instead.
+  it.each([
+    ["Ctrl", "{Control>}{Enter}{/Control}"],
+    ["Meta", "{Meta>}{Enter}{/Meta}"],
+    ["Alt", "{Alt>}{Enter}{/Alt}"],
+  ])("leaves %s+Enter alone", async (_name, keys) => {
+    const user = userEvent.setup();
+    renderWithMantine(<TwoStringHarness />);
+
+    noteField().focus();
+    await user.keyboard(keys);
+
+    expect(noteField().tagName).toBe("INPUT");
+  });
+
+  it("leaves the field alone for any other key", async () => {
+    const user = userEvent.setup();
+    renderWithMantine(<TwoStringHarness />);
+
+    await user.type(noteField(), "text ");
+    await user.keyboard("{Escape}{ArrowDown} ");
+
+    expect(noteField().tagName).toBe("INPUT");
+  });
+
+  // The keystroke has to mean what it says. Enlarging without entering the
+  // newline consumes the key and leaves the next word running on from the last
+  // one — the user pressed "new line" and got a reshaped box.
+  it("enters the newline it was asked for, not just the text area", async () => {
+    const user = userEvent.setup();
+    renderWithMantine(<TwoStringHarness />);
+
+    await user.type(noteField(), "before");
+    await user.keyboard("{Enter}");
+    expect(noteField().value).toBe("before\n");
+
+    await user.keyboard("after");
+    expect(noteField().value).toBe("before\nafter");
+  });
+
+  // The caret follows the newline, so what is typed next lands under it rather
+  // than back on the first line.
+  it("leaves the caret after the newline", async () => {
+    const user = userEvent.setup();
+    renderWithMantine(<TwoStringHarness />);
+
+    await user.type(noteField(), "before");
+    await user.keyboard("{Enter}");
+
+    expect(noteField().selectionStart).toBe("before\n".length);
+  });
+
+  // A newline is a character, so a field with no room for one is enlarged
+  // without it rather than pushed past a constraint its schema states.
+  it("enlarges without a newline when the field is at its maxLength", async () => {
+    const user = userEvent.setup();
+    renderWithMantine(
+      <SchemaForm
+        schema={{
+          type: "object",
+          properties: { note: { type: "string", title: "Note", maxLength: 3 } },
+        }}
+        values={{ note: "abc" }}
+        onChange={vi.fn()}
+      />,
+    );
+
+    noteField().focus();
+    await user.keyboard("{Enter}");
+
+    expect(noteField().tagName).toBe("TEXTAREA");
+    expect(noteField().value).toBe("abc");
+  });
+
+  // An empty field still has room, so the newline is entered there too.
+  it("enters a newline in an empty field", async () => {
+    const user = userEvent.setup();
+    renderWithMantine(<TwoStringHarness />);
+
+    noteField().focus();
+    await user.keyboard("{Enter}");
+
+    expect(noteField().value).toBe("\n");
+  });
+
+  // The binding is the keyboard's only route in now, so announce that the
+  // field carries one rather than leaving it undiscoverable.
+  it("advertises the shortcut on the single-line field, not the text area", async () => {
+    const user = userEvent.setup();
+    renderWithMantine(<TwoStringHarness />);
+
+    expect(noteField()).toHaveAttribute("aria-keyshortcuts", "Enter");
+    noteField().focus();
+    await user.keyboard("{Enter}");
+
+    expect(noteField()).not.toHaveAttribute("aria-keyshortcuts");
+  });
+
+  // Same reasoning as the disabled button: a text area mounting disabled cannot
+  // take focus, so it would drop focus to the document.
+  it("cannot be enlarged by keyboard while the form is disabled", async () => {
+    const user = userEvent.setup();
+    renderWithMantine(<TwoStringHarness disabled />);
+
+    noteField().focus();
+    await user.keyboard("{Enter}");
+
+    expect(noteField().tagName).toBe("INPUT");
+  });
+});

clients/web/src/components/groups/SchemaForm/SchemaForm.tsx

@@ -718,22 +718,67 @@ export function SchemaForm({
         );
       }
 
+      const enlarge = () =>
+        setEnlargedFields((previous) => new Set([...previous, fieldName]));
+
+      // Enter both enlarges the field and enters the newline that was asked
+      // for. Enlarging alone would consume the keystroke and leave the caret
+      // where it was, so the next thing typed runs on from the last word — the
+      // user pressed the key that means "new line" and got a reshaped box.
+      //
+      // The newline lands at the end rather than at the caret because that is
+      // where EnlargedStringField puts the caret regardless (see its comment);
+      // going anywhere else would separate the newline from the text that
+      // follows it. A field already at its maxLength is enlarged without one,
+      // since the alternative is breaching a constraint the schema states.
+      const enlargeWithNewline = () => {
+        const current = (rawValue as string) ?? "";
+        const room =
+          fieldSchema.maxLength === undefined ||
+          current.length < fieldSchema.maxLength;
+        if (room) handleFieldChange(fieldName, `${current}\n`);
+        enlarge();
+      };
+
       return (
         <TextInput
           key={fieldName}
           {...sharedProps}
           rightSectionPointerEvents="auto"
           rightSectionWidth={clearButton ? TWO_ACTION_WIDTH : ONE_ACTION_WIDTH}
+          // The keyboard's way into multiline mode, now that the enlarge button
+          // is out of the tab order (#2138). Enter is inert in this field —
+          // nothing renders SchemaForm inside a `<form>`, so there is no
+          // implicit submission to displace — and it is the very key a user
+          // presses trying to enter the newline a single-line input swallows,
+          // which is what #2042 exists to fix. So the gesture that fails is the
+          // one that enlarges, rather than a shortcut nobody would guess.
+          onKeyDown={(event) => {
+            // Shift+Enter is the other newline gesture, so it enlarges too. The
+            // Ctrl/Cmd/Alt chords are deliberately left alone: those read as
+            // "submit" in a form, and a consumer binding one (run the tool)
+            // must not be overridden into enlarging a field instead.
+            if (
+              event.key !== "Enter" ||
+              event.ctrlKey ||
+              event.metaKey ||
+              event.altKey
+            ) {
+              return;
+            }
+            event.preventDefault();
+            enlargeWithNewline();
+          }}
+          // Announces that the field carries a shortcut at all. A keyboard user
+          // no longer meets the button by tabbing, so without this the binding
+          // is undiscoverable rather than merely unlabelled.
+          aria-keyshortcuts="Enter"
           rightSection={
             <FieldActions>
               <EnlargeButton
                 ariaLabel={`Enlarge ${label}`}
                 disabled={disabled}
-                onClick={() =>
+                onClick={enlarge}
-                  setEnlargedFields(
-                    (previous) => new Set([...previous, fieldName]),
-                  )
-                }
               />
               {clearButton}
             </FieldActions>