diff --git a/client/e2e/forms-public.unauth.spec.ts b/client/e2e/forms-public.unauth.spec.ts index c9e411322..a21d3f8fa 100644 --- a/client/e2e/forms-public.unauth.spec.ts +++ b/client/e2e/forms-public.unauth.spec.ts @@ -6,6 +6,7 @@ import { expect, request as playwrightRequest, type APIRequestContext, + type Locator, } from "@playwright/test"; const API_URL = process.env.TEST_API_URL || "http://api:8000"; @@ -33,8 +34,20 @@ async def ${PROVIDER_FN}(): const SUBMIT_SOURCE = `from bifrost import workflow @workflow(name="${SUBMIT_FN}") -async def ${SUBMIT_FN}(company: str, email: str, company_name: str = ""): - return {"company": company, "email": email, "company_name": company_name} +async def ${SUBMIT_FN}( + company: str, + email: str, + company_name: str = "", + company_size: str = "", + referral_source: str = "", +): + return { + "company": company, + "email": email, + "company_name": company_name, + "company_size": company_size, + "referral_source": referral_source, + } `; async function expectOk( @@ -47,6 +60,8 @@ test.describe.serial("Public form iframe", () => { let api: APIRequestContext; let formId: string; let publicKey: string; + let scrollFormId: string; + let scrollPublicKey: string; test.beforeAll(async () => { const credentials = JSON.parse( @@ -123,10 +138,82 @@ test.describe.serial("Public form iframe", () => { }); await expectOk(created); formId = ((await created.json()) as { id: string }).id; + + const scrollForm = await api.post("/api/forms", { + data: { + name: `Embedded scroll regression ${UNIQUE}`, + workflow_id: workflow!.id, + form_schema: { + fields: [ + { + name: "company", + label: "Company", + type: "select", + required: true, + data_provider_id: provider!.id, + auto_fill: { company_name: "company_name" }, + }, + { + name: "company_name", + label: "Company name", + type: "text", + }, + { + name: "company_size", + label: "Company size", + type: "select", + options: [ + { value: "small", label: "1–10 people" }, + { value: "medium", label: "11–100 people" }, + { value: "large", label: "101+ people" }, + ], + }, + { + name: "email", + label: "Email", + type: "email", + required: true, + }, + { + name: "referral_source", + label: "How did you hear about us", + type: "select", + options: [ + { value: "search", label: "Search engine" }, + { value: "referral", label: "Referral" }, + { value: "event", label: "Event" }, + ], + }, + ], + }, + }, + }); + await expectOk(scrollForm); + scrollFormId = ((await scrollForm.json()) as { id: string }).id; + + const scrollReview = await api.get( + `/api/forms/${scrollFormId}/publication-review`, + ); + await expectOk(scrollReview); + const review = (await scrollReview.json()) as { fingerprint: string }; + const scrollPublication = await api.put( + `/api/forms/${scrollFormId}/publication`, + { + data: { + reviewed_fingerprint: review.fingerprint, + allowed_origins: ["http://allowed-origin"], + }, + }, + ); + await expectOk(scrollPublication); + scrollPublicKey = ( + (await scrollPublication.json()) as { public_key: string } + ).public_key; }); test.afterAll(async () => { if (!api) return; + if (scrollFormId) await api.delete(`/api/forms/${scrollFormId}`); if (formId) await api.delete(`/api/forms/${formId}`); await api.delete( `/api/files/editor?path=${encodeURIComponent(PROVIDER_PATH)}`, @@ -283,7 +370,9 @@ test.describe.serial("Public form iframe", () => { name: `Public website form ${UNIQUE}`, }), ).toBeVisible({ timeout: 15_000 }); - await frame.getByRole("combobox", { name: "Company" }).click(); + await frame + .getByRole("combobox", { name: "Company *", exact: true }) + .click(); await frame.getByText("Acme Corporation", { exact: true }).click(); await expect(frame.getByLabel("Company name")).toHaveValue( "Acme Corporation", @@ -335,6 +424,80 @@ test.describe.serial("Public form iframe", () => { await expectOk(hmacSecret); }); + test("keeps the host page fixed while opening consecutive form dropdowns", async ({ + page, + }) => { + await page.route("http://allowed-origin/**", (route) => + route.request().resourceType() === "script" + ? route.abort() + : route.continue(), + ); + await page.goto("http://allowed-origin/"); + await page.setContent(` +
+ + `); + + const frame = page.frameLocator('iframe[title="Bottom form"]'); + await expect( + frame.getByRole("heading", { + name: `Embedded scroll regression ${UNIQUE}`, + }), + ).toBeVisible({ timeout: 15_000 }); + await page.evaluate(() => + window.scrollTo(0, document.body.scrollHeight), + ); + const bottom = await page.evaluate(() => window.scrollY); + const clickVisibleControl = async (control: Locator) => { + const bounds = await control.boundingBox(); + expect(bounds).not.toBeNull(); + expect(bounds!.y).toBeGreaterThanOrEqual(0); + expect(bounds!.y + bounds!.height).toBeLessThanOrEqual( + page.viewportSize()!.height, + ); + await page.mouse.click( + bounds!.x + bounds!.width / 2, + bounds!.y + bounds!.height / 2, + ); + }; + const expectParentToRemainFixed = async () => { + const positions = await page.evaluate(async () => { + const samples: number[] = []; + for (let frame = 0; frame < 5; frame += 1) { + await new Promise((resolve) => + requestAnimationFrame(() => resolve()), + ); + samples.push(window.scrollY); + } + return samples; + }); + expect(positions).toEqual(Array(5).fill(bottom)); + }; + + await clickVisibleControl( + frame.getByRole("combobox", { name: "Company size" }), + ); + await expect( + frame.getByRole("option", { name: "1–10 people" }), + ).toBeVisible(); + await expectParentToRemainFixed(); + await page.keyboard.press("Escape"); + + await clickVisibleControl( + frame.getByRole("combobox", { + name: "How did you hear about us", + }), + ); + await expect( + frame.getByRole("option", { name: "Search engine" }), + ).toBeVisible(); + await expectParentToRemainFixed(); + }); + test("shows only the signed session's execution result after an HMAC submission", async ({ page, }) => { @@ -362,7 +525,9 @@ test.describe.serial("Public form iframe", () => { name: `Public website form ${UNIQUE}`, }), ).toBeVisible(); - await frame.getByRole("combobox", { name: "Company" }).click(); + await frame + .getByRole("combobox", { name: "Company *", exact: true }) + .click(); await frame.getByText("Acme Corporation", { exact: true }).click(); await frame.getByLabel("Email").fill("hmac@example.com"); await expect( diff --git a/client/src/components/forms/FormConfirmation.test.tsx b/client/src/components/forms/FormConfirmation.test.tsx index e5192bd6a..80cb753c5 100644 --- a/client/src/components/forms/FormConfirmation.test.tsx +++ b/client/src/components/forms/FormConfirmation.test.tsx @@ -15,6 +15,7 @@ describe("FormConfirmation", () => { }); it("focuses and renders safe Markdown without raw HTML", () => { + const focus = vi.spyOn(HTMLElement.prototype, "focus"); render( { const status = screen.getByRole("status"); expect(status).toHaveFocus(); + expect(focus).toHaveBeenCalledWith({ preventScroll: true }); expect( screen.getByRole("heading", { name: "Thank you" }), ).toBeVisible(); diff --git a/client/src/components/forms/FormConfirmation.tsx b/client/src/components/forms/FormConfirmation.tsx index c06e079d8..2dc7c2d19 100644 --- a/client/src/components/forms/FormConfirmation.tsx +++ b/client/src/components/forms/FormConfirmation.tsx @@ -41,7 +41,7 @@ export function FormConfirmation({ formId, markdown }: FormConfirmationProps) { useEffect(() => { window.scrollTo({ top: 0, behavior: "smooth" }); - containerRef.current?.focus(); + containerRef.current?.focus({ preventScroll: true }); const targetOrigin = parentOrigin(); if (!targetOrigin || window.parent === window) return; diff --git a/client/src/components/ui/command.test.tsx b/client/src/components/ui/command.test.tsx new file mode 100644 index 000000000..deacf2011 --- /dev/null +++ b/client/src/components/ui/command.test.tsx @@ -0,0 +1,96 @@ +import { render, screen, waitFor } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; + +import { Command, CommandGroup, CommandItem, CommandList } from "./command"; + +const nativeScrollIntoView = Object.getOwnPropertyDescriptor( + HTMLElement.prototype, + "scrollIntoView", +); + +afterEach(() => { + if (nativeScrollIntoView) { + Object.defineProperty( + HTMLElement.prototype, + "scrollIntoView", + nativeScrollIntoView, + ); + } else { + Reflect.deleteProperty(HTMLElement.prototype, "scrollIntoView"); + } +}); + +describe("Command scrolling", () => { + it("keeps cmdk's selected-item scroll inside the command list", async () => { + const documentScroll = vi.fn(); + Object.defineProperty(HTMLElement.prototype, "scrollIntoView", { + configurable: true, + value: documentScroll, + }); + + render( + + + + Item one + + + , + ); + + const list = screen.getByRole("listbox"); + const item = screen.getByRole("option", { name: "Item one" }); + Object.defineProperty(list, "scrollTop", { + configurable: true, + writable: true, + value: 20, + }); + vi.spyOn(list, "getBoundingClientRect").mockReturnValue({ + top: 100, + bottom: 200, + left: 0, + right: 200, + width: 200, + height: 100, + x: 0, + y: 100, + toJSON: () => ({}), + }); + const itemRect = vi + .spyOn(item, "getBoundingClientRect") + .mockReturnValue({ + top: 210, + bottom: 240, + left: 0, + right: 200, + width: 200, + height: 30, + x: 0, + y: 210, + toJSON: () => ({}), + }); + + item.scrollIntoView({ block: "nearest" }); + + expect(list.scrollTop).toBe(60); + + itemRect.mockReturnValue({ + top: 70, + bottom: 100, + left: 0, + right: 200, + width: 200, + height: 30, + x: 0, + y: 70, + toJSON: () => ({}), + }); + item.scrollIntoView({ block: "nearest" }); + + expect(list.scrollTop).toBe(30); + await waitFor(() => + expect(item).toHaveAttribute("aria-selected", "true"), + ); + expect(documentScroll).not.toHaveBeenCalled(); + }); +}); diff --git a/client/src/components/ui/command.tsx b/client/src/components/ui/command.tsx index 0547a1466..2eb6fa3b1 100644 --- a/client/src/components/ui/command.tsx +++ b/client/src/components/ui/command.tsx @@ -17,6 +17,46 @@ import { } from "@/components/ui/input-group" import { SearchIcon, CheckIcon } from "lucide-react" +function scrollWithinCommandList(item: HTMLElement) { + const list = item.closest('[cmdk-list=""]') + if (!list) return + + const listRect = list.getBoundingClientRect() + const itemRect = item.getBoundingClientRect() + const styles = window.getComputedStyle(list) + const scrollPaddingTop = Number.parseFloat(styles.scrollPaddingTop) || 0 + const scrollPaddingBottom = Number.parseFloat(styles.scrollPaddingBottom) || 0 + const visibleTop = listRect.top + scrollPaddingTop + const visibleBottom = listRect.bottom - scrollPaddingBottom + + if (itemRect.top < visibleTop) { + list.scrollTop -= visibleTop - itemRect.top + } else if (itemRect.bottom > visibleBottom) { + list.scrollTop += itemRect.bottom - visibleBottom + } +} + +function setCommandItemRef( + ref: React.Ref | undefined, + item: HTMLDivElement | null +) { + if (item) { + // cmdk calls scrollIntoView when its selected item changes. The native + // method may scroll every ancestor, including a cross-origin iframe's host + // page. Keep that internal selection bookkeeping inside the command list. + Object.defineProperty(item, "scrollIntoView", { + configurable: true, + value: () => scrollWithinCommandList(item), + }) + } + + if (typeof ref === "function") { + ref(item) + } else if (ref) { + ref.current = item + } +} + function Command({ className, ...props @@ -192,10 +232,17 @@ function CommandSeparator({ function CommandItem({ className, children, + ref, ...props }: React.ComponentProps) { + const commandItemRef = React.useCallback( + (item: HTMLDivElement | null) => setCommandItemRef(ref, item), + [ref] + ) + return (