fix(frontend): keep MCP configuration dialog within viewport (#5492)

* fix(frontend): keep MCP configuration dialog within viewport

* fix(frontend): keep MCP dialog actions outside fallback scroll area
This commit is contained in:
Wenchao An 2026-09-17 08:31:16 +08:00 committed by GitHub
parent e831720304
commit de02eeb856
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 186 additions and 18 deletions

View File

@ -287,14 +287,16 @@ function MCPServerList({
open={editor !== null}
onOpenChange={(open) => !open && !isWriting && closeEditor()}
>
<DialogContent>
<DialogHeader>
<DialogContent className="flex max-h-[calc(100dvh-2rem)] flex-col overflow-hidden sm:max-w-2xl">
<DialogHeader className="shrink-0 pr-6 break-words">
<DialogTitle>
{editor?.mode === "edit"
? t.settings.tools.editServer
: t.settings.tools.addServer}
</DialogTitle>
<DialogDescription>
</DialogHeader>
<div className="-m-1 flex min-h-0 flex-col gap-4 overflow-y-auto p-1">
<DialogDescription className="shrink-0 text-center break-words sm:text-left">
{editor?.mode === "edit"
? t.settings.tools.editServerDescription.replace(
"{name}",
@ -302,21 +304,24 @@ function MCPServerList({
)
: t.settings.tools.addServerDescription}
</DialogDescription>
</DialogHeader>
<Textarea
className="min-h-52 font-mono text-xs"
aria-label={t.settings.tools.serverDefinitionLabel}
spellCheck={false}
value={definition}
placeholder={t.settings.tools.addServerPlaceholder}
onChange={(event) => setDefinition(event.target.value)}
/>
{definitionError && (
<div className="text-destructive text-sm" role="alert">
{definitionError}
</div>
)}
<DialogFooter>
<Textarea
className="field-sizing-fixed h-96 min-h-24 resize-none overflow-auto font-mono text-xs"
aria-label={t.settings.tools.serverDefinitionLabel}
spellCheck={false}
value={definition}
placeholder={t.settings.tools.addServerPlaceholder}
onChange={(event) => setDefinition(event.target.value)}
/>
{definitionError && (
<div
className="text-destructive shrink-0 text-sm break-words"
role="alert"
>
{definitionError}
</div>
)}
</div>
<DialogFooter className="shrink-0">
<Button
variant="outline"
disabled={isWriting}

View File

@ -2,6 +2,169 @@ import { expect, test } from "@playwright/test";
import { mockLangGraphAPI } from "./utils/mock-api";
for (const { viewport, name } of [
{ viewport: { width: 500, height: 280 }, name: "github" },
{
viewport: { width: 844, height: 390 },
name: `github-${"long-server-name-".repeat(30)}`,
},
]) {
test(`MCP dialog actions remain reachable during fallback scrolling at ${viewport.width}x${viewport.height}`, async ({
page,
}, testInfo) => {
await page.setViewportSize(viewport);
mockLangGraphAPI(page);
const server = { enabled: false, command: "npx", args: ["example"] };
await page.route("**/api/mcp/config", (route) =>
route.fulfill({ json: { mcp_servers: { [name]: server } } }),
);
await page.goto("/workspace/capabilities");
for (const mode of ["edit", "add"] as const) {
await page
.getByRole("button", {
name: mode === "edit" ? `Edit ${name}` : "Add MCP plugin",
exact: true,
})
.click();
const dialog = page.getByRole("dialog");
const textbox = dialog.getByRole("textbox");
if (mode === "add") {
await textbox.fill(JSON.stringify({ mcpServers: { [name]: server } }));
// The duplicate-name validation message also contains user text.
await dialog.getByRole("button", { name: "Save", exact: true }).click();
await expect(dialog.getByRole("alert")).toBeVisible();
}
// Find the actual overflowing ancestor, so this also exercises the
// pre-fix dialog-level fallback rather than assuming a wrapper exists.
expect(
await textbox.evaluate((element) => {
for (
let parent = element.parentElement;
parent;
parent = parent.parentElement
) {
if (
/auto|scroll/.test(getComputedStyle(parent).overflowY) &&
parent.scrollHeight > parent.clientHeight
) {
parent.scrollTop = parent.scrollHeight;
return parent.scrollTop;
}
}
return 0;
}),
).toBeGreaterThan(0);
const expectActionsVisible = async () => {
for (const element of [
dialog,
dialog.getByRole("heading"),
dialog.getByRole("button", { name: "Close", exact: true }),
dialog.getByRole("button", { name: "Save", exact: true }),
dialog.getByRole("button", { name: "Cancel", exact: true }),
]) {
await expect(element).toBeInViewport({ ratio: 1 });
}
};
await expectActionsVisible();
await textbox.fill("{invalid");
await dialog.getByRole("button", { name: "Save", exact: true }).click();
await dialog.getByRole("alert").scrollIntoViewIfNeeded();
await expectActionsVisible();
await page.screenshot({
path: testInfo.outputPath(`${mode}-fallback.png`),
});
await dialog
.getByRole("button", {
name: mode === "edit" ? "Close" : "Cancel",
exact: true,
})
.click();
await expect(dialog).toBeHidden();
}
});
}
for (const viewport of [
{ width: 1280, height: 720 },
{ width: 390, height: 667 },
{ width: 844, height: 390 },
]) {
test(`long MCP definitions stay usable at ${viewport.width}x${viewport.height}`, async ({
page,
}, testInfo) => {
const errors: string[] = [];
page.on("pageerror", (error) => errors.push(error.message));
await page.setViewportSize(viewport);
mockLangGraphAPI(page);
const server = {
enabled: false,
description: "GitHub MCP server for repository operations",
type: "stdio",
command: "npx",
args: ["-y", "@modelcontextprotocol/server-github"],
env: Object.fromEntries(
Array.from({ length: 60 }, (_, index) => [`TEST_${index}`, "example"]),
),
};
await page.route("**/api/mcp/config", (route) =>
route.fulfill({ json: { mcp_servers: { github: server } } }),
);
await page.goto("/workspace/capabilities");
for (const mode of ["edit", "add"] as const) {
await page
.getByRole("button", {
name: mode === "edit" ? "Edit github" : "Add MCP plugin",
exact: true,
})
.click();
const dialog = page.getByRole("dialog");
const textbox = dialog.getByRole("textbox");
if (mode === "add") {
await textbox.fill(
JSON.stringify({ mcpServers: { github: server } }, null, 2),
);
}
const expectWithinViewport = async () => {
for (const element of [
dialog,
dialog.getByRole("heading"),
textbox,
dialog.getByRole("button", { name: "Cancel", exact: true }),
dialog.getByRole("button", { name: "Save", exact: true }),
dialog.getByRole("button", { name: "Close", exact: true }),
]) {
await expect(element).toBeInViewport({ ratio: 1 });
}
};
await expectWithinViewport();
expect(
await textbox.evaluate(
(element) => element.scrollHeight > element.clientHeight,
),
).toBe(true);
await textbox.hover();
await page.mouse.wheel(0, 10000);
await expect
.poll(() => textbox.evaluate((element) => element.scrollTop))
.toBeGreaterThan(0);
await expectWithinViewport();
await page.screenshot({ path: testInfo.outputPath(`${mode}.png`) });
await textbox.fill("{invalid");
await dialog.getByRole("button", { name: "Save", exact: true }).click();
await expect(dialog.getByRole("alert")).toBeVisible();
await expectWithinViewport();
await dialog.getByRole("button", { name: "Cancel", exact: true }).click();
await expect(dialog).toBeHidden();
}
expect(errors).toEqual([]);
});
}
test.describe("MCP server settings", () => {
test("edits one server without dropping advanced fields or siblings", async ({
page,