Remove extra horizontal scrollbar space, improve row height estimation

packages/core/src/data-editor/data-editor-fns.ts

@@ -225,3 +225,71 @@ export function copyToClipboard(
 export function toggleBoolean(data: boolean | null | undefined): boolean | null | undefined {
     return data !== true;
 }
+
+/**
+ * Computes the intrinsic `[width, height]` (as CSS px strings) for the grid's scrollable content,
+ * used to size the container when the consumer doesn't pass an explicit `width`/`height`.
+ */
+export function computeIdealSize(
+    rowHeight: number | ((row: number) => number),
+    rows: number,
+    showTrailingBlankRow: boolean,
+    totalHeaderHeight: number,
+    contentWidth: number,
+    clientAreaWidth: number,
+    clientAreaHeight: number,
+    scrollbarWidth: number
+): readonly [string, string] {
+    let h: number;
+    const rowsCountWithTrailingRow = rows + (showTrailingBlankRow ? 1 : 0);
+    if (typeof rowHeight === "number") {
+        h = totalHeaderHeight + rowsCountWithTrailingRow * rowHeight;
+    } else {
+        h = totalHeaderHeight;
+        if (clientAreaHeight > 0) {
+            // Sum exactly until the viewport is full (an undershoot here spuriously shows a
+            // scrollbar); extrapolate the rest from the average to avoid an O(n) scan on huge datasets.
+            const availableRowSpace = clientAreaHeight - totalHeaderHeight;
+            // Keep summing past a viewport-filling overflow until we've sampled at least this many
+            // rows, so the average used to extrapolate the rest isn't based on a single tall row.
+            const minSamples = Math.min(rowsCountWithTrailingRow, 10);
+            let summedHeight = 0;
+            let rowsSummed = 0;
+            while (rowsSummed < rowsCountWithTrailingRow) {
+                const rh = rowHeight(rowsSummed);
+                h += rh;
+                summedHeight += rh;
+                rowsSummed++;
+                if (summedHeight > availableRowSpace && rowsSummed >= minSamples) {
+                    break;
+                }
+            }
+            const remainingRows = rowsCountWithTrailingRow - rowsSummed;
+            if (remainingRows > 0) {
+                h += (summedHeight / rowsSummed) * remainingRows;
+            }
+        } else {
+            // Not yet measured (e.g. first render) — fall back to a bounded sample average
+            // rather than summing every row.
+            let avg = 0;
+            const toAverage = Math.min(rowsCountWithTrailingRow, 10);
+            for (let i = 0; i < toAverage; i++) {
+                avg += rowHeight(i);
+            }
+            avg = toAverage > 0 ? Math.floor(avg / toAverage) : 0;
+            h += rowsCountWithTrailingRow * avg;
+        }
+    }
+
+    // Only reserve vertical room for a horizontal scrollbar when the content actually overflows
+    // horizontally; reserving it unconditionally left dead space below the grid when it didn't.
+    if (clientAreaWidth > 0 && contentWidth > clientAreaWidth) {
+        h += scrollbarWidth;
+    }
+
+    const w = contentWidth + scrollbarWidth;
+
+    // We need to set a reasonable cap here as some browsers will just ignore huge values
+    // rather than treat them as huge values.
+    return [`${Math.min(100_000, w)}px`, `${Math.min(100_000, h)}px`];
+}

packages/core/src/data-editor/data-editor.tsx

@@ -57,7 +57,7 @@ import { measureColumn, useColumnSizer } from "./use-column-sizer.js";
 import { isHotkey } from "../common/is-hotkey.js";
 import { type SelectionBlending, useSelectionBehavior } from "../internal/data-grid/use-selection-behavior.js";
 import { useCellsForSelection } from "./use-cells-for-selection.js";
-import { unquote, expandSelection, copyToClipboard, toggleBoolean } from "./data-editor-fns.js";
+import { unquote, expandSelection, copyToClipboard, toggleBoolean, computeIdealSize } from "./data-editor-fns.js";
 import { DataEditorContainer } from "../internal/data-editor-container/data-grid-container.js";
 import { useAutoscroll } from "./use-autoscroll.js";
 import type { CustomRenderer, CellRenderer, InternalCellRenderer } from "../cells/cell-types.js";
@@ -1096,6 +1096,8 @@ const DataEditorImpl: React.ForwardRefRenderFunction<DataEditorRef, DataEditorPr
     }, [theme]);
 
     const [clientSize, setClientSize] = React.useState<readonly [number, number, number]>([0, 0, 0]);
+    // Primitives, not `clientSize` itself, so deps below don't re-fire on every scroll tick's new array.
+    const [clientAreaWidth, clientAreaHeight] = clientSize;
 
     const rendererMap = React.useMemo(() => {
         if (renderers === undefined) return {};
@@ -4272,29 +4274,28 @@ const DataEditorImpl: React.ForwardRefRenderFunction<DataEditorRef, DataEditorPr
     }, []);
 
     const [idealWidth, idealHeight] = React.useMemo(() => {
-        let h: number;
         const scrollbarWidth = experimental?.scrollbarWidthOverride ?? getScrollBarWidth();
-        const rowsCountWithTrailingRow = rows + (showTrailingBlankRow ? 1 : 0);
-        if (typeof rowHeight === "number") {
-            h = totalHeaderHeight + rowsCountWithTrailingRow * rowHeight;
-        } else {
-            let avg = 0;
-            const toAverage = Math.min(rowsCountWithTrailingRow, 10);
-            for (let i = 0; i < toAverage; i++) {
-                avg += rowHeight(i);
-            }
-            avg = Math.floor(avg / toAverage);
-
-            h = totalHeaderHeight + rowsCountWithTrailingRow * avg;
-        }
-        h += scrollbarWidth;
-
-        const w = mangledCols.reduce((acc, x) => x.width + acc, 0) + scrollbarWidth;
-
-        // We need to set a reasonable cap here as some browsers will just ignore huge values
-        // rather than treat them as huge values.
-        return [`${Math.min(100_000, w)}px`, `${Math.min(100_000, h)}px`];
-    }, [mangledCols, experimental?.scrollbarWidthOverride, rowHeight, rows, showTrailingBlankRow, totalHeaderHeight]);
+        const contentWidth = mangledCols.reduce((acc, x) => x.width + acc, 0);
+        return computeIdealSize(
+            rowHeight,
+            rows,
+            showTrailingBlankRow,
+            totalHeaderHeight,
+            contentWidth,
+            clientAreaWidth,
+            clientAreaHeight,
+            scrollbarWidth
+        );
+    }, [
+        mangledCols,
+        experimental?.scrollbarWidthOverride,
+        rowHeight,
+        rows,
+        showTrailingBlankRow,
+        totalHeaderHeight,
+        clientAreaWidth,
+        clientAreaHeight,
+    ]);
 
     const cssStyle = React.useMemo(() => {
         return makeCSSStyle(mergedTheme);

packages/core/test/data-editor-fns.test.ts

@@ -1,6 +1,6 @@
 /* eslint-disable sonarjs/no-duplicate-string */
 import { CompactSelection, type GridSelection } from "../src/index.js";
-import { expandSelection, unquote } from "../src/data-editor/data-editor-fns.js"; // Adjust the import path to your setup
+import { expandSelection, unquote, computeIdealSize } from "../src/data-editor/data-editor-fns.js"; // Adjust the import path to your setup
 import { vi, expect, describe, it, afterEach } from "vitest";
 
 describe("unquote", () => {
@@ -151,3 +151,64 @@ describe("expandSelection", () => {
         expect(getCellsForSelection).toHaveBeenCalledTimes(1);
     });
 });
+
+describe("computeIdealSize", () => {
+    it("sums a numeric row height across all rows, plus the trailing row when shown", () => {
+        expect(computeIdealSize(20, 10, false, 50, 0, 0, 0, 0)).toEqual(["0px", "250px"]);
+        expect(computeIdealSize(20, 10, true, 50, 0, 0, 0, 0)).toEqual(["0px", "270px"]);
+    });
+
+    it("falls back to a bounded sample average when the viewport hasn't been measured, without scanning every row", () => {
+        const rowHeight = vi.fn(() => 25);
+        const [, height] = computeIdealSize(rowHeight, 100_000, false, 50, 0, 0, 0, 0);
+
+        expect(rowHeight).toHaveBeenCalledTimes(10);
+        expect(height).toBe("100000px"); // 50 + 100_000 * 25, capped at 100_000
+    });
+
+    it("computes the unmeasured-average estimate correctly for fewer than 10 rows", () => {
+        const rowHeight = vi.fn((i: number) => 10 + i); // 10, 11, 12, 13, 14 -> avg 12
+        const [, height] = computeIdealSize(rowHeight, 5, false, 0, 0, 0, 0, 0);
+
+        expect(rowHeight).toHaveBeenCalledTimes(5);
+        expect(height).toBe("60px"); // 5 rows * avg(12)
+    });
+
+    it("sums exact row heights until the measured viewport overflows, then extrapolates the rest", () => {
+        const rowHeight = vi.fn(() => 20);
+        const [, height] = computeIdealSize(rowHeight, 1000, false, 0, 0, 0, 100, 0);
+
+        // availableRowSpace is 100; 5 rows exactly fill it, but the loop keeps sampling up to the
+        // 10-row floor before extrapolating the remaining 990 rows from that average.
+        expect(rowHeight).toHaveBeenCalledTimes(10);
+        expect(height).toBe("20000px"); // 200 summed + (200 / 10) * 990
+    });
+
+    it("keeps sampling at least 10 rows even if the very first row alone overflows the viewport", () => {
+        const rowHeight = vi.fn((i: number) => (i === 0 ? 5000 : 20));
+        const [, height] = computeIdealSize(rowHeight, 50, false, 0, 0, 0, 100, 0);
+
+        expect(rowHeight).toHaveBeenCalledTimes(10);
+        expect(height).toBe("25900px"); // 5180 summed + (5180 / 10) * 40 remaining rows
+    });
+
+    it("doesn't crash and does no work when there are no rows", () => {
+        const rowHeight = vi.fn(() => 20);
+
+        expect(computeIdealSize(rowHeight, 0, false, 40, 0, 0, 100, 0)).toEqual(["0px", "40px"]);
+        expect(computeIdealSize(rowHeight, 0, false, 40, 0, 0, 0, 0)).toEqual(["0px", "40px"]);
+        expect(rowHeight).not.toHaveBeenCalled();
+    });
+
+    it("reserves horizontal-scrollbar height only when content overflows the measured client width", () => {
+        expect(computeIdealSize(20, 5, false, 0, 500, 1000, 0, 15)).toEqual(["515px", "100px"]);
+        expect(computeIdealSize(20, 5, false, 0, 1500, 1000, 0, 15)).toEqual(["1515px", "115px"]);
+        // clientAreaWidth <= 0 means the width hasn't been measured yet, so it never reserves.
+        expect(computeIdealSize(20, 5, false, 0, 1500, 0, 0, 15)).toEqual(["1515px", "100px"]);
+    });
+
+    it("caps both dimensions at 100_000px", () => {
+        expect(computeIdealSize(20, 1, false, 0, 200_000, 0, 0, 15)).toEqual(["100000px", "20px"]);
+        expect(computeIdealSize(500_000, 1, false, 0, 0, 0, 0, 0)).toEqual(["0px", "100000px"]);
+    });
+});