From 6f9a2595c139b27e84d02bba052c2e17182354d0 Mon Sep 17 00:00:00 2001
From: Wenchao An <ansusu1025@gmail.com>
Date: Tue, 15 Sep 2026 21:28:24 +0800
Subject: [PATCH] fix(agents): keep settings dialog within the viewport (#5458)

* fix(agents): keep settings dialog within the viewport

* fix(agents): reveal and describe expanded subagent details
---
 .../agents/agent-settings-dialog.tsx          |  88 +++---
 .../tests/e2e/agent-settings-layout.spec.ts   | 255 ++++++++++++++++++
 2 files changed, 313 insertions(+), 30 deletions(-)
 create mode 100644 frontend/tests/e2e/agent-settings-layout.spec.ts

diff --git a/frontend/src/components/workspace/agents/agent-settings-dialog.tsx b/frontend/src/components/workspace/agents/agent-settings-dialog.tsx
index 0e8154301..ba6bba57f 100644
--- a/frontend/src/components/workspace/agents/agent-settings-dialog.tsx
+++ b/frontend/src/components/workspace/agents/agent-settings-dialog.tsx
@@ -1,6 +1,7 @@
 "use client";
 
-import { useMemo, useState } from "react";
+import { ChevronRightIcon } from "lucide-react";
+import { useId, useMemo, useState } from "react";
 import { toast } from "sonner";
 
 import { Button } from "@/components/ui/button";
@@ -61,6 +62,7 @@ export function AgentSettingsDialog({
   const { t } = useI18n();
   const { models } = useModels();
   const { subagents } = useSubagents();
+  const subagentDescriptionId = useId();
   const updateAgent = useUpdateAgent();
   const [displayName, setDisplayName] = useState(agent.display_name ?? "");
 
@@ -164,13 +166,13 @@ export function AgentSettingsDialog({
 
   return (
     <Dialog open={open} onOpenChange={onOpenChange}>
-      <DialogContent>
-        <DialogHeader>
+      <DialogContent className="max-h-[calc(100dvh-2rem)] grid-rows-[auto_minmax(0,1fr)_auto] overflow-hidden">
+        <DialogHeader className="pr-6">
           <DialogTitle>{t.agents.settingsTitle}</DialogTitle>
           <DialogDescription>{t.agents.settingsDescription}</DialogDescription>
         </DialogHeader>
 
-        <div className="space-y-4 py-1">
+        <div className="min-h-0 min-w-0 space-y-4 overflow-y-auto overscroll-contain px-1 py-1">
           <div className="space-y-1.5">
             <label htmlFor="agent-display-name" className="text-sm font-medium">
               {t.agents.settingsDisplayName}
@@ -333,33 +335,59 @@ export function AgentSettingsDialog({
               </SelectContent>
             </Select>
             {subagentAccess === "selected" && (
-              <div className="max-h-40 space-y-2 overflow-y-auto rounded-md border p-3">
+              <div className="space-y-3 rounded-md border p-3">
                 {selectableSubagents.map((item) => (
-                  <label
-                    key={item.name}
-                    className="flex items-start gap-2 text-sm"
-                  >
-                    <input
-                      type="checkbox"
-                      className="mt-0.5 size-4"
-                      checked={selectedSubagents.includes(item.name)}
-                      onChange={(event) =>
-                        setSelectedSubagents((current) =>
-                          event.target.checked
-                            ? [...current, item.name]
-                            : current.filter((name) => name !== item.name),
-                        )
-                      }
-                    />
-                    <span>
-                      <span className="font-medium">
+                  <div key={item.name} className="min-w-0 space-y-1">
+                    <label className="flex items-start gap-2 text-sm">
+                      <input
+                        type="checkbox"
+                        className="mt-0.5 size-4 shrink-0"
+                        checked={selectedSubagents.includes(item.name)}
+                        onChange={(event) =>
+                          setSelectedSubagents((current) =>
+                            event.target.checked
+                              ? [...current, item.name]
+                              : current.filter((name) => name !== item.name),
+                          )
+                        }
+                      />
+                      <span className="min-w-0 font-medium [overflow-wrap:anywhere]">
                         {item.display_name ?? item.name}
                       </span>
-                      <span className="text-muted-foreground block text-xs">
-                        {item.description}
-                      </span>
-                    </span>
-                  </label>
+                    </label>
+                    {item.description && (
+                      <details
+                        className="group text-muted-foreground ml-6 text-xs"
+                        onToggle={(event) => {
+                          if (event.currentTarget.open) {
+                            event.currentTarget
+                              .querySelector("p")
+                              ?.scrollIntoView({ block: "nearest" });
+                          }
+                        }}
+                      >
+                        <summary
+                          className="focus-visible:ring-ring flex cursor-pointer list-none items-start gap-1 rounded-sm focus-visible:ring-2 [&::-webkit-details-marker]:hidden"
+                          aria-label={`${item.display_name ?? item.name}: ${t.settings.subagents.descriptionLabel}`}
+                          aria-describedby={`${subagentDescriptionId}-${item.name}`}
+                        >
+                          <ChevronRightIcon className="size-3 shrink-0 transition-transform group-open:rotate-90" />
+                          <span
+                            id={`${subagentDescriptionId}-${item.name}`}
+                            className="line-clamp-2 [overflow-wrap:anywhere] group-open:hidden"
+                          >
+                            {item.description}
+                          </span>
+                          <span className="hidden group-open:inline">
+                            {t.settings.subagents.descriptionLabel}
+                          </span>
+                        </summary>
+                        <p className="mt-1 [overflow-wrap:anywhere] whitespace-pre-wrap">
+                          {item.description}
+                        </p>
+                      </details>
+                    )}
+                  </div>
                 ))}
                 {missingSubagents.map((name) => (
                   <label
@@ -368,7 +396,7 @@ export function AgentSettingsDialog({
                   >
                     <input
                       type="checkbox"
-                      className="mt-0.5 size-4"
+                      className="mt-0.5 size-4 shrink-0"
                       checked
                       onChange={() =>
                         setSelectedSubagents((current) =>
@@ -376,7 +404,7 @@ export function AgentSettingsDialog({
                         )
                       }
                     />
-                    <span>
+                    <span className="min-w-0 [overflow-wrap:anywhere]">
                       <span className="font-medium">{name}</span>
                       <span className="block text-xs">
                         {t.settings.subagents.missing}
diff --git a/frontend/tests/e2e/agent-settings-layout.spec.ts b/frontend/tests/e2e/agent-settings-layout.spec.ts
new file mode 100644
index 000000000..215d04a07
--- /dev/null
+++ b/frontend/tests/e2e/agent-settings-layout.spec.ts
@@ -0,0 +1,255 @@
+import { expect, test, type Locator, type Page } from "@playwright/test";
+
+import { mockLangGraphAPI } from "./utils/mock-api";
+
+const description =
+  "A general-purpose worker for independent analysis. " +
+  "Investigate the delegated question, check the evidence, and report findings to the caller. ".repeat(
+    15,
+  );
+
+const longName = "research" + "a".repeat(57);
+const missingName = "missing" + "b".repeat(58);
+
+async function expectInsideViewport(element: Locator) {
+  await expect(element).toBeVisible();
+  const bounds = await element.boundingBox();
+  const viewport = element.page().viewportSize()!;
+  expect(bounds).not.toBeNull();
+  expect(bounds!.x).toBeGreaterThanOrEqual(0);
+  expect(bounds!.y).toBeGreaterThanOrEqual(0);
+  expect(bounds!.x + bounds!.width).toBeLessThanOrEqual(viewport.width);
+  expect(bounds!.y + bounds!.height).toBeLessThanOrEqual(viewport.height);
+}
+
+async function openSettings(
+  page: Page,
+  allowedSubagents: string[] | null = null,
+) {
+  const agent = {
+    name: "test-agent",
+    model: "test-model",
+    thinking_enabled: true,
+    reasoning_effort: "high",
+    allowed_subagents: allowedSubagents,
+  };
+  mockLangGraphAPI(page, { agents: [agent] });
+  await page.route("**/api/models", (route) =>
+    route.fulfill({
+      json: {
+        models: [
+          {
+            name: "test-model",
+            display_name: "Test model",
+            supports_thinking: true,
+            supports_reasoning_effort: true,
+          },
+        ],
+      },
+    }),
+  );
+  await page.route("**/api/subagents", (route) =>
+    route.fulfill({
+      json: {
+        subagents: [
+          { name: "general-purpose", description, enabled: true },
+          { name: "bash", description: "Run shell commands.", enabled: true },
+          {
+            name: longName,
+            description: "A worker with a long name.",
+            enabled: true,
+          },
+        ],
+      },
+    }),
+  );
+  let saved: Record<string, unknown> | undefined;
+  await page.route("**/api/agents/test-agent", (route) => {
+    if (route.request().method() === "PUT") {
+      saved = route.request().postDataJSON() as Record<string, unknown>;
+      return route.fulfill({ json: { ...agent, ...saved } });
+    }
+    return route.fulfill({ json: agent });
+  });
+  await page.goto("/workspace/agents");
+  await page.getByTitle("Agent settings", { exact: true }).click();
+  const dialog = page.getByRole("dialog");
+  await dialog.getByRole("combobox").last().click();
+  await page
+    .getByRole("option", { name: "Selected subagents", exact: true })
+    .click();
+  return { dialog, saved: () => saved };
+}
+
+for (const viewport of [
+  { width: 1280, height: 720 },
+  { width: 390, height: 640 },
+]) {
+  test(`agent settings remain usable at ${viewport.width}x${viewport.height}`, async ({
+    page,
+  }, testInfo) => {
+    await page.setViewportSize(viewport);
+    const { dialog, saved } = await openSettings(page);
+    await expectInsideViewport(dialog);
+    await expectInsideViewport(
+      dialog.getByRole("heading", { name: "Agent settings" }),
+    );
+    await expectInsideViewport(
+      dialog.getByRole("button", { name: "Close", exact: true }),
+    );
+    await expectInsideViewport(
+      dialog.getByRole("button", { name: "Save", exact: true }),
+    );
+    await expectInsideViewport(
+      dialog.getByRole("button", { name: "Cancel", exact: true }),
+    );
+
+    const general = dialog.getByRole("checkbox", {
+      name: "general-purpose",
+      exact: true,
+    });
+    await general.check();
+    const details = dialog.locator("details").first();
+    const summary = details.locator("summary");
+    await summary.scrollIntoViewIfNeeded();
+    const preview = summary.locator("span").first();
+    for (const lineHeight of [null, "20px"]) {
+      if (lineHeight)
+        await preview.evaluate((element, value) => {
+          element.style.lineHeight = value;
+        }, lineHeight);
+      const metrics = await preview.evaluate((element) => ({
+        height: element.getBoundingClientRect().height,
+        lineHeight: parseFloat(getComputedStyle(element).lineHeight),
+      }));
+      expect(metrics.height).toBeGreaterThan(0);
+      expect(metrics.height).toBeLessThanOrEqual(metrics.lineHeight * 2 + 1);
+    }
+    await summary.focus();
+    await page.keyboard.press("Enter");
+    await expect(details).toHaveAttribute("open", "");
+    await expect(details.locator("p")).toHaveText(description);
+    await expect(general).toBeChecked();
+    await expectInsideViewport(
+      dialog.getByRole("button", { name: "Save", exact: true }),
+    );
+    await page.keyboard.press("Enter");
+    await expect(details).not.toHaveAttribute("open", "");
+    await dialog.getByRole("checkbox", { name: "bash", exact: true }).check();
+    const screenshotPath = testInfo.outputPath("agent-settings.png");
+    await page.screenshot({ path: screenshotPath });
+    await testInfo.attach("agent-settings", {
+      path: screenshotPath,
+      contentType: "image/png",
+    });
+    await dialog.getByRole("button", { name: "Save", exact: true }).click();
+    await expect(dialog).toBeHidden();
+    expect(saved()?.allowed_subagents).toEqual(["general-purpose", "bash"]);
+  });
+
+  test(`collapsed descriptions are accessible at ${viewport.width}x${viewport.height}`, async ({
+    page,
+  }) => {
+    await page.setViewportSize(viewport);
+    const { dialog } = await openSettings(page);
+    const summary = dialog.locator("summary").first();
+    await summary.focus();
+    await expect(summary).toHaveAccessibleName(
+      "general-purpose: Delegation description",
+    );
+    await expect(summary).toHaveAccessibleDescription(description);
+    await expect(summary.locator("..")).not.toHaveAttribute("open", "");
+  });
+
+  test(`bottom-edge expansion reveals text at ${viewport.width}x${viewport.height}`, async ({
+    page,
+  }) => {
+    await page.setViewportSize(viewport);
+    const { dialog } = await openSettings(page);
+    const details = dialog.locator("details").first();
+    const summary = details.locator("summary");
+    await summary.focus();
+    // Reproduce scrolling a focused disclosure to the last eight visible
+    // pixels. A locator click would auto-scroll and hide this bug.
+    const clickPoint = await summary.evaluate((element) => {
+      let scrollArea = element.parentElement!;
+      while (getComputedStyle(scrollArea).overflowY !== "auto")
+        scrollArea = scrollArea.parentElement!;
+      const area = scrollArea.getBoundingClientRect();
+      scrollArea.scrollTop +=
+        element.getBoundingClientRect().top - (area.bottom - 8);
+      const bounds = element.getBoundingClientRect();
+      return { x: bounds.x + 5, y: bounds.y + 4 };
+    });
+    await page.mouse.click(clickPoint.x, clickPoint.y);
+    await expect(details).toHaveAttribute("open", "");
+    await expect
+      .poll(() =>
+        details.locator("p").evaluate((paragraph) => {
+          let scrollArea = paragraph.parentElement!;
+          while (getComputedStyle(scrollArea).overflowY !== "auto")
+            scrollArea = scrollArea.parentElement!;
+          const area = scrollArea.getBoundingClientRect();
+          const bounds = paragraph.getBoundingClientRect();
+          return (
+            bounds.top >= area.top - 1 &&
+            bounds.top + parseFloat(getComputedStyle(paragraph).lineHeight) <=
+              area.bottom
+          );
+        }),
+      )
+      .toBe(true);
+    await expect(summary).toBeFocused();
+    await expectInsideViewport(
+      dialog.getByRole("button", { name: "Save", exact: true }),
+    );
+    await page.keyboard.press("Enter");
+    await expect(details).not.toHaveAttribute("open", "");
+    await expect(
+      dialog.getByRole("checkbox", { name: "general-purpose", exact: true }),
+    ).not.toBeChecked();
+  });
+
+  test(`long worker names wrap at ${viewport.width}x${viewport.height}`, async ({
+    page,
+  }) => {
+    await page.setViewportSize(viewport);
+    const { dialog, saved } = await openSettings(page, [missingName]);
+    for (const name of [longName, missingName]) {
+      const checkbox = dialog.getByRole("checkbox", {
+        name: new RegExp(`^${name}`),
+      });
+      await checkbox.scrollIntoViewIfNeeded();
+      await expectInsideViewport(checkbox);
+      const metrics = await checkbox.evaluate((element) => {
+        const label = element.closest("label")!;
+        const nameElement = label.querySelector("span.font-medium")!;
+        const bounds = nameElement.getBoundingClientRect();
+        const box = element.getBoundingClientRect();
+        const labelBounds = label.getBoundingClientRect();
+        return {
+          nameHeight: bounds.height,
+          lineHeight: parseFloat(getComputedStyle(nameElement).lineHeight),
+          fits:
+            bounds.left >= labelBounds.left &&
+            bounds.right <= labelBounds.right + 1,
+          checkboxWidth: box.width,
+          expectedWidth: parseFloat(getComputedStyle(element).height),
+        };
+      });
+      expect(metrics.nameHeight).toBeGreaterThan(metrics.lineHeight);
+      expect(metrics.fits).toBe(true);
+      expect(metrics.checkboxWidth).toBeCloseTo(metrics.expectedWidth, 1);
+      if (name === longName) {
+        await checkbox.check();
+      } else {
+        // Removing an unavailable selection also removes its row.
+        await checkbox.click();
+        await expect(checkbox).toHaveCount(0);
+      }
+    }
+    await dialog.getByRole("button", { name: "Save", exact: true }).click();
+    await expect(dialog).toBeHidden();
+    expect(saved()?.allowed_subagents).toEqual([longName]);
+  });
+}