Merge pull request #2139 from modelcontextprotocol/v2/feat/2138-enlarge-button-tab-order

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

Author: cliffhall

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
- * 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.
+ * `tabIndex={-1}` keeps it clickable but out of the keyboard tab order, the same
+ * as ClearButton (#1487) — a form's string fields each carry one, so leaving
+ * them in doubled the tab stops between one field and the next (#2138).
+ *
+ * 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,