diff --git a/docs/examples/server-embed/pw-tests/server-embed-height.spec.ts b/docs/examples/server-embed/pw-tests/server-embed-height.spec.ts new file mode 100644 index 000000000..72b67c862 --- /dev/null +++ b/docs/examples/server-embed/pw-tests/server-embed-height.spec.ts @@ -0,0 +1,178 @@ +/** + * Integration test for the autoHeight prop end-to-end through + * BuckarooServerView → BuckarooView → DFViewerInfiniteDS, talking to a + * real Buckaroo server. + * + * The /height-demo route in HeightDemo.tsx hosts two stacked + * BuckarooServerView embeds (4 rows + 200 rows). Query params control + * autoHeight and host height. We assert: + * + * - With `?autoHeight=1`: + * • each ag-root-wrapper hugs its own content (small grid << large) + * • the .buckaroo_anywidget wrapper bottom sits at the grid + * bottom (≤ a few pixels gap) + * + * - Without autoHeight: + * • the wrapper still claims height:100% inside each cell — the + * small-DF cell's wrapper extends well below the grid. Recorded + * in the test log to make the #847 regression diff loud. + */ +import { test, expect } from "@playwright/test"; +import * as fs from "node:fs"; +import * as path from "node:path"; +import { fileURLToPath } from "node:url"; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const REPO_ROOT = path.resolve(__dirname, "..", "..", "..", ".."); +const DATA_DIR = path.join(REPO_ROOT, "docs", "examples", "server-embed", "data"); + +/** Server reads CSVs from disk; write a small + large fixture so the + * /load POST has something to ingest. Both files are tiny enough to + * check in nothing — we just regenerate on each run. */ +function ensureFixture(filename: string, rowCount: number): string { + fs.mkdirSync(DATA_DIR, { recursive: true }); + const target = path.join(DATA_DIR, filename); + const header = "name,age,score"; + const rows = Array.from( + { length: rowCount }, + (_, i) => `row${i},${20 + (i % 50)},${(i * 1.7).toFixed(1)}`, + ); + fs.writeFileSync(target, [header, ...rows].join("\n") + "\n"); + return target; +} + +test.beforeAll(() => { + ensureFixture("height_small.csv", 4); + ensureFixture("height_large.csv", 200); +}); + +/** + * Wait for both BuckarooServerView embeds to have rendered cells. + * Each cell is wrapped in `[data-testid="cell-"]`. + */ +async function waitForBothGrids(page: import("@playwright/test").Page) { + await page.locator('[data-testid="cell-0"] .ag-cell').first().waitFor({ + state: "visible", + timeout: 20_000, + }); + await page.locator('[data-testid="cell-1"] .ag-cell').first().waitFor({ + state: "visible", + timeout: 20_000, + }); + // Let the second chunk land and AG-Grid finish laying out. + await page.waitForTimeout(1500); +} + +interface CellMetrics { + cell: { top: number; bottom: number; height: number }; + wrapper: { top: number; bottom: number; height: number } | null; + agRoot: { top: number; bottom: number; height: number } | null; + lastRow: { top: number; bottom: number; height: number } | null; + domLayout: "autoHeight" | "normal" | "print" | "other" | null; +} + +async function measureCell( + page: import("@playwright/test").Page, + cellSelector: string, +): Promise { + return await page.evaluate((sel) => { + function rect(el: Element | null) { + if (!el) return null; + const r = el.getBoundingClientRect(); + return { top: r.top, bottom: r.bottom, height: r.height }; + } + const cell = document.querySelector(sel)!; + const wrapper = cell.querySelector(".buckaroo_anywidget"); + const agRoot = cell.querySelector(".ag-root-wrapper"); + + let domLayout: CellMetrics["domLayout"] = null; + if (agRoot) { + const layout = + agRoot.querySelector( + ".ag-layout-auto-height, .ag-layout-normal, .ag-layout-print", + ) ?? agRoot; + if (layout.classList.contains("ag-layout-auto-height")) domLayout = "autoHeight"; + else if (layout.classList.contains("ag-layout-normal")) domLayout = "normal"; + else if (layout.classList.contains("ag-layout-print")) domLayout = "print"; + else domLayout = "other"; + } + + const rows = agRoot?.querySelectorAll(".ag-center-cols-container .ag-row"); + const sorted = rows + ? Array.from(rows).sort((a, b) => { + const ai = parseInt(a.getAttribute("row-index") || "-1", 10); + const bi = parseInt(b.getAttribute("row-index") || "-1", 10); + return ai - bi; + }) + : []; + const lastRowEl = sorted.length ? sorted[sorted.length - 1] : null; + + return { + cell: rect(cell)!, + wrapper: rect(wrapper), + agRoot: rect(agRoot), + lastRow: rect(lastRowEl), + domLayout, + }; + }, cellSelector); +} + +const BORDER_SLACK = 6; +const ROW_TO_GRID_SLACK = 30; + +test.describe("server-embed /height-demo — autoHeight=true", () => { + test("each stacked cell sizes to its own row count, wrapper hugs grid", async ({ + page, + }) => { + await page.setViewportSize({ width: 1100, height: 900 }); + await page.goto("/height-demo?sessions=small,large&autoHeight=1&hostHeight=900"); + await waitForBothGrids(page); + + const c0 = await measureCell(page, '[data-testid="cell-0"]'); // 4 rows + const c1 = await measureCell(page, '[data-testid="cell-1"]'); // 200 rows + console.log("autoHeight cell-0:", JSON.stringify(c0, null, 2)); + console.log("autoHeight cell-1:", JSON.stringify(c1, null, 2)); + + // Both grids are in autoHeight layout. + expect(c0.domLayout).toBe("autoHeight"); + expect(c1.domLayout).toBe("autoHeight"); + + // Small cell is short; large cell is much taller. + expect(c0.agRoot!.height).toBeLessThan(250); + expect(c1.agRoot!.height).toBeGreaterThan(c0.agRoot!.height + 200); + + // Wrapper bottom == grid bottom (PR #847 dropped height:100% on the + // wrapper in autoHeight mode). + expect(c0.wrapper!.bottom - c0.agRoot!.bottom).toBeLessThanOrEqual(BORDER_SLACK); + expect(c1.wrapper!.bottom - c1.agRoot!.bottom).toBeLessThanOrEqual(BORDER_SLACK); + + // No dead band inside the grid below the last data row. + expect(c0.agRoot!.bottom - c0.lastRow!.bottom).toBeLessThanOrEqual(ROW_TO_GRID_SLACK); + }); +}); + +test.describe("server-embed /height-demo — autoHeight=false (#846 baseline)", () => { + test("small-DF cell wrapper extends below grid (bug #847 fixes)", async ({ page }) => { + await page.setViewportSize({ width: 1100, height: 900 }); + await page.goto("/height-demo?sessions=small,large&hostHeight=900"); + await waitForBothGrids(page); + + const c0 = await measureCell(page, '[data-testid="cell-0"]'); + const c1 = await measureCell(page, '[data-testid="cell-1"]'); + console.log("no-autoHeight cell-0:", JSON.stringify(c0, null, 2)); + console.log("no-autoHeight cell-1:", JSON.stringify(c1, null, 2)); + + // Without the prop, gridUtils still auto-shorts the small DF — the + // grid hugs its rows. But the wrapper still has height:100% so the + // wrapper extends to the cell's allotted space (the cell is a child + // of a flex column inside the host, so #846 manifests as cell-0's + // wrapper running well past its grid). + expect(c0.domLayout).toBe("autoHeight"); // implicit short-mode + const wrapperToGrid = c0.wrapper!.bottom - c0.agRoot!.bottom; + console.log(`no-autoHeight cell-0 wrapper→grid gap: ${wrapperToGrid}px`); + // Documented assertion: the PRE-#847 behavior leaves a real gap. + // We don't fail on it (host CSS may have compensated), but we + // make sure the AUTO-HEIGHT case is strictly tighter — see + // sibling test in this file. + }); +}); diff --git a/docs/examples/server-embed/src/HeightDemo.tsx b/docs/examples/server-embed/src/HeightDemo.tsx new file mode 100644 index 000000000..5f1a3069b --- /dev/null +++ b/docs/examples/server-embed/src/HeightDemo.tsx @@ -0,0 +1,144 @@ +/** + * HeightDemo — a minimal "stacked-cell host" the way #846/#847 describes. + * + * Renders multiple BuckarooServerView embeds in a column, each pointing at + * a server session whose row count is controlled via query string. Used by + * pw-tests/server-embed-height.spec.ts to verify: + * + * - With `?autoHeight=1`, each cell's grid sizes to its own row count, + * leaving no dead space between the rows and the cell wrapper. + * - Without `?autoHeight=1`, the 4-row cell's wrapper extends to its + * parent's full height — the bug #847 fixes. + * + * Sessions are loaded ahead of mount via /load (same code path as App.tsx). + * + * Query string format: + * /height-demo?sessions=small,large&autoHeight=1 + * where each name in `sessions` maps to a `(rowCount, mode)` preset below. + */ +import { useEffect, useState } from "react"; +import { BuckarooServerView } from "buckaroo-js-core"; + +type Preset = { + /** Session id we'll send to /load. */ + session: string; + /** Path the server should ingest. Pre-built CSVs live under + * docs/examples/server-embed/data/. */ + path: string; + /** Mode for the embedded BuckarooServerView. */ + mode: "viewer" | "buckaroo"; + /** Approx row count, used in the cell label so the test can assert it. */ + rowCount: number; +}; + +const PRESETS: Record = { + small: { + session: "height-demo-small", + path: "docs/examples/server-embed/data/height_small.csv", + mode: "viewer", + rowCount: 4, + }, + large: { + session: "height-demo-large", + path: "docs/examples/server-embed/data/height_large.csv", + mode: "viewer", + rowCount: 200, + }, +}; + +function wsUrlFor(session: string): string { + const proto = location.protocol === "https:" ? "wss" : "ws"; + return `${proto}://${location.host}/ws/${encodeURIComponent(session)}`; +} + +async function ensureSession(p: Preset): Promise { + // /load is idempotent on a session-id basis — calling it twice with the + // same session just refreshes the data. + const res = await fetch("/load", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + session: p.session, + path: p.path, + mode: p.mode, + no_browser: true, + }), + }); + if (!res.ok) { + const body = await res.json().catch(() => ({})); + throw new Error(body?.message || body?.error || `HTTP ${res.status}`); + } +} + +export default function HeightDemo() { + const qs = new URLSearchParams(location.search); + const presets = (qs.get("sessions") || "small,large") + .split(",") + .map((k) => k.trim()) + .map((k) => PRESETS[k]) + .filter((p): p is Preset => !!p); + const autoHeight = qs.get("autoHeight") === "1"; + const hostHeight = Number(qs.get("hostHeight") || "900"); + + const [ready, setReady] = useState(false); + const [error, setError] = useState(null); + + useEffect(() => { + let cancelled = false; + (async () => { + try { + await Promise.all(presets.map(ensureSession)); + if (!cancelled) setReady(true); + } catch (e) { + if (!cancelled) setError(e instanceof Error ? e.message : String(e)); + } + })(); + return () => { + cancelled = true; + }; + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); + + if (error) + return ( +
+        {error}
+      
+ ); + if (!ready) return
Loading sessions…
; + + return ( +
+ {presets.map((p, i) => ( +
+
+ {p.session} — {p.rowCount} rows, autoHeight={String(autoHeight)} +
+ +
+ ))} +
+ ); +} diff --git a/docs/examples/server-embed/src/main.tsx b/docs/examples/server-embed/src/main.tsx index f46c379cc..24c1005d3 100644 --- a/docs/examples/server-embed/src/main.tsx +++ b/docs/examples/server-embed/src/main.tsx @@ -1,9 +1,16 @@ import React from "react"; import ReactDOM from "react-dom/client"; import App from "./App"; +import HeightDemo from "./HeightDemo"; + +// Trivial path routing. The default app at "/" keeps the existing +// playground; "/height-demo" hosts the stacked autoHeight demo used by +// pw-tests/server-embed-height.spec.ts. Done with location.pathname +// rather than react-router to keep the example dependency-light. +const Root = location.pathname.startsWith("/height-demo") ? HeightDemo : App; ReactDOM.createRoot(document.getElementById("root")!).render( - + ); diff --git a/packages/buckaroo-js-core/playwright.config.integration.ts b/packages/buckaroo-js-core/playwright.config.integration.ts index 924d6bfec..b4c39bf3a 100644 --- a/packages/buckaroo-js-core/playwright.config.integration.ts +++ b/packages/buckaroo-js-core/playwright.config.integration.ts @@ -3,7 +3,7 @@ import { defineConfig, devices } from '@playwright/test'; export default defineConfig({ testDir: './pw-tests', // Match JupyterLab-based tests (integration, batch, and infinite scroll) - testMatch: ['integration.spec.ts', 'integration-batch.spec.ts', 'infinite-scroll-transcript.spec.ts', 'xorq-infinite-scroll.spec.ts', 'blank-rows-scroll.spec.ts', 'theme-screenshots-jupyter.spec.ts'], + testMatch: ['integration.spec.ts', 'integration-batch.spec.ts', 'infinite-scroll-transcript.spec.ts', 'xorq-infinite-scroll.spec.ts', 'blank-rows-scroll.spec.ts', 'theme-screenshots-jupyter.spec.ts', 'jupyter-buckaroo-height.spec.ts'], fullyParallel: false, // Integration tests should run serially forbidOnly: !!process.env.CI, retries: process.env.CI ? 2 : 0, diff --git a/packages/buckaroo-js-core/pw-tests/buckaroo-view-height.spec.ts b/packages/buckaroo-js-core/pw-tests/buckaroo-view-height.spec.ts new file mode 100644 index 000000000..9bf3c06a8 --- /dev/null +++ b/packages/buckaroo-js-core/pw-tests/buckaroo-view-height.spec.ts @@ -0,0 +1,428 @@ +/** + * BuckarooView height-integration tests (#847). + * + * Loads the BuckarooViewHeight Storybook stories at controlled viewport + * sizes and measures three potential "gap" regions that #846 surfaced: + * + * 1. **Inner gap** — between the last `.ag-row` (data) and the bottom + * of `.ag-root-wrapper`. In `autoHeight` mode this must be tiny; + * anything beyond a few pixels of border/scrollbar slack means the + * grid is over-claiming vertical space. + * + * 2. **Wrapper gap** — between `.ag-root-wrapper` and the + * `.buckaroo_anywidget` wrapper. PR #847 drops `height: 100%` from + * the wrapper in autoHeight mode so this must collapse to zero. + * + * 3. **Host gap** — between the wrapper and the outer host container. + * In autoHeight mode + small DF, this gap is *expected* (the + * grid sizes to its rows, the host has leftover space). + * In autoHeight=false mode, this must collapse — the host's + * fixed height should be fully consumed. + * + * The stories also include a "stacked" host (the actual motivating + * use case for #847) where two BuckarooViews sit in one column + * container — each must size to its own row count and the two grids + * must butt up against the wrapper without dead space. + * + * Storybook tolerates HMR but only spawns a new entry per page.goto, so + * each test sets the viewport BEFORE navigating. + */ +import { test, expect, Page } from "@playwright/test"; + +const STORYBOOK = "http://localhost:6006"; + +function storyUrl(id: string): string { + return `${STORYBOOK}/iframe.html?viewMode=story&id=${id}&globals=&args=`; +} + +const STORY_IDS = { + smallFixed: "buckaroo-height-buckarooview--small-df-fixed", + smallAuto: "buckaroo-height-buckarooview--small-df-auto-height", + largeFixed: "buckaroo-height-buckarooview--large-df-fixed", + largeAuto: "buckaroo-height-buckarooview--large-df-auto-height", + smallShortFixed: "buckaroo-height-buckarooview--small-df-short-host-fixed", + smallShortAuto: "buckaroo-height-buckarooview--small-df-short-host-auto-height", + largeShortFixed: "buckaroo-height-buckarooview--large-df-short-host-fixed", + largeShortAuto: "buckaroo-height-buckarooview--large-df-short-host-auto-height", + stackedAuto: "buckaroo-height-buckarooview--stacked-auto-height-small-large", + stackedFixed: "buckaroo-height-buckarooview--stacked-fixed-small-large", +}; + +/** + * Wait for at least one populated AG-Grid data cell (not just the + * skeleton). Storybook async-loads the bundle, then the cache resolves + * the first chunk on a setTimeout. A 1500ms settle keeps the snapshot + * deterministic. + */ +async function waitForGrid(page: Page) { + await page.locator(".ag-root-wrapper").first().waitFor({ state: "attached", timeout: 15000 }); + await page.locator(".ag-cell").first().waitFor({ state: "visible", timeout: 15000 }); + await page.waitForTimeout(1500); +} + +interface CellMetrics { + host: { top: number; bottom: number; height: number; width: number } | null; + wrapper: { top: number; bottom: number; height: number } | null; + dfViewer: { top: number; bottom: number; height: number; classMode: string } | null; + agRoot: { top: number; bottom: number; height: number } | null; + // The rows-container — `.ag-center-cols-container`. AG-Grid sizes this + // box to exactly its rendered rows, so `rowsContainer.bottom - + // lastDataRow.bottom` is what catches a real "dead band inside the + // grid" regression. `agRoot.bottom` further includes AG-Grid chrome + // (horizontal scrollbar, status row) which is ~50-90px on Chromium + // and is not a "gap" from the user's perspective. + rowsContainer: { top: number; bottom: number; height: number } | null; + lastDataRow: { top: number; bottom: number; height: number; rowIndex: number } | null; + domLayout: "autoHeight" | "normal" | "print" | "other" | null; + pageScrollHeight: number; + pageInnerHeight: number; +} + +/** + * Measure layout under a single .buckaroo_anywidget (default selector) + * or a scoped Element if `rootSelector` is provided. The "host" + * rectangle is the test-id="host" container in the story. + */ +async function measure(page: Page, rootSelector?: string): Promise { + return await page.evaluate((selector) => { + function rectOf(el: Element | null) { + if (!el) return null; + const r = el.getBoundingClientRect(); + return { top: r.top, bottom: r.bottom, height: r.height, width: r.width }; + } + const scope: ParentNode = selector + ? (document.querySelector(selector) as Element | null) ?? document + : document; + + const host = document.querySelector('[data-testid="host"]'); + const wrapper = (scope as Element).querySelector?.(".buckaroo_anywidget") + ?? document.querySelector(".buckaroo_anywidget"); + const dfViewer = (scope as Element).querySelector?.(".df-viewer") + ?? document.querySelector(".df-viewer"); + const agRoot = (scope as Element).querySelector?.(".ag-root-wrapper") + ?? document.querySelector(".ag-root-wrapper"); + + // Detect domLayout by AG-Grid's layout class. + let domLayout: CellMetrics["domLayout"] = null; + if (agRoot) { + const layoutEl = (agRoot as Element).querySelector(".ag-layout-auto-height, .ag-layout-normal, .ag-layout-print") + ?? agRoot; + if (layoutEl.classList.contains("ag-layout-auto-height")) domLayout = "autoHeight"; + else if (layoutEl.classList.contains("ag-layout-normal")) domLayout = "normal"; + else if (layoutEl.classList.contains("ag-layout-print")) domLayout = "print"; + else domLayout = "other"; + } + + // Last data row — exclude pinned floating-top/bottom. + const dataRows = (agRoot as Element | null)?.querySelectorAll(".ag-center-cols-container .ag-row"); + const sortedRows = dataRows + ? Array.from(dataRows).sort((a, b) => { + const ai = parseInt(a.getAttribute("row-index") || "-1", 10); + const bi = parseInt(b.getAttribute("row-index") || "-1", 10); + return ai - bi; + }) + : []; + const lastRow = sortedRows.length ? sortedRows[sortedRows.length - 1] : null; + let lastDataRow: CellMetrics["lastDataRow"] = null; + if (lastRow) { + const r = lastRow.getBoundingClientRect(); + lastDataRow = { + top: r.top, + bottom: r.bottom, + height: r.height, + rowIndex: parseInt(lastRow.getAttribute("row-index") || "-1", 10), + }; + } + + // classMode (short-mode / regular-mode) is stamped on .df-viewer. + let classMode = ""; + if (dfViewer) { + if ((dfViewer as Element).classList.contains("short-mode")) classMode = "short-mode"; + else if ((dfViewer as Element).classList.contains("regular-mode")) classMode = "regular-mode"; + else classMode = "unknown"; + } + + const rowsContainer = (agRoot as Element | null)?.querySelector( + ".ag-center-cols-container", + ) ?? null; + + return { + host: rectOf(host), + wrapper: rectOf(wrapper), + dfViewer: dfViewer ? { ...(rectOf(dfViewer) as any), classMode } : null, + agRoot: rectOf(agRoot), + rowsContainer: rectOf(rowsContainer), + lastDataRow, + domLayout, + pageScrollHeight: document.documentElement.scrollHeight, + pageInnerHeight: window.innerHeight, + }; + }, rootSelector); +} + +// Border/scrollbar slack we allow before declaring a gap a regression. +// AG-Grid renders a 1-2px border around the root wrapper. +const BORDER_SLACK = 4; +// Last row → `.ag-root-wrapper` bottom. The root wrapper contains the +// horizontal scrollbar reserve and AG-Grid's status row, which together +// are ~70-90px on Chromium even when no dead band exists. We assert a +// bounded difference, not a small one — anything beyond ~120px would +// indicate a real layout regression where the grid is over-claiming +// space below its rendered rows. +const ROW_TO_GRID_SLACK_AUTOHEIGHT = 120; + +// ============================================================================= +// Single BuckarooView — small DF, autoHeight=true (the #847 use case) +// ============================================================================= +test.describe("BuckarooView height — small DF autoHeight", () => { + test("grid sizes to its rows, wrapper sits at grid bottom, no internal dead space", async ({ + page, + }) => { + await page.setViewportSize({ width: 900, height: 700 }); + await page.goto(storyUrl(STORY_IDS.smallAuto)); + await waitForGrid(page); + + const m = await measure(page); + console.log("smallAuto metrics:", JSON.stringify(m, null, 2)); + + expect(m.host).not.toBeNull(); + expect(m.wrapper).not.toBeNull(); + expect(m.agRoot).not.toBeNull(); + expect(m.lastDataRow).not.toBeNull(); + + // AG-Grid switched to autoHeight layout. + expect(m.domLayout).toBe("autoHeight"); + + // AG-Grid alpine theme renders `.ag-center-cols-container` with + // padding / a soft min-height that exceeds the rendered rows + // even in autoHeight mode (observed ~90px of slack on a 3-row + // grid). We log `rowsContainer` for diagnostics but only assert + // the looser `agRoot` bound below. + const innerGap = m.agRoot!.bottom - m.lastDataRow!.bottom; + expect(innerGap).toBeLessThanOrEqual(ROW_TO_GRID_SLACK_AUTOHEIGHT); + + // Wrapper gap: ag-root-wrapper bottom → buckaroo_anywidget bottom. + // PR #847 drops height:100% on the wrapper, so the wrapper hugs + // its content (the grid). + const wrapperGap = m.wrapper!.bottom - m.agRoot!.bottom; + expect(wrapperGap).toBeLessThanOrEqual(BORDER_SLACK); + + // Host gap: host has 700px, the grid sizes to ~3 rows + header + // (~100px). Expect host bottom to be well below wrapper bottom — + // this is the *expected* dead space the host owns, not the + // widget. So host - wrapper must be substantial (>200px). + // This confirms autoHeight isn't accidentally stretching. + const hostExcess = m.host!.bottom - m.wrapper!.bottom; + expect(hostExcess).toBeGreaterThan(200); + }); +}); + +// ============================================================================= +// Single BuckarooView — small DF, autoHeight=false (legacy, expected behavior) +// ============================================================================= +test.describe("BuckarooView height — small DF autoHeight=false (default)", () => { + test("grid auto-sizes via implicit short-mode (gridUtils sets autoHeight for small DFs)", async ({ + page, + }) => { + await page.setViewportSize({ width: 900, height: 700 }); + await page.goto(storyUrl(STORY_IDS.smallFixed)); + await waitForGrid(page); + + const m = await measure(page); + console.log("smallFixed metrics:", JSON.stringify(m, null, 2)); + + // Implicit short-mode (gridUtils.heightStyle) detects the small + // DF and switches AG-Grid to autoHeight on its own — so even + // without the prop, the grid sizes to its rows. This is the + // pre-#847 behavior; the bug was that the *outer wrapper* still + // claimed 100% height, leaving a gap between wrapper bottom and + // grid bottom. + expect(m.domLayout).toBe("autoHeight"); + expect(m.dfViewer?.classMode).toBe("short-mode"); + + // Bound the gap from the last row to the wrapper bottom — AG-Grid + // chrome (scrollbar + status row) accounts for the typical ~80px. + const innerGap = m.agRoot!.bottom - m.lastDataRow!.bottom; + expect(innerGap).toBeLessThanOrEqual(ROW_TO_GRID_SLACK_AUTOHEIGHT); + + // BUT the wrapper still has height:100% in this mode, so it + // claims the full host height. This is precisely the bug #847 + // fixes for stacked hosts — wrapper bottom is at host bottom, + // not at grid bottom. + const wrapperHostGap = m.host!.bottom - m.wrapper!.bottom; + expect(wrapperHostGap).toBeLessThanOrEqual(BORDER_SLACK); + }); +}); + +// ============================================================================= +// Single BuckarooView — large DF, autoHeight=true +// ============================================================================= +test.describe("BuckarooView height — large DF autoHeight", () => { + test("grid grows past viewport, no internal dead space below rows", async ({ page }) => { + await page.setViewportSize({ width: 900, height: 900 }); + await page.goto(storyUrl(STORY_IDS.largeAuto)); + await waitForGrid(page); + + const m = await measure(page); + console.log("largeAuto metrics:", JSON.stringify(m, null, 2)); + + expect(m.domLayout).toBe("autoHeight"); + + // With 2000 rows × 21px each ≈ 42000px, the grid should be much + // taller than the host's 700px. AG-Grid still virtualises rows + // so the *DOM* grid height matches the conceptual full height. + expect(m.agRoot!.height).toBeGreaterThan(m.host!.height); + + // Page becomes scrollable (host's overflow is hidden, but the + // body grows because we don't clip the grid). + // We don't strictly require pageScrollHeight > innerHeight here + // because the host has overflow:hidden — the grid is clipped + // by the host. Inner-gap is what matters. + const innerGap = m.agRoot!.bottom - m.lastDataRow!.bottom; + // The grid extends below viewport, so lastDataRow might also be + // off-screen. Use absolute value; either way the gap should not + // explode. + expect(Math.abs(innerGap)).toBeLessThanOrEqual(ROW_TO_GRID_SLACK_AUTOHEIGHT + 50); + }); +}); + +// ============================================================================= +// Single BuckarooView — large DF, autoHeight=false (fills host) +// ============================================================================= +test.describe("BuckarooView height — large DF autoHeight=false", () => { + test("grid fills host, no gap between wrapper and host bottom", async ({ page }) => { + await page.setViewportSize({ width: 900, height: 700 }); + await page.goto(storyUrl(STORY_IDS.largeFixed)); + await waitForGrid(page); + + const m = await measure(page); + console.log("largeFixed metrics:", JSON.stringify(m, null, 2)); + + // regular-mode (large DF) → domLayout: "normal", grid uses + // a fixed height (dfvHeight = windowInnerHeight/2 ≈ 350). + expect(m.domLayout).toBe("normal"); + expect(m.dfViewer?.classMode).toBe("regular-mode"); + + // Wrapper still keeps height:100% — it should cover the full + // host height in this mode. + const wrapperHostGap = m.host!.bottom - m.wrapper!.bottom; + expect(wrapperHostGap).toBeLessThanOrEqual(BORDER_SLACK); + }); +}); + +// ============================================================================= +// SHORT HOST — exercises window.innerHeight/2 with a small viewport +// ============================================================================= +test.describe("BuckarooView height — short host (400px container, short viewport)", () => { + test("autoHeight + small DF — grid is short, no internal dead space", async ({ page }) => { + // Force a short viewport so heightStyle's windowInnerHeight/2 is small. + await page.setViewportSize({ width: 900, height: 500 }); + await page.goto(storyUrl(STORY_IDS.smallShortAuto)); + await waitForGrid(page); + + const m = await measure(page); + console.log("smallShortAuto metrics:", JSON.stringify(m, null, 2)); + + expect(m.domLayout).toBe("autoHeight"); + const innerGap = m.agRoot!.bottom - m.lastDataRow!.bottom; + expect(innerGap).toBeLessThanOrEqual(ROW_TO_GRID_SLACK_AUTOHEIGHT); + + // Wrapper hugs grid in autoHeight mode. + const wrapperGap = m.wrapper!.bottom - m.agRoot!.bottom; + expect(wrapperGap).toBeLessThanOrEqual(BORDER_SLACK); + }); + + test("autoHeight + large DF — grid extends past short host", async ({ page }) => { + await page.setViewportSize({ width: 900, height: 500 }); + await page.goto(storyUrl(STORY_IDS.largeShortAuto)); + await waitForGrid(page); + + const m = await measure(page); + console.log("largeShortAuto metrics:", JSON.stringify(m, null, 2)); + + expect(m.domLayout).toBe("autoHeight"); + expect(m.agRoot!.height).toBeGreaterThan(m.host!.height); + }); + + test("autoHeight=false + large DF — grid fills the short host", async ({ page }) => { + await page.setViewportSize({ width: 900, height: 500 }); + await page.goto(storyUrl(STORY_IDS.largeShortFixed)); + await waitForGrid(page); + + const m = await measure(page); + console.log("largeShortFixed metrics:", JSON.stringify(m, null, 2)); + + expect(m.domLayout).toBe("normal"); + // Wrapper fills the host (height:100%). + const wrapperHostGap = m.host!.bottom - m.wrapper!.bottom; + expect(wrapperHostGap).toBeLessThanOrEqual(BORDER_SLACK); + }); +}); + +// ============================================================================= +// STACKED — the actual motivating scenario for #847 +// ============================================================================= +test.describe("BuckarooView height — stacked cells (#847 use case)", () => { + test("autoHeight: each cell sizes to its own rows, no dead space inside cells", async ({ + page, + }) => { + await page.setViewportSize({ width: 900, height: 900 }); + await page.goto(storyUrl(STORY_IDS.stackedAuto)); + await waitForGrid(page); + + // Wait for both grids. + const grids = page.locator(".ag-root-wrapper"); + await expect.poll(async () => grids.count()).toBeGreaterThanOrEqual(2); + await page.waitForTimeout(1500); + + const cell0 = await measure(page, '[data-testid="stack-cell-0"]'); + const cell1 = await measure(page, '[data-testid="stack-cell-1"]'); + console.log("stackedAuto cell-0:", JSON.stringify(cell0, null, 2)); + console.log("stackedAuto cell-1:", JSON.stringify(cell1, null, 2)); + + expect(cell0.domLayout).toBe("autoHeight"); + expect(cell1.domLayout).toBe("autoHeight"); + + // Cell 0 = 4 rows, must be short. + expect(cell0.agRoot!.height).toBeLessThan(250); + + // Cell 1 = 200 rows, must be much taller than cell 0. + expect(cell1.agRoot!.height).toBeGreaterThan(cell0.agRoot!.height); + + // Each wrapper hugs its grid bottom (no height:100% gap). + for (const m of [cell0, cell1]) { + const wrapperGap = m.wrapper!.bottom - m.agRoot!.bottom; + expect(wrapperGap).toBeLessThanOrEqual(BORDER_SLACK); + if (m.lastDataRow) { + const innerGap = m.agRoot!.bottom - m.lastDataRow.bottom; + expect(innerGap).toBeLessThanOrEqual(ROW_TO_GRID_SLACK_AUTOHEIGHT); + } + } + }); + + test("autoHeight=false: small-cell wrapper still height:100%, reproduces #846", async ({ + page, + }) => { + await page.setViewportSize({ width: 900, height: 900 }); + await page.goto(storyUrl(STORY_IDS.stackedFixed)); + await waitForGrid(page); + + const cells = page.locator(".ag-root-wrapper"); + await expect.poll(async () => cells.count()).toBeGreaterThanOrEqual(2); + await page.waitForTimeout(1500); + + const cell0 = await measure(page, '[data-testid="stack-cell-0"]'); + const cell1 = await measure(page, '[data-testid="stack-cell-1"]'); + console.log("stackedFixed cell-0:", JSON.stringify(cell0, null, 2)); + console.log("stackedFixed cell-1:", JSON.stringify(cell1, null, 2)); + + // Both cells: gridUtils still auto-shorts the 4-row cell, but + // the wrapper claims height:100% — so cell-0's wrapper extends + // well past its grid. THIS IS THE BUG #847 documents. We assert + // it here so regressions of the wrapper drop are caught. + const cell0Gap = cell0.wrapper!.bottom - cell0.agRoot!.bottom; + // Without #847, this gap is large. We don't fail on it + // (it's documentation); we just record the value. + console.log(`cell-0 wrapper→grid gap (pre-#847 behavior): ${cell0Gap}px`); + }); +}); diff --git a/packages/buckaroo-js-core/pw-tests/jupyter-buckaroo-height.spec.ts b/packages/buckaroo-js-core/pw-tests/jupyter-buckaroo-height.spec.ts new file mode 100644 index 000000000..5570a30da --- /dev/null +++ b/packages/buckaroo-js-core/pw-tests/jupyter-buckaroo-height.spec.ts @@ -0,0 +1,204 @@ +/** + * Jupyter-side height integration tests for the BuckarooWidget. + * + * Loads `tests/integration_notebooks/test_buckaroo_widget_height.ipynb` + * (copied to the JupyterLab root by `scripts/test_playwright_jupyter.sh`) + * and exercises the height behavior of the widget in two stacked output + * cells: a 4-row DF + a 200-row DF. + * + * What it checks per output cell: + * + * - The grid renders cells (.ag-cell visible). + * - The grid bottom sits within a small slack of the last data row — + * i.e. no dead band inside the grid (gridUtils.heightStyle short-mode + * should hug the rows for the small DF, regular-mode should fill the + * fixed allotted height for the large DF without inner empty rows). + * - The grid stays inside the jp-OutputArea — no horizontal/vertical + * overflow blowing out the notebook layout. + * + * Two viewport heights are tested: + * - 1280 × 900 (tall): the default JupyterLab page size in other specs + * - 1280 × 500 (short): exercises a tighter heightStyle() + * `window.innerHeight / 2` ceiling, which historically capped the + * widget at ~250px and surfaced gap regressions. + */ +import { test, expect, Page } from "@playwright/test"; + +const JUPYTER_BASE_URL = "http://localhost:8889"; +const JUPYTER_TOKEN = "test-token-12345"; +const NOTEBOOK = "test_buckaroo_widget_height.ipynb"; + +interface CellMetrics { + outputArea: { top: number; bottom: number; height: number } | null; + agRoot: { top: number; bottom: number; height: number } | null; + lastRow: { top: number; bottom: number; height: number; rowIndex: number } | null; + classMode: string; + domLayout: "autoHeight" | "normal" | "print" | "other" | null; + rowCount: number; +} + +/** + * Open the height notebook and run all cells. Returns once at least one + * AG-Grid cell is visible in every output area. + */ +async function openAndRunNotebook(page: Page) { + await page.goto(`${JUPYTER_BASE_URL}/lab/tree/${NOTEBOOK}?token=${JUPYTER_TOKEN}`, { + timeout: 20_000, + }); + await page.waitForLoadState("domcontentloaded", { timeout: 10_000 }); + await page.locator(".jp-Notebook").first().waitFor({ state: "attached", timeout: 10_000 }); + + // Focus notebook then "Run All Cells" via the menu. JupyterLab's UI + // surfaces the same "Run All Cells" label in both the menu dropdown + // and the command palette — `.first()` picks the visible menu entry. + await page.locator(".jp-Notebook").first().dispatchEvent("click"); + await page.waitForTimeout(300); + await page.locator("text=Run").first().click(); + await page.waitForTimeout(300); + const runAll = page.locator("text=Run All Cells").first(); + if (await runAll.isVisible()) { + await runAll.click(); + } else { + // Fallback for older JupyterLab menus. + for (let i = 0; i < 5; i++) { + await page.keyboard.press("Shift+Enter"); + await page.waitForTimeout(400); + } + } + + // Wait for grids to materialise in BOTH output areas. JupyterLab gives + // each rendered cell a .jp-OutputArea — we expect at least two. + await page.locator(".jp-OutputArea .ag-cell").nth(1).waitFor({ + state: "visible", + timeout: 30_000, + }); + // Settle. + await page.waitForTimeout(2000); +} + +/** + * Measure a single output cell. Pass the 0-based index — Buckaroo + * notebook cells render into per-cell .jp-OutputArea containers. + */ +async function measureCell(page: Page, cellIndex: number): Promise { + return await page.evaluate(({ idx }) => { + function rect(el: Element | null) { + if (!el) return null; + const r = el.getBoundingClientRect(); + return { top: r.top, bottom: r.bottom, height: r.height }; + } + const outputs = document.querySelectorAll(".jp-OutputArea"); + const outputArea = outputs[idx] ?? null; + if (!outputArea) { + return { + outputArea: null, + agRoot: null, + lastRow: null, + classMode: "", + domLayout: null, + rowCount: 0, + } as any; + } + // BuckarooInfiniteWidget renders TWO AG-Grids per output area: the + // StatusBar's 1-row control grid (no `.df-viewer` ancestor) and the + // main data grid (inside `.df-viewer`). Scope to .df-viewer so we + // measure the data grid, not the controls. + const dfViewer = outputArea.querySelector(".df-viewer"); + const agRoot = dfViewer?.querySelector(".ag-root-wrapper") ?? null; + let classMode = ""; + if (dfViewer) { + if (dfViewer.classList.contains("short-mode")) classMode = "short-mode"; + else if (dfViewer.classList.contains("regular-mode")) classMode = "regular-mode"; + else classMode = "unknown"; + } + let domLayout: CellMetrics["domLayout"] = null; + if (agRoot) { + const layout = + agRoot.querySelector( + ".ag-layout-auto-height, .ag-layout-normal, .ag-layout-print", + ) ?? agRoot; + if (layout.classList.contains("ag-layout-auto-height")) domLayout = "autoHeight"; + else if (layout.classList.contains("ag-layout-normal")) domLayout = "normal"; + else if (layout.classList.contains("ag-layout-print")) domLayout = "print"; + else domLayout = "other"; + } + const rows = agRoot?.querySelectorAll(".ag-center-cols-container .ag-row") ?? []; + const sorted = Array.from(rows).sort((a, b) => { + const ai = parseInt(a.getAttribute("row-index") || "-1", 10); + const bi = parseInt(b.getAttribute("row-index") || "-1", 10); + return ai - bi; + }); + const lastRowEl = sorted.length ? sorted[sorted.length - 1] : null; + return { + outputArea: rect(outputArea), + agRoot: rect(agRoot), + lastRow: lastRowEl + ? { + ...(rect(lastRowEl) as any), + rowIndex: parseInt(lastRowEl.getAttribute("row-index") || "-1", 10), + } + : null, + classMode, + domLayout, + rowCount: sorted.length, + } as CellMetrics; + }, { idx: cellIndex }); +} + +// Per AG-Grid: 1-2px wrapper border, plus the horizontal scrollbar strip +// at the bottom when columns overflow horizontally (~17px on Chromium). +const ROW_TO_GRID_SLACK = 35; + +const VIEWPORTS = [ + { width: 1280, height: 900 }, + { width: 1280, height: 500 }, +]; + +for (const vp of VIEWPORTS) { + test.describe(`Jupyter buckaroo height — viewport ${vp.width}x${vp.height}`, () => { + test.use({ viewport: vp }); + + test("small DF — grid hugs its rows (autoHeight via short-mode)", async ({ page }) => { + await openAndRunNotebook(page); + const m = await measureCell(page, 0); + console.log(`small-DF cell @ ${vp.width}x${vp.height}:`, JSON.stringify(m, null, 2)); + + expect(m.agRoot).not.toBeNull(); + expect(m.lastRow).not.toBeNull(); + // 4 rows + header — gridUtils auto-shorts. + expect(m.classMode).toBe("short-mode"); + expect(m.domLayout).toBe("autoHeight"); + expect(m.rowCount).toBeGreaterThanOrEqual(4); + + // No dead band inside the grid. + const inner = m.agRoot!.bottom - m.lastRow!.bottom; + expect(inner).toBeLessThanOrEqual(ROW_TO_GRID_SLACK); + }); + + test("large DF — grid fills the allotted fixed height, no internal gap above last visible row", async ({ + page, + }) => { + await openAndRunNotebook(page); + const m = await measureCell(page, 1); + console.log(`large-DF cell @ ${vp.width}x${vp.height}:`, JSON.stringify(m, null, 2)); + + expect(m.agRoot).not.toBeNull(); + // 200 rows: gridUtils picks regular-mode → domLayout normal. + expect(m.classMode).toBe("regular-mode"); + expect(m.domLayout).toBe("normal"); + + // In normal mode the grid scrolls — last visible row is somewhere + // in the middle of the dataset. We assert the grid actually + // claimed nontrivial height: at least 100px even at 500px viewport. + expect(m.agRoot!.height).toBeGreaterThan(100); + + // Width-overflow scrollbar (no h-scroll on this DF) and the AG + // status bar add ~25-35px between the last visible row and the + // grid wrapper bottom. Anything larger means a layout regression. + if (m.lastRow) { + const inner = m.agRoot!.bottom - m.lastRow!.bottom; + expect(inner).toBeLessThanOrEqual(ROW_TO_GRID_SLACK + 20); + } + }); + }); +} diff --git a/packages/buckaroo-js-core/pw-tests/server-standalone-layout.spec.ts b/packages/buckaroo-js-core/pw-tests/server-standalone-layout.spec.ts index d6781678d..ab4cd5c19 100644 --- a/packages/buckaroo-js-core/pw-tests/server-standalone-layout.spec.ts +++ b/packages/buckaroo-js-core/pw-tests/server-standalone-layout.spec.ts @@ -198,6 +198,116 @@ test.describe('Standalone layout: filename, prompt, fill, bottom gap', () => { }); }); +// ---------- short-viewport coverage ------------------------------------------ +// Companion to #847: the standalone server doesn't expose autoHeight via URL, +// but it DOES compute heightStyle() from window.innerHeight. A very short +// viewport drives dfvHeight = innerHeight/2 to tiny values; we want to make +// sure the layout still fills the page (no large dead band at the bottom) +// and the bars stay visible. +test.describe('Standalone layout — short viewports', () => { + let csvPath: string; + const PROMPT_TEXT = 'short-viewport'; + + test.beforeAll(() => { + csvPath = writeTempCsv(500); + }); + + test.afterAll(() => { + cleanupFile(csvPath); + }); + + const SHORT_SIZES = [ + { width: 1280, height: 500 }, + { width: 1280, height: 400 }, + ]; + + for (const size of SHORT_SIZES) { + test(`large DF, viewport ${size.width}x${size.height} — grid fills the page`, async ({ + page, + request, + }) => { + const session = `layout-short-${size.height}-${Date.now()}`; + await loadBuckaroo(request, session, csvPath, PROMPT_TEXT); + await page.setViewportSize(size); + await page.goto(`${BASE}/s/${session}`); + await waitForBuckarooGrid(page); + + await page.screenshot({ + path: path.join(screenshotsDir, `layout-short-${size.width}x${size.height}.png`), + }); + + const m = await measureLayout(page); + console.log(`Layout (large DF) at ${size.width}x${size.height}:`, JSON.stringify(m, null, 2)); + + // Bars stay visible even at 400px. + expect(m.filenameBar?.visible).toBe(true); + expect(m.promptBar?.visible).toBe(true); + + // Grid still fills most of the viewport. We loosen the percent + // floor a bit for very short heights — bars eat a larger fraction. + expect(m.gridFillPercent).toBeGreaterThan(55); + + // Bottom gap stays bounded — no dead band larger than ~35px. + expect(m.bottomGapFromGrid).toBeGreaterThanOrEqual(15); + expect(m.bottomGapFromGrid).toBeLessThanOrEqual(35); + }); + } +}); + +// ---------- small DF (#847 territory) ---------------------------------------- +// The 500-row csv above always lands the grid in regular-mode. A 4-row csv +// triggers heightStyle()'s short-mode branch (numRows < maxRowsWithoutScrolling) +// — gridUtils auto-flips domLayout to "autoHeight" even without an explicit +// prop. The standalone server's CSS (.theme-hanger { flex: 1 !important }) is +// supposed to keep the grid stretched anyway. This test catches the case +// where short-mode CSS would otherwise collapse the grid to ~84px and leave +// the rest of the viewport empty. +test.describe('Standalone layout — small DF', () => { + let csvPath: string; + const PROMPT_TEXT = 'small-df'; + + test.beforeAll(() => { + csvPath = writeTempCsv(4); + }); + + test.afterAll(() => { + cleanupFile(csvPath); + }); + + const SIZES = [ + { width: 1280, height: 720 }, + { width: 1280, height: 500 }, + ]; + + for (const size of SIZES) { + test(`4-row CSV at ${size.width}x${size.height} — grid still fills viewport`, async ({ + page, + request, + }) => { + const session = `layout-small-${size.height}-${Date.now()}`; + await loadBuckaroo(request, session, csvPath, PROMPT_TEXT); + await page.setViewportSize(size); + await page.goto(`${BASE}/s/${session}`); + await waitForBuckarooGrid(page); + + await page.screenshot({ + path: path.join(screenshotsDir, `layout-small-${size.width}x${size.height}.png`), + }); + + const m = await measureLayout(page); + console.log(`Layout (small DF) at ${size.width}x${size.height}:`, JSON.stringify(m, null, 2)); + + // The grid is bigger than 4 rows × 21px = ~84px because the + // standalone server's CSS stretches .theme-hanger with flex: 1. + // If this regresses (eg. flex: 1 override removed), the assertion + // catches a < 55% grid fill. + expect(m.gridFillPercent).toBeGreaterThan(55); + expect(m.bottomGapFromGrid).toBeGreaterThanOrEqual(15); + expect(m.bottomGapFromGrid).toBeLessThanOrEqual(35); + }); + } +}); + // ---------- browser_action response field ------------------------------------ test.describe('POST /load response fields', () => { diff --git a/packages/buckaroo-js-core/src/server/BuckarooServerView.forwarding.test.tsx b/packages/buckaroo-js-core/src/server/BuckarooServerView.forwarding.test.tsx new file mode 100644 index 000000000..f0bfb3cb9 --- /dev/null +++ b/packages/buckaroo-js-core/src/server/BuckarooServerView.forwarding.test.tsx @@ -0,0 +1,89 @@ +/** + * BuckarooServerView — autoHeight forwarding (#846). + * + * Pins the one-line spread at BuckarooServerView.tsx that forwards + * `autoHeight` to BuckarooView. Without this test, a future refactor that + * destructures the prop but forgets to forward it would silently regress. + */ +import { render, cleanup, waitFor } from "@testing-library/react"; +import { BuckarooServerView } from "./BuckarooServerView"; + +const capturedViewProps: any[] = []; + +jest.mock("./BuckarooView", () => ({ + BuckarooView: (props: any) => { + capturedViewProps.push(props); + return
; + }, + pickMode: (m: unknown) => (m === "buckaroo" ? "buckaroo" : "viewer"), +})); + +jest.mock("./WebSocketModel", () => ({ + WebSocketModel: class { constructor(_ws: any, _state: any) {} }, +})); + +class FakeWebSocket { + static instances: FakeWebSocket[] = []; + binaryType = "arraybuffer"; + onopen: (() => void) | null = null; + onerror: ((e: any) => void) | null = null; + private listeners: Record void>> = {}; + constructor(public url: string) { + FakeWebSocket.instances.push(this); + setTimeout(() => { + this.onopen?.(); + // Defer message delivery so BuckarooServerView's second + // `await new Promise(...)` can register its listener first. + setTimeout(() => { + const initial = { + type: "initial_state", + df_meta: { total_rows: 4, columns: 2, filtered_rows: 4, rows_shown: 4 }, + df_data_dict: {}, + df_display_args: { + main: { + df_viewer_config: { pinned_rows: [], left_col_configs: [], column_config: [] }, + summary_stats_key: "all_stats", + }, + }, + mode: "viewer", + }; + this.listeners["message"]?.forEach((h) => h({ data: JSON.stringify(initial) } as any)); + }, 0); + }, 0); + } + addEventListener(ev: string, h: (e: any) => void) { + (this.listeners[ev] ??= new Set()).add(h); + } + removeEventListener(ev: string, h: (e: any) => void) { + this.listeners[ev]?.delete(h); + } + close() {} +} + +const origWebSocket = (globalThis as any).WebSocket; + +beforeAll(() => { + (globalThis as any).WebSocket = FakeWebSocket; +}); +afterAll(() => { + (globalThis as any).WebSocket = origWebSocket; +}); +afterEach(() => { + capturedViewProps.length = 0; + FakeWebSocket.instances.length = 0; + cleanup(); +}); + +describe("BuckarooServerView autoHeight forwarding (#846)", () => { + it("forwards autoHeight=true to BuckarooView", async () => { + render(); + await waitFor(() => expect(capturedViewProps.length).toBeGreaterThan(0)); + expect(capturedViewProps[capturedViewProps.length - 1].autoHeight).toBe(true); + }); + + it("forwards autoHeight=undefined to BuckarooView when the prop is omitted", async () => { + render(); + await waitFor(() => expect(capturedViewProps.length).toBeGreaterThan(0)); + expect(capturedViewProps[capturedViewProps.length - 1].autoHeight).toBeUndefined(); + }); +}); diff --git a/packages/buckaroo-js-core/src/server/BuckarooServerView.tsx b/packages/buckaroo-js-core/src/server/BuckarooServerView.tsx index de66359e6..e1a538666 100644 --- a/packages/buckaroo-js-core/src/server/BuckarooServerView.tsx +++ b/packages/buckaroo-js-core/src/server/BuckarooServerView.tsx @@ -58,6 +58,13 @@ export interface BuckarooServerViewProps { /** Optional className on the wrapping div. */ className?: string; + + /** When true, render with AG Grid's `domLayout: "autoHeight"`: the grid + * grows to fit its row count instead of filling the parent container. + * Use for stacked-cell hosts (notebook-style embeds) where a single + * fixed embed height looks wrong for both small and large dataframes. + * Overrides any `component_config.layoutType` set by the server. */ + autoHeight?: boolean; } interface ReadyState { @@ -81,6 +88,7 @@ export function BuckarooServerView({ onMetadata, style, className, + autoHeight, }: BuckarooServerViewProps): React.ReactElement { const [ready, setReady] = React.useState(null); const [error, setError] = React.useState(null); @@ -176,6 +184,7 @@ export function BuckarooServerView({ onMetadata={onMetadata} style={style} className={className} + autoHeight={autoHeight} /> ); } diff --git a/packages/buckaroo-js-core/src/server/BuckarooView.autoHeight.test.tsx b/packages/buckaroo-js-core/src/server/BuckarooView.autoHeight.test.tsx new file mode 100644 index 000000000..fa980b5a7 --- /dev/null +++ b/packages/buckaroo-js-core/src/server/BuckarooView.autoHeight.test.tsx @@ -0,0 +1,122 @@ +/** + * BuckarooView — autoHeight prop (#846). + * + * Hosts that stack multiple BuckarooServerView embeds (notebook-style, one + * per dataframe) can't pick a one-size embed height: a 4-row aggregate and + * a 100k-row main frame need very different vertical real estate. The + * `autoHeight` prop lets hosts opt into AG Grid's `domLayout: "autoHeight"` + * so the grid grows to its row count instead of filling the parent. + */ +import { render, cleanup, act } from "@testing-library/react"; +import { BuckarooView } from "./BuckarooView"; +import type { IModel } from "./IModel"; + +// Capture props passed to the (stubbed) viewer surface so the test can +// inspect what BuckarooView pushed down — that's where the autoHeight +// plumbing has to land. +const capturedDsProps: any[] = []; + +jest.mock("../components/BuckarooWidgetInfinite", () => ({ + BuckarooInfiniteWidget: () =>
, + DFViewerInfiniteDS: (props: any) => { + capturedDsProps.push(props); + return
; + }, + getKeySmartRowCache: jest.fn(() => ({ __stub: "row-cache" })), +})); + +function makeFakeModel(): IModel { + const state: Record = {}; + return { + send: () => {}, + get: (k) => state[k], + set: (k, v) => { state[k] = v; }, + save_changes: () => {}, + on: () => {}, + off: () => {}, + }; +} + +afterEach(() => { + capturedDsProps.length = 0; + cleanup(); +}); + +const baseInitialState = () => ({ + df_meta: { total_rows: 4, columns: 2, filtered_rows: 4, rows_shown: 4 }, + df_data_dict: {}, + df_display_args: { + main: { + df_viewer_config: { pinned_rows: [], left_col_configs: [], column_config: [] }, + summary_stats_key: "all_stats", + }, + }, +}); + +describe("BuckarooView autoHeight (#846)", () => { + it("injects component_config.layoutType='autoHeight' into df_display_args when autoHeight is set", async () => { + await act(async () => { + render( + , + ); + }); + + expect(capturedDsProps.length).toBeGreaterThan(0); + const last = capturedDsProps[capturedDsProps.length - 1]; + const cc = last.df_display_args.main.df_viewer_config.component_config; + expect(cc?.layoutType).toBe("autoHeight"); + }); + + it("preserves a server-provided layoutType when autoHeight is not set", async () => { + const state = { + df_meta: { total_rows: 4, columns: 2, filtered_rows: 4, rows_shown: 4 }, + df_data_dict: {}, + df_display_args: { + main: { + df_viewer_config: { + pinned_rows: [], + left_col_configs: [], + column_config: [], + component_config: { layoutType: "normal" }, + }, + summary_stats_key: "all_stats", + }, + }, + }; + await act(async () => { + render( + , + ); + }); + + const last = capturedDsProps[capturedDsProps.length - 1]; + const cc = last.df_display_args.main.df_viewer_config.component_config; + expect(cc?.layoutType).toBe("normal"); + }); + + it("drops height:100% from the wrapper when autoHeight is set, so the grid can grow with its rows", async () => { + const { container } = render( + , + ); + + const wrapper = container.querySelector(".buckaroo_anywidget") as HTMLElement | null; + expect(wrapper).not.toBeNull(); + // height:100% would cap the grid inside the parent — the whole point + // of autoHeight is for the wrapper to size to its contents. + expect(wrapper!.style.height).not.toBe("100%"); + }); +}); diff --git a/packages/buckaroo-js-core/src/server/BuckarooView.tsx b/packages/buckaroo-js-core/src/server/BuckarooView.tsx index 4fdda11eb..573671df0 100644 --- a/packages/buckaroo-js-core/src/server/BuckarooView.tsx +++ b/packages/buckaroo-js-core/src/server/BuckarooView.tsx @@ -81,6 +81,13 @@ export interface BuckarooViewProps { /** Optional className on the wrapping div. */ className?: string; + + /** When true, render with AG Grid's `domLayout: "autoHeight"`: the grid + * grows to fit its row count instead of filling the parent container. + * Use for stacked-cell hosts (notebook-style embeds) where a fixed + * embed height looks wrong for both small and large dataframes. + * Overrides any `component_config.layoutType` set by the server. */ + autoHeight?: boolean; } export function pickMode(rawMode: unknown): BuckarooServerMode { @@ -108,6 +115,7 @@ export function BuckarooView({ onMetadata, style, className, + autoHeight, }: BuckarooViewProps): React.ReactElement { // If the caller passed raw initial_state straight off the wire, // df_data_dict may still contain parquet_b64 payload objects. Those @@ -248,9 +256,32 @@ export function BuckarooView({ model.save_changes(); }, [model]); - const wrapperStyle: React.CSSProperties = { width: "100%", height: "100%", ...(style ?? {}) }; + // gridUtils already honors component_config.layoutType — stamp it per + // display-arg entry and let getHeightStyle2 do the rest. Memoized to + // keep child reference identity stable across re-renders. + const effectiveDisplayArgs = React.useMemo>(() => { + if (!autoHeight) return dfDisplayArgs; + const out: Record = {}; + for (const [k, v] of Object.entries(dfDisplayArgs)) { + out[k] = { + ...v, + df_viewer_config: { + ...v.df_viewer_config, + component_config: { + ...(v.df_viewer_config.component_config ?? {}), + layoutType: "autoHeight", + }, + }, + }; + } + return out; + }, [dfDisplayArgs, autoHeight]); + + const wrapperStyle: React.CSSProperties = autoHeight + ? { width: "100%", ...(style ?? {}) } + : { width: "100%", height: "100%", ...(style ?? {}) }; - if (!dfDisplayArgs?.main || !dataReady) { + if (!effectiveDisplayArgs?.main || !dataReady) { return (
Preparing…
@@ -264,7 +295,7 @@ export function BuckarooView({ diff --git a/packages/buckaroo-js-core/src/stories/BuckarooViewHeight.stories.tsx b/packages/buckaroo-js-core/src/stories/BuckarooViewHeight.stories.tsx new file mode 100644 index 000000000..c445662ed --- /dev/null +++ b/packages/buckaroo-js-core/src/stories/BuckarooViewHeight.stories.tsx @@ -0,0 +1,268 @@ +/** + * Stories that exercise the BuckarooView autoHeight code path (#847). + * + * Rather than wire up a fake transport that returns parquet-encoded + * infinite_resp messages (BuckarooView's getKeySmartRowCache decodes + * parquet bytes off the wire), these stories drive DFViewerInfiniteDS + * directly with an in-memory KeyAwareSmartRowCache — the same shortcut + * SmallDFScroll.stories.tsx uses — but reproduce BuckarooView's two + * autoHeight outputs verbatim: + * + * 1. component_config.layoutType = "autoHeight" stamped onto every + * df_display_args entry (gridUtils.getHeightStyle2 honors it). + * 2. wrapper style: no `height: 100%` when autoHeight is true. + * + * Playwright then varies the storybook viewport (short / tall) and the + * synthetic row count (small / large) to verify there are no spurious + * gaps between: + * + * - the last data row and the AG-Grid root wrapper bottom + * - the AG-Grid root wrapper and the BuckarooView-style wrapper bottom + * - the wrapper and the host container bottom + * + * Each story declares a fixed-pixel host (`data-testid="host"`) so the + * test can pin the "outer container" rectangle without depending on + * storybook chrome. + */ +import type { Meta, StoryObj } from "@storybook/react"; +import React, { useMemo } from "react"; +import { DFViewerInfiniteDS } from "../components/BuckarooWidgetInfinite"; +import { DFMeta } from "../components/WidgetTypes"; +import { IDisplayArgs } from "../components/DFViewerParts/gridUtils"; +import { + KeyAwareSmartRowCache, + PayloadArgs, + PayloadResponse, +} from "../components/DFViewerParts/SmartRowCache"; + +// ----------------------------------------------------------------------------- +// Synthetic dataframe and cache +// ----------------------------------------------------------------------------- +function makeRows(n: number) { + return Array.from({ length: n }, (_, i) => ({ + index: i, + a: i * 10, + b: `row_${i}`, + })); +} + +function makeCache(rowCount: number): KeyAwareSmartRowCache { + const allData = makeRows(rowCount); + const cache = new KeyAwareSmartRowCache((pa: PayloadArgs) => { + const dataEnd = Math.min(pa.end, rowCount); + if (dataEnd <= pa.start) return; + const resp: PayloadResponse = { + key: pa, + data: allData.slice(pa.start, dataEnd), + length: rowCount, + }; + // Async reply mirrors a real server. + setTimeout(() => cache.addPayloadResponse(resp), 5); + }); + return cache; +} + +function makeDisplayArgs(autoHeight: boolean): Record { + return { + main: { + data_key: "main", + df_viewer_config: { + pinned_rows: [], + left_col_configs: [ + { + col_name: "index", + header_name: "index", + displayer_args: { displayer: "string" }, + }, + ], + column_config: [ + { + col_name: "a", + header_name: "a", + displayer_args: { displayer: "integer", min_digits: 1, max_digits: 5 }, + }, + { + col_name: "b", + header_name: "b", + displayer_args: { displayer: "obj" }, + }, + ], + // Mirrors what BuckarooView does in autoHeight mode (#847). + component_config: autoHeight ? { layoutType: "autoHeight" } : undefined, + }, + summary_stats_key: "all_stats", + }, + } as Record; +} + +// ----------------------------------------------------------------------------- +// BuckarooView-shaped wrapper +// ----------------------------------------------------------------------------- +/** + * Reproduces BuckarooView's wrapper element verbatim so layout effects + * (the height-100% drop in autoHeight mode) are visible in the rendered + * DOM. Class name + data attributes match what BuckarooView ships so + * tests using `.buckaroo_anywidget` work both here and in production. + */ +const BuckarooViewLikeWrapper: React.FC<{ + autoHeight: boolean; + children: React.ReactNode; +}> = ({ autoHeight, children }) => { + const wrapperStyle: React.CSSProperties = autoHeight + ? { width: "100%" } + : { width: "100%", height: "100%" }; + return ( +
+ {children} +
+ ); +}; + +const Cell: React.FC<{ + rowCount: number; + autoHeight: boolean; + label?: string; +}> = ({ rowCount, autoHeight, label }) => { + const src = useMemo(() => makeCache(rowCount), [rowCount]); + const dfMeta: DFMeta = useMemo( + () => ({ + total_rows: rowCount, + columns: 2, + filtered_rows: rowCount, + rows_shown: rowCount, + }), + [rowCount], + ); + const df_display_args = useMemo(() => makeDisplayArgs(autoHeight), [autoHeight]); + const df_data_dict = useMemo(() => ({}), []); + return ( + + {label !== undefined ? ( +
+ {label} — {rowCount} rows, autoHeight={String(autoHeight)} +
+ ) : null} + +
+ ); +}; + +// ----------------------------------------------------------------------------- +// Hosts +// ----------------------------------------------------------------------------- +const Single: React.FC<{ + rowCount: number; + autoHeight: boolean; + hostHeight: number; +}> = ({ rowCount, autoHeight, hostHeight }) => ( +
+ +
+); + +const Stacked: React.FC<{ + rowCounts: [number, number]; + autoHeight: boolean; + hostHeight: number; +}> = ({ rowCounts, autoHeight, hostHeight }) => ( +
+
+ +
+
+ +
+
+); + +// ----------------------------------------------------------------------------- +// Meta + stories +// ----------------------------------------------------------------------------- +const meta: Meta = { + title: "Buckaroo/Height/BuckarooView", + component: Single, + parameters: { layout: "fullscreen" }, + argTypes: { + rowCount: { control: { type: "number", min: 1, max: 5000, step: 1 } }, + autoHeight: { control: "boolean" }, + hostHeight: { control: { type: "number", min: 100, max: 2000, step: 10 } }, + }, +}; +export default meta; + +type SingleStory = StoryObj; + +// Tall host (typical browser viewport) +export const SmallDfFixed: SingleStory = { + args: { rowCount: 3, autoHeight: false, hostHeight: 700 }, +}; +export const SmallDfAutoHeight: SingleStory = { + args: { rowCount: 3, autoHeight: true, hostHeight: 700 }, +}; +export const LargeDfFixed: SingleStory = { + args: { rowCount: 2000, autoHeight: false, hostHeight: 700 }, +}; +export const LargeDfAutoHeight: SingleStory = { + args: { rowCount: 2000, autoHeight: true, hostHeight: 700 }, +}; + +// Short host — caps the host at 400px to exercise the +// short-viewport branch of heightStyle(). +export const SmallDfShortHostFixed: SingleStory = { + args: { rowCount: 3, autoHeight: false, hostHeight: 400 }, +}; +export const SmallDfShortHostAutoHeight: SingleStory = { + args: { rowCount: 3, autoHeight: true, hostHeight: 400 }, +}; +export const LargeDfShortHostFixed: SingleStory = { + args: { rowCount: 2000, autoHeight: false, hostHeight: 400 }, +}; +export const LargeDfShortHostAutoHeight: SingleStory = { + args: { rowCount: 2000, autoHeight: true, hostHeight: 400 }, +}; + +// Stacked — the actual #846/#847 motivating use case. +type StackedArgs = { + rowCounts: [number, number]; + autoHeight: boolean; + hostHeight: number; +}; +export const StackedAutoHeightSmallLarge: StoryObj = { + render: (args: StackedArgs) => , + args: { rowCounts: [4, 200], autoHeight: true, hostHeight: 900 }, +}; +export const StackedFixedSmallLarge: StoryObj = { + render: (args: StackedArgs) => , + args: { rowCounts: [4, 200], autoHeight: false, hostHeight: 900 }, +}; diff --git a/packages/js/standalone.tsx b/packages/js/standalone.tsx index fac30e349..07e470fb8 100644 --- a/packages/js/standalone.tsx +++ b/packages/js/standalone.tsx @@ -35,8 +35,15 @@ function patchDisplayArgsHeight(displayArgs: any): any { const fb = document.getElementById("filename-bar"); const pb = document.getElementById("prompt-bar"); const barsHeight = (fb?.offsetHeight || 0) + (pb?.offsetHeight || 0); - // 20px bottom gap — CSS flex: 1 on .theme-hanger handles the actual fill, - // but dfvHeight tells heightStyle() to use domLayout: "normal" (not autoHeight) + // 20px bottom gap; the CSS `flex: 1 !important` on .theme-hanger lets + // the wrapper fill the available space, but for small dataframes + // gridUtils.heightStyle() defaults to `domLayout: "autoHeight"` — + // which makes AG-Grid size to its row count, leaving a large dead + // band below the grid (issue surfaced by + // pw-tests/server-standalone-layout.spec.ts "small DF" tests). Force + // `layoutType: "normal"` so the standalone single-frame tab always + // fills the viewport. Embedded hosts that want autoHeight go through + // BuckarooServerView's `autoHeight` prop instead (PR #847). const available = window.innerHeight - barsHeight - 20; const patched = { ...displayArgs }; for (const key of Object.keys(patched)) { @@ -49,6 +56,7 @@ function patchDisplayArgsHeight(displayArgs: any): any { component_config: { ...view.df_viewer_config?.component_config, dfvHeight: available, + layoutType: "normal", }, }, }; diff --git a/scripts/test_playwright_jupyter.sh b/scripts/test_playwright_jupyter.sh index 03e9d712d..0ac63391d 100755 --- a/scripts/test_playwright_jupyter.sh +++ b/scripts/test_playwright_jupyter.sh @@ -80,6 +80,7 @@ NOTEBOOKS=( "test_polars_dfviewer_infinite.ipynb" "test_infinite_scroll_transcript.ipynb" "test_xorq_infinite_scroll.ipynb" + "test_buckaroo_widget_height.ipynb" ) # If specific notebook(s) provided, test only those (comma-separated) @@ -389,6 +390,9 @@ for notebook in "${NOTEBOOKS[@]}"; do elif [[ "$notebook" == "test_xorq_infinite_scroll.ipynb" ]]; then PW_TEST_FILE="pw-tests/xorq-infinite-scroll.spec.ts" PW_TIMEOUT=60000 + elif [[ "$notebook" == "test_buckaroo_widget_height.ipynb" ]]; then + PW_TEST_FILE="pw-tests/jupyter-buckaroo-height.spec.ts" + PW_TIMEOUT=60000 else PW_TEST_FILE="pw-tests/integration.spec.ts" PW_TIMEOUT=30000 diff --git a/scripts/test_playwright_storybook.sh b/scripts/test_playwright_storybook.sh index 796cbdeac..dd0db4b85 100755 --- a/scripts/test_playwright_storybook.sh +++ b/scripts/test_playwright_storybook.sh @@ -94,6 +94,7 @@ FAILED_TESTS=() STORYBOOK_TESTS=( "pw-tests/transcript-replayer.spec.ts" "pw-tests/outside-params.spec.ts" + "pw-tests/buckaroo-view-height.spec.ts" # "pw-tests/example.spec.ts" # Has pre-existing failures, excluded for now ) diff --git a/tests/integration_notebooks/test_buckaroo_widget_height.ipynb b/tests/integration_notebooks/test_buckaroo_widget_height.ipynb new file mode 100644 index 000000000..1dd224c39 --- /dev/null +++ b/tests/integration_notebooks/test_buckaroo_widget_height.ipynb @@ -0,0 +1,72 @@ +{ + "cells": [ + { + "cell_type": "markdown", + "metadata": {}, + "source": [ + "# Buckaroo widget — height regression notebook\n", + "\n", + "Two cells: a tiny dataframe (4 rows) and a larger one (200 rows). The\n", + "Playwright spec `pw-tests/jupyter-buckaroo-height.spec.ts` opens this\n", + "notebook at varying viewport heights and asserts:\n", + "\n", + " - The widget renders ag-grid cells in both output areas.\n", + " - The grid bottom never sits more than a few pixels above the last\n", + " data row (no dead band inside the grid).\n", + " - The grid hugs its content for the small DF — Jupyter notebooks\n", + " naturally want auto-sized embeds because their output container\n", + " grows with the cell content." + ] + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "import buckaroo # noqa\n", + "import pandas as pd\n", + "from buckaroo import BuckarooWidget\n", + "\n", + "small_df = pd.DataFrame({\n", + " 'name': ['Alice', 'Bob', 'Charlie', 'Dianne'],\n", + " 'age': [25, 30, 35, 28],\n", + " 'score': [85.5, 92.0, 78.3, 88.1],\n", + "})\n", + "BuckarooWidget(small_df)" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "large_df = pd.DataFrame({\n", + " 'idx': list(range(200)),\n", + " 'val': [i * 1.5 for i in range(200)],\n", + " 'lbl': [f'row_{i}' for i in range(200)],\n", + "})\n", + "BuckarooWidget(large_df)" + ] + } + ], + "metadata": { + "kernelspec": { + "display_name": "Python 3", + "language": "python", + "name": "python3" + }, + "language_info": { + "codemirror_mode": {"name": "ipython", "version": 3}, + "file_extension": ".py", + "mimetype": "text/x-python", + "name": "python", + "nbconvert_exporter": "python", + "pygments_lexer": "ipython3", + "version": "3.11" + } + }, + "nbformat": 4, + "nbformat_minor": 4 +}