diff --git a/tests/e2e/ui/tests/tables/tableScrolling.spec.ts b/tests/e2e/ui/tests/tables/tableScrolling.spec.ts index f51c772185c..5c7438cfd11 100644 --- a/tests/e2e/ui/tests/tables/tableScrolling.spec.ts +++ b/tests/e2e/ui/tests/tables/tableScrolling.spec.ts @@ -4,37 +4,23 @@ import { Page } from "../../fixtures/pages"; import { navigateToPage } from "../../helpers/navigation"; import { CHAT_MODEL_A, masterKey, sendChatCompletion, waitForSpendLog } from "../../helpers/traffic"; -/** - * LIT-4738 table scrolling. On the paginated pages the app shell
is the only page scroller - * and must never overflow: rows scroll inside the table body under a header that stays put, and the - * pagination footer sits at the bottom of the page instead of below the fold or inside a clipped - * box. Pages that keep plain page scrolling must never paint rows past a fixed-height ancestor. - * The viewport is pinned so "more rows than fit" means the same thing on every machine. - */ - const VIEWPORT = { width: 1280, height: 720 }; const SEED_ROWS = 40; const LOG_ROWS = 20; const BODY_SCROLL_PX = 500; -/** p-8 on Keys and Teams, p-6 on Logs: the footer may sit at most one page padding above the edge. */ const MAX_FOOTER_GAP_PX = 40; -const uniqueSuffix = (): string => `${Date.now()}-${Math.random().toString(36).slice(2, 8)}`; +interface GeneratedKey { + key: string; +} -const adminHeaders = (): Record => ({ - Authorization: `Bearer ${masterKey()}`, -}); +interface CreatedTeam { + team_id: string; +} -/** Keys and Teams render a
of their own inside the app shell's, which comes first in document order. */ -const pageScroller = (page: PlaywrightPage): Locator => page.locator("main").first(); - -/** Tabs keep every panel mounted, so a bare test id can match a hidden table; scope to the visible one. */ -const visibleTestId = (page: PlaywrightPage, id: string): Locator => page.getByTestId(id).filter({ visible: true }); - -/** The page's data table; Model Hub also renders a plain links table above it, which this skips. */ -const visibleDataTable = (page: PlaywrightPage): Locator => visibleTestId(page, "data-table-root").first(); - -const visibleRows = (page: PlaywrightPage): Locator => visibleDataTable(page).locator("tbody tr"); +interface CreatedModel { + model_info: { id: string }; +} interface BoxMetrics { top: number; @@ -45,6 +31,18 @@ interface BoxMetrics { clientWidth: number; } +const uniqueSuffix = (): string => `${Date.now()}-${Math.random().toString(36).slice(2, 8)}`; + +const adminHeaders = (): Record => ({ Authorization: `Bearer ${masterKey()}` }); + +const appShellMain = (page: PlaywrightPage): Locator => page.locator("main").first(); + +const visibleTestId = (page: PlaywrightPage, id: string): Locator => page.getByTestId(id).filter({ visible: true }); + +const visibleDataTable = (page: PlaywrightPage): Locator => visibleTestId(page, "data-table-root").first(); + +const visibleRows = (page: PlaywrightPage): Locator => visibleDataTable(page).locator("tbody tr"); + const metrics = (locator: Locator): Promise => locator.evaluate((el) => { const rect = el.getBoundingClientRect(); @@ -58,24 +56,17 @@ const metrics = (locator: Locator): Promise => }; }); -async function postOk( - request: APIRequestContext, - path: string, - data: Record, -): Promise> { +async function postOk(request: APIRequestContext, path: string, data: Record): Promise { const res = await request.post(path, { headers: adminHeaders(), data }); expect(res.ok(), `POST ${path} failed (${res.status()}): ${await res.text()}`).toBe(true); - return (await res.json()) as Record; + return (await res.json()) as T; } -/** One request at a time: a burst of forty management calls starves the proxy's transaction pool. */ -async function oneAtATime(count: number, call: (index: number) => Promise): Promise { - const results: T[] = []; - for (let i = 0; i < count; i++) { - results.push(await call(i)); - } - return results; -} +const oneAtATime = (count: number, call: (index: number) => Promise): Promise => + Array.from({ length: count }, (_, i) => i).reduce>( + async (previous, i) => [...(await previous), await call(i)], + Promise.resolve([]), + ); async function expectRowsAtLeast(page: PlaywrightPage, count: number): Promise { await expect.poll(() => visibleRows(page).count(), { timeout: 30_000 }).toBeGreaterThanOrEqual(count); @@ -86,12 +77,8 @@ async function setRowsPerPage(page: PlaywrightPage, size: "25" | "50" | "100"): await page.getByRole("option", { name: size, exact: true }).click(); } -/** - * The page scroller stays put, the table body is what scrolls, the header does not move while the - * body scrolls, and the pagination footer sits at the bottom of the page. - */ async function expectBodyIsTheOnlyScroller(page: PlaywrightPage): Promise { - const scroller = await metrics(pageScroller(page)); + const scroller = await metrics(appShellMain(page)); const body = visibleTestId(page, "data-table-scroller"); const bodyBefore = await metrics(body); const headBefore = await metrics(visibleTestId(page, "data-table-head")); @@ -115,73 +102,61 @@ async function expectBodyIsTheOnlyScroller(page: PlaywrightPage): Promise expect(Math.round(headAfter.top), "header must stay put while the body scrolls").toBe(Math.round(headBefore.top)); } -/** - * Every row must sit inside each ancestor up to the nearest one that really scrolls vertically; a - * fixed-height box that neither grows nor scrolls lets rows paint past its bottom edge. - */ const rowsPaintingPastAnAncestor = (page: PlaywrightPage): Promise => visibleDataTable(page) .locator("table") .evaluate((table) => { const scrollsVertically = (el: Element): boolean => /auto|scroll/.test(getComputedStyle(el).overflowY) && el.scrollHeight > el.clientHeight + 1; + const boxesUpToTheScroller = (el: Element | null): Element[] => + el === null || el === document.body || scrollsVertically(el) + ? [] + : [el, ...boxesUpToTheScroller(el.parentElement)]; const describe = (el: Element): string => `<${el.tagName.toLowerCase()} class="${el.getAttribute("class") ?? ""}">`; return Array.from(table.querySelectorAll("tbody tr")).flatMap((row, index) => { const rowBottom = row.getBoundingClientRect().bottom; - const spills: string[] = []; - for (let el = row.parentElement; el && el !== document.body && !scrollsVertically(el); el = el.parentElement) { - const bottom = el.getBoundingClientRect().bottom; - if (rowBottom > bottom + 1) { - spills.push( - `row ${index} bottom ${Math.round(rowBottom)} past ${describe(el)} bottom ${Math.round(bottom)}`, - ); - } - } - return spills; + return boxesUpToTheScroller(row.parentElement) + .filter((box) => rowBottom > box.getBoundingClientRect().bottom + 1) + .map( + (box) => + `row ${index} bottom ${Math.round(rowBottom)} past ${describe(box)} bottom ${Math.round(box.getBoundingClientRect().bottom)}`, + ); }); }); -type Cleanup = (request: APIRequestContext) => Promise; -const cleanups: Cleanup[] = []; - test.describe("Admin tables scroll inside the page", () => { test.use({ storageState: ADMIN_STORAGE_PATH, viewport: VIEWPORT }); - test.afterEach(async ({ request }) => { - for (const cleanup of cleanups.splice(0)) { - // Teardown must never turn a passing test red or mask a real failure. - await cleanup(request).catch(() => {}); - } - }); - test("Virtual Keys: rows scroll under a sticky header and the page itself never scrolls", async ({ page, request, }) => { const suffix = uniqueSuffix(); - const created = await oneAtATime(SEED_ROWS, (i) => - postOk(request, "/key/generate", { key_alias: `e2e-scroll-key-${suffix}-${i}` }), + const keys = await oneAtATime(SEED_ROWS, (i) => + postOk(request, "/key/generate", { key_alias: `e2e-scroll-key-${suffix}-${i}` }), ); - cleanups.push((r) => r.post("/key/delete", { headers: adminHeaders(), data: { keys: created.map((k) => k.key) } })); - - await navigateToPage(page, Page.ApiKeys); - await expectRowsAtLeast(page, SEED_ROWS); - await expectBodyIsTheOnlyScroller(page); + try { + await navigateToPage(page, Page.ApiKeys); + await expectRowsAtLeast(page, SEED_ROWS); + await expectBodyIsTheOnlyScroller(page); + } finally { + await request.post("/key/delete", { headers: adminHeaders(), data: { keys: keys.map((k) => k.key) } }); + } }); test("Teams: rows scroll under a sticky header and the page itself never scrolls", async ({ page, request }) => { const suffix = uniqueSuffix(); - const created = await oneAtATime(SEED_ROWS, (i) => - postOk(request, "/team/new", { team_alias: `e2e-scroll-team-${suffix}-${i}` }), + const teams = await oneAtATime(SEED_ROWS, (i) => + postOk(request, "/team/new", { team_alias: `e2e-scroll-team-${suffix}-${i}` }), ); - cleanups.push((r) => - r.post("/team/delete", { headers: adminHeaders(), data: { team_ids: created.map((t) => t.team_id) } }), - ); - - await navigateToPage(page, Page.Teams); - await expectRowsAtLeast(page, SEED_ROWS); - await expectBodyIsTheOnlyScroller(page); + try { + await navigateToPage(page, Page.Teams); + await expectRowsAtLeast(page, SEED_ROWS); + await expectBodyIsTheOnlyScroller(page); + } finally { + await request.post("/team/delete", { headers: adminHeaders(), data: { team_ids: teams.map((t) => t.team_id) } }); + } }); test("Request Logs: rows scroll under a sticky header and the page itself never scrolls", async ({ @@ -204,20 +179,24 @@ test.describe("Admin tables scroll inside the page", () => { test("Tags: no row paints past the box it lives in", async ({ page, request }) => { const suffix = uniqueSuffix(); const names = Array.from({ length: SEED_ROWS }, (_, i) => `e2e-scroll-tag-${suffix}-${i}`); - await oneAtATime(SEED_ROWS, (i) => postOk(request, "/tag/new", { name: names[i], description: "LIT-4738 scroll" })); - cleanups.push((r) => - oneAtATime(SEED_ROWS, (i) => r.post("/tag/delete", { headers: adminHeaders(), data: { name: names[i] } })), + await oneAtATime(SEED_ROWS, (i) => + postOk(request, "/tag/new", { name: names[i], description: "LIT-4738 scroll" }), ); - - await navigateToPage(page, Page.TagManagement); - await expectRowsAtLeast(page, SEED_ROWS); - expect(await rowsPaintingPastAnAncestor(page)).toEqual([]); + try { + await navigateToPage(page, Page.TagManagement); + await expectRowsAtLeast(page, SEED_ROWS); + expect(await rowsPaintingPastAnAncestor(page)).toEqual([]); + } finally { + await oneAtATime(SEED_ROWS, (i) => + request.post("/tag/delete", { headers: adminHeaders(), data: { name: names[i] } }), + ); + } }); test("Model Hub: no row paints past the box it lives in", async ({ page, request }) => { const suffix = uniqueSuffix(); - const created = await oneAtATime(SEED_ROWS, (i) => - postOk(request, "/model/new", { + const models = await oneAtATime(SEED_ROWS, (i) => + postOk(request, "/model/new", { model_name: `e2e-scroll-model-${suffix}-${i}`, litellm_params: { model: "openai/fake-gpt-4", @@ -226,14 +205,14 @@ test.describe("Admin tables scroll inside the page", () => { }, }), ); - cleanups.push((r) => - oneAtATime(SEED_ROWS, (i) => - r.post("/model/delete", { headers: adminHeaders(), data: { id: created[i].model_info.id } }), - ), - ); - - await navigateToPage(page, Page.ModelHubTable); - await expectRowsAtLeast(page, SEED_ROWS); - expect(await rowsPaintingPastAnAncestor(page)).toEqual([]); + try { + await navigateToPage(page, Page.ModelHubTable); + await expectRowsAtLeast(page, SEED_ROWS); + expect(await rowsPaintingPastAnAncestor(page)).toEqual([]); + } finally { + await oneAtATime(SEED_ROWS, (i) => + request.post("/model/delete", { headers: adminHeaders(), data: { id: models[i].model_info.id } }), + ); + } }); }); diff --git a/ui/litellm-dashboard/src/components/shared/DataTable/DataTable.test.tsx b/ui/litellm-dashboard/src/components/shared/DataTable/DataTable.test.tsx index daf20d927e7..149554a3ac3 100644 --- a/ui/litellm-dashboard/src/components/shared/DataTable/DataTable.test.tsx +++ b/ui/litellm-dashboard/src/components/shared/DataTable/DataTable.test.tsx @@ -616,7 +616,6 @@ describe("DataTable layout", () => { const scroller = screen.getByTestId("data-table-scroller"); expect(scroller).toHaveStyle({ maxHeight: "240px" }); expect(scroller).toHaveClass("overflow-auto"); - // As in fill mode: the Table primitive's own overflow container would otherwise capture the sticky header. expect(scroller).toHaveClass("[&_[data-slot=table-container]]:overflow-visible"); expect(screen.getByTestId("data-table-head")).toHaveClass("sticky", "bg-background"); }); diff --git a/ui/litellm-dashboard/src/components/shared/DataTable/DataTable.tsx b/ui/litellm-dashboard/src/components/shared/DataTable/DataTable.tsx index f04430c1698..17a5fe42d1c 100644 --- a/ui/litellm-dashboard/src/components/shared/DataTable/DataTable.tsx +++ b/ui/litellm-dashboard/src/components/shared/DataTable/DataTable.tsx @@ -69,12 +69,6 @@ const FILL_CLASSES = { const NO_FILL_CLASSES = { outer: "", frame: "", body: "" } as const; -/** - * Sticky header, in both fill and maxBodyHeight mode. `table-container` is the Table primitive's own - * overflow-x wrapper; left as a scroll box it captures the sticky header and the header scrolls away - * with the rows. And rows pass under that header, which the semi-transparent header row tint alone - * would not hide. - */ const STICKY_CLASSES = { body: "[&_[data-slot=table-container]]:overflow-visible", header: "bg-background",