refactor(ui): type and de-mutate the table scrolling spec, drop CSS narration

DataTable loses the comment that narrated its sticky header classes. The
table scrolling e2e spec now types every management API response it
reads, seeds rows through an immutable reduce instead of pushing into
arrays, and deletes what it seeded in each test's finally block instead
of draining a shared mutable list in afterEach.

Refs LIT-4738

Claude-Session: https://claude.ai/code/session_018yW93iDaEMhoQUXcYjus7D
This commit is contained in:
ryan-crabbe-berri 2026-09-03 17:25:26 -07:00
parent dc9f40c11f
commit ff97e71652
3 changed files with 79 additions and 107 deletions

View file

@ -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 <main> 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<string, string> => ({
Authorization: `Bearer ${masterKey()}`,
});
interface CreatedTeam {
team_id: string;
}
/** Keys and Teams render a <main> 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<string, string> => ({ 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<BoxMetrics> =>
locator.evaluate((el) => {
const rect = el.getBoundingClientRect();
@ -58,24 +56,17 @@ const metrics = (locator: Locator): Promise<BoxMetrics> =>
};
});
async function postOk(
request: APIRequestContext,
path: string,
data: Record<string, unknown>,
): Promise<Record<string, any>> {
async function postOk<T>(request: APIRequestContext, path: string, data: Record<string, unknown>): Promise<T> {
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<string, any>;
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<T>(count: number, call: (index: number) => Promise<T>): Promise<T[]> {
const results: T[] = [];
for (let i = 0; i < count; i++) {
results.push(await call(i));
}
return results;
}
const oneAtATime = <T>(count: number, call: (index: number) => Promise<T>): Promise<readonly T[]> =>
Array.from({ length: count }, (_, i) => i).reduce<Promise<readonly T[]>>(
async (previous, i) => [...(await previous), await call(i)],
Promise.resolve([]),
);
async function expectRowsAtLeast(page: PlaywrightPage, count: number): Promise<void> {
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<void> {
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<void>
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<string[]> =>
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<unknown>;
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<GeneratedKey>(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<CreatedTeam>(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<unknown>(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<CreatedModel>(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 } }),
);
}
});
});

View file

@ -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");
});

View file

@ -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",