From fac0115574c5dfed29b12b1f8a5d541f74f5e585 Mon Sep 17 00:00:00 2001 From: marikyan Date: Thu, 20 Aug 2026 10:50:25 +0400 Subject: [PATCH] fix: remove extra horizontal scrollbar space, calculate row heights new logic --- .../core/src/data-editor/data-editor-fns.ts | 68 +++++++++++++++++++ packages/core/src/data-editor/data-editor.tsx | 47 ++++++------- packages/core/test/data-editor-fns.test.ts | 63 ++++++++++++++++- 3 files changed, 154 insertions(+), 24 deletions(-) diff --git a/packages/core/src/data-editor/data-editor-fns.ts b/packages/core/src/data-editor/data-editor-fns.ts index f350cf8ce..d5d82b1bc 100644 --- a/packages/core/src/data-editor/data-editor-fns.ts +++ b/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`]; +} diff --git a/packages/core/src/data-editor/data-editor.tsx b/packages/core/src/data-editor/data-editor.tsx index 9e9ea046b..1a6155d40 100644 --- a/packages/core/src/data-editor/data-editor.tsx +++ b/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([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 { - 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); diff --git a/packages/core/test/data-editor-fns.test.ts b/packages/core/test/data-editor-fns.test.ts index 83b03b620..5fe829d13 100644 --- a/packages/core/test/data-editor-fns.test.ts +++ b/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"]); + }); +});