From 310b6576cddaa5cd7caf99e8f4a8522293dc7c78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20Bramer=20Schmidt?= Date: Fri, 17 Jul 2026 21:44:37 +0700 Subject: [PATCH 1/2] fix: smooth horizontal scrolling in wide data tables Fixes prisma/studio#1476 (duplicate: #1514). Three defects in center-column virtualization made horizontal scrolling in wide tables jumpy, made the last column unreachable, and snapped the viewport left when clicking a cell: - The virtualization window was derived from a raw scroll-position state that was updated behind a requestAnimationFrame hop, so every scrolled pixel re-rendered the whole grid while the mounted window lagged the real scroll position, showing blank columns and jumpy repaints. The window is now computed synchronously in the scroll/resize handlers and stored in state only when the set of mounted columns actually changes. - The focused-cell auto-scroll effect re-ran on every scroll update while the focused cell's element was not rendered (focused column outside the virtualization window, or focused row not on the current page), re-applying a column-aligned scrollLeft each time and fighting user scrolling. It now runs at most once per focused-cell change and falls back to any rendered cell in the focused row for the vertical reveal. - The window was computed from scrollLeft shifted left by the pinned-column width, under-rendering columns at the right viewport edge. Sticky pinned columns occupy as much viewport as they occupy row start, so container scrollLeft maps 1:1 onto center-column offsets. Verified in the ppg demo against the 61-column all_data_types table: the window follows scrolling to the very end, the last column renders fully, and clicking or focusing cells no longer moves the viewport. Regression tests cover the auto-scroll fight, animation-frame-free window updates, and end-of-grid window coverage. Co-Authored-By: Claude Fable 5 --- .changeset/smooth-horizontal-scroll.md | 5 + Architecture/wide-grid-performance.md | 25 +++ FEATURES.md | 1 + ui/studio/grid/DataGrid.interactions.test.tsx | 95 +++++++++ ui/studio/grid/DataGrid.tsx | 199 ++++++++++-------- .../grid/DataGrid.virtualization.test.tsx | 57 ++++- ui/studio/grid/column-virtualization.test.ts | 77 ++++++- ui/studio/grid/column-virtualization.ts | 26 +++ 8 files changed, 400 insertions(+), 85 deletions(-) create mode 100644 .changeset/smooth-horizontal-scroll.md diff --git a/.changeset/smooth-horizontal-scroll.md b/.changeset/smooth-horizontal-scroll.md new file mode 100644 index 00000000..d02d5646 --- /dev/null +++ b/.changeset/smooth-horizontal-scroll.md @@ -0,0 +1,5 @@ +--- +"@prisma/studio-core": patch +--- + +Fix broken horizontal scrolling in wide data tables. The column virtualization window now follows scrolling synchronously and only re-renders the grid when the set of mounted columns changes, so scrolling no longer jumps between columns and the last column is reachable. Focused-cell auto-scroll now runs at most once per focus change, so clicking a cell to edit no longer snaps the viewport back and off-screen focus targets no longer fight user scrolling. Also corrects the virtualization window offset when columns are pinned. diff --git a/Architecture/wide-grid-performance.md b/Architecture/wide-grid-performance.md index 2583c2c0..9f547521 100644 --- a/Architecture/wide-grid-performance.md +++ b/Architecture/wide-grid-performance.md @@ -60,6 +60,31 @@ Expected impact: lower baseline render cost. Expected impact: prevents future regressions. +## Column Virtualization Update Model (implemented) + +Center-column virtualization is driven by scroll events, not render-time state: + +- The virtualization window (`computeColumnVirtualizationWindow`) is computed + inside the scroll/resize handlers from the live `scrollLeft`/`clientWidth` + of the grid scroll container, synchronously and without an animation-frame + hop. Deferring the window behind `requestAnimationFrame` plus a raw + scroll-position state update let fast scrolling outrun the overscan area and + caused blank columns and jumpy repaints. +- Only the resolved window (`startIndex`/`endIndex`/spacer widths) is stored + in React state, and state is only updated when the window actually changes. + Plain scrolling inside the overscan area therefore never re-renders the + grid. +- The window is computed in center-column coordinates. Left-pinned columns + occupy the start of the scrollable row and overlay the same amount of + viewport width (they are sticky), so the container `scrollLeft` maps 1:1 + onto center-column offsets and must not be shifted by the pinned width. +- Focused-cell auto-scroll runs at most once per focused-cell change. The + focused column can be outside the mounted window (its cell element does not + exist), so retrying until the element appears would re-apply the computed + scroll position on every scroll update and fight user scrolling. Vertical + reveal falls back to any rendered cell of the focused row because rows are + never virtualized. + ## Rollout Plan 1. Implement column virtualization for center (non-pinned) columns. diff --git a/FEATURES.md b/FEATURES.md index 2acaa70e..e4732c6f 100644 --- a/FEATURES.md +++ b/FEATURES.md @@ -172,6 +172,7 @@ Table data is shown in a grid with server-backed pagination, filtered-row counts The footer keeps page navigation, a page jump field, a fixed rows-per-page dropdown, and infinite-scroll mode in one compact control group, so users can either jump directly to a page, switch page density from a known preset, or turn on lazy-loading without leaving the grid. Rows-per-page and infinite-scroll preferences persist across tables through local storage, while the known filtered-row count keeps the footer stable during page transitions for the same filtered result set. Infinite scroll preloads before the hard bottom edge, always appends in fixed 25-row chunks regardless of the paginated page-size setting, keeps filling tall viewports until the grid is actually scrollable, and appends new rows in place without snapping the grid back to the top. Rapid sort and filter changes keep the latest request authoritative, and superseded table reads are aborted so a slower older result cannot overwrite the visible grid. +Wide tables virtualize their non-pinned columns so only the columns near the viewport are mounted. The virtualization window follows horizontal scrolling synchronously and grid re-renders only happen when the set of mounted columns actually changes, which keeps horizontal scrolling smooth, makes the last column reachable, and lets cell focus changes reveal off-screen columns without fighting user-initiated scrolling. ## PostgreSQL Stored Temporal Values diff --git a/ui/studio/grid/DataGrid.interactions.test.tsx b/ui/studio/grid/DataGrid.interactions.test.tsx index bd166daf..702e1f5d 100644 --- a/ui/studio/grid/DataGrid.interactions.test.tsx +++ b/ui/studio/grid/DataGrid.interactions.test.tsx @@ -693,6 +693,101 @@ describe("DataGrid interactions", () => { } }); + it("auto-scrolls once and never fights user scrolling when the focused column is outside the virtualization window", async () => { + const clientWidthDescriptor = Object.getOwnPropertyDescriptor( + HTMLElement.prototype, + "clientWidth", + ); + + Object.defineProperty(HTMLElement.prototype, "clientWidth", { + configurable: true, + get() { + return 400; + }, + }); + + let cleanupGrid: (() => void) | null = null; + + try { + const columnIds = Array.from( + { length: 20 }, + (_, index) => `col_${index}`, + ); + const row: GridRow = { __ps_rowid: "row_1" }; + + for (const columnId of columnIds) { + row[columnId] = `${columnId} value`; + } + + const { cleanup, container, setFocusedCell } = renderGrid({ + columnDefs: createReadOnlyColumns({ columnIds }), + focusedCell: null, + manageFocusedCell: true, + rows: [row], + }); + cleanupGrid = cleanup; + + const scrollContainer = container.querySelector( + '[data-grid-scroll-container="true"]', + ); + + if (!(scrollContainer instanceof HTMLDivElement)) { + throw new Error("Could not find table scroll container"); + } + + await flushMicrotasks(); + + // The virtualization window at scrollLeft 0 with a 400px viewport only + // renders the first few 200px columns, so col_10 has no cell element. + expect( + container.querySelector('td[data-grid-column-id="col_10"]'), + ).toBeNull(); + + // Focus a cell whose element never renders: the column sits outside + // the virtualization window and the visual row index is not on the + // current page. + setFocusedCell({ + columnId: "col_10", + rowIndex: 5, + }); + await flushMicrotasks(); + + // Focusing the off-window column auto-scrolls once to reveal it: + // columnEnd (11 * 200) minus the 400px viewport. + expect(scrollContainer.scrollLeft).toBe(1800); + + act(() => { + scrollContainer.scrollLeft = 2600; + scrollContainer.dispatchEvent(new Event("scroll")); + }); + await flushMicrotasks(); + + // Subsequent user scrolling must win even though the focused cell was + // not rendered when the auto-scroll ran. + expect(scrollContainer.scrollLeft).toBe(2600); + + act(() => { + scrollContainer.scrollLeft = 3600; + scrollContainer.dispatchEvent(new Event("scroll")); + }); + await flushMicrotasks(); + + expect(scrollContainer.scrollLeft).toBe(3600); + } finally { + cleanupGrid?.(); + + if (clientWidthDescriptor) { + Object.defineProperty( + HTMLElement.prototype, + "clientWidth", + clientWidthDescriptor, + ); + } else { + Reflect.deleteProperty(HTMLElement.prototype, "clientWidth"); + } + } + }); + it("loads more rows when infinite scroll reaches the bottom threshold", async () => { const onLoadMoreRows = vi.fn(); const { cleanup, container } = renderGrid({ diff --git a/ui/studio/grid/DataGrid.tsx b/ui/studio/grid/DataGrid.tsx index 6a611298..8c93e798 100644 --- a/ui/studio/grid/DataGrid.tsx +++ b/ui/studio/grid/DataGrid.tsx @@ -82,7 +82,12 @@ import { DEFAULT_GRID_COLUMN_SIZE, resolveColumnSizingStateUpdate, } from "./column-sizing"; -import { computeColumnVirtualizationWindow } from "./column-virtualization"; +import { + type ColumnVirtualizationWindow, + columnVirtualizationWindowsAreEqual, + computeColumnVirtualizationWindow, + DISABLED_COLUMN_VIRTUALIZATION_WINDOW, +} from "./column-virtualization"; import { DataGridLoadingBar } from "./DataGridLoadingBar"; import { DataGridPagination } from "./DataGridPagination"; import { getColumnPinningStyles } from "./features/column-pinning"; @@ -1048,9 +1053,16 @@ export function DataGrid(props: DataGridProps) { }, }); const sensors = useSensors(mouseSensor); - const [centerViewport, setCenterViewport] = useState({ - scrollLeft: 0, - width: 0, + const [centerColumnWindow, setCenterColumnWindow] = + useState(DISABLED_COLUMN_VIRTUALIZATION_WINDOW); + const centerVirtualizationInputsRef = useRef<{ + columnWidths: number[]; + leftPinnedWidth: number; + rightPinnedWidth: number; + }>({ + columnWidths: [], + leftPinnedWidth: 0, + rightPinnedWidth: 0, }); const [contextMenuTarget, setContextMenuTarget] = useState(null); @@ -1194,21 +1206,59 @@ export function DataGrid(props: DataGridProps) { const rightPinnedWidth = table .getRightVisibleLeafColumns() .reduce((total, column) => total + column.getSize(), 0); - const centerViewportWidth = Math.max( - 0, - centerViewport.width - leftPinnedWidth - rightPinnedWidth, - ); - const centerViewportScrollLeft = Math.max( - 0, - centerViewport.scrollLeft - leftPinnedWidth, - ); - const centerColumnWindow = computeColumnVirtualizationWindow({ - columnWidths: centerVisibleLeafColumns.map((column) => column.getSize()), - minColumnCount: COLUMN_VIRTUALIZATION_MIN_COLUMN_COUNT, - overscanPx: COLUMN_VIRTUALIZATION_OVERSCAN_PX, - scrollLeft: centerViewportScrollLeft, - viewportWidth: centerViewportWidth, - }); + const centerVirtualizationInputsKey = `${centerVisibleLeafColumns + .map((column) => `${column.id}:${column.getSize()}`) + .join("|")}|left:${leftPinnedWidth}|right:${rightPinnedWidth}`; + + // Recomputes the center-column virtualization window from the live scroll + // position. State only changes when the window itself changes, so plain + // scrolling inside the overscan area never re-renders the grid. + const recomputeCenterColumnWindow = useCallback(() => { + const scrollContainer = tableRef.current?.parentElement; + + if (!scrollContainer) { + return; + } + + const inputs = centerVirtualizationInputsRef.current; + // The virtualization window is computed in center-column coordinates. + // Left-pinned columns occupy the start of the scrollable row and overlay + // the same amount of viewport width (they are sticky), so the container + // scrollLeft maps 1:1 onto the center-column offset space. + const nextWindow = computeColumnVirtualizationWindow({ + columnWidths: inputs.columnWidths, + minColumnCount: COLUMN_VIRTUALIZATION_MIN_COLUMN_COUNT, + overscanPx: COLUMN_VIRTUALIZATION_OVERSCAN_PX, + scrollLeft: Math.max(0, scrollContainer.scrollLeft), + viewportWidth: Math.max( + 0, + scrollContainer.clientWidth - + inputs.leftPinnedWidth - + inputs.rightPinnedWidth, + ), + }); + + setCenterColumnWindow((current) => + columnVirtualizationWindowsAreEqual(current, nextWindow) + ? current + : nextWindow, + ); + }, []); + + useLayoutEffect(() => { + centerVirtualizationInputsRef.current = { + columnWidths: table + .getCenterVisibleLeafColumns() + .map((column) => column.getSize()), + leftPinnedWidth: table + .getLeftVisibleLeafColumns() + .reduce((total, column) => total + column.getSize(), 0), + rightPinnedWidth: table + .getRightVisibleLeafColumns() + .reduce((total, column) => total + column.getSize(), 0), + }; + recomputeCenterColumnWindow(); + }, [centerVirtualizationInputsKey, recomputeCenterColumnWindow, table]); const visibleLeafColumns = table.getVisibleLeafColumns(); const selectableColumnIds = useMemo(() => { @@ -1757,6 +1807,13 @@ export function DataGrid(props: DataGridProps) { return; } + // Mark the focused cell as handled before scrolling so the auto-scroll + // runs at most once per focused-cell change. Retrying while the focused + // column is outside the virtualization window (its cell element is not + // rendered) would re-apply the computed scrollLeft on every scroll update + // and fight user-initiated scrolling. + autoScrolledFocusedCellRef.current = focusedCell; + const tableElement = tableRef.current; const scrollContainer = tableElement?.parentElement; @@ -1770,9 +1827,24 @@ export function DataGrid(props: DataGridProps) { return; } + const currentLeftPinnedWidth = table + .getLeftVisibleLeafColumns() + .reduce((total, column) => total + column.getSize(), 0); + const currentRightPinnedWidth = table + .getRightVisibleLeafColumns() + .reduce((total, column) => total + column.getSize(), 0); + const currentCenterViewportWidth = Math.max( + 0, + scrollContainer.clientWidth - + currentLeftPinnedWidth - + currentRightPinnedWidth, + ); let nextScrollLeft: number | null = null; - if (focusedColumn.getIsPinned() === false && centerViewportWidth > 0) { + if ( + focusedColumn.getIsPinned() === false && + currentCenterViewportWidth > 0 + ) { const centerColumns = table.getCenterVisibleLeafColumns(); const currentCenterScrollLeft = Math.max(0, scrollContainer.scrollLeft); const nextCenterScrollLeft = getFocusedCellScrollLeft({ @@ -1780,7 +1852,7 @@ export function DataGrid(props: DataGridProps) { columnWidths: centerColumns.map((column) => column.getSize()), currentScrollLeft: currentCenterScrollLeft, focusedColumnId: focusedCell.columnId, - viewportWidth: centerViewportWidth, + viewportWidth: currentCenterViewportWidth, }); nextScrollLeft = Math.max(0, nextCenterScrollLeft); @@ -1790,14 +1862,19 @@ export function DataGrid(props: DataGridProps) { } } - const focusedCellElement = Array.from( + // The focused column may be outside the rendered virtualization window. + // Rows are never virtualized, so fall back to any rendered cell in the + // focused row to keep the vertical scroll-into-view behavior. + const focusedRowCells = Array.from( scrollContainer.querySelectorAll( `td[data-grid-visual-row-index="${focusedCell.rowIndex}"][data-grid-column-id]`, ), - ).find( - (cellElement) => - cellElement.dataset.gridColumnId === focusedCell.columnId, ); + const focusedCellElement = + focusedRowCells.find( + (cellElement) => + cellElement.dataset.gridColumnId === focusedCell.columnId, + ) ?? focusedRowCells[0]; if (!focusedCellElement) { return; @@ -1812,16 +1889,7 @@ export function DataGrid(props: DataGridProps) { scrollContainer.scrollLeft = nextScrollLeft; scrollContainer.dispatchEvent(new Event("scroll")); } - - autoScrolledFocusedCellRef.current = focusedCell; - }, [ - centerViewport.scrollLeft, - centerViewportWidth, - focusedCell, - leftPinnedWidth, - rightPinnedWidth, - table, - ]); + }, [focusedCell, table]); useEffect(() => { const tableElement = tableRef.current; @@ -1831,67 +1899,32 @@ export function DataGrid(props: DataGridProps) { return; } - let animationFrameId: number | null = null; - - const updateViewport = () => { - animationFrameId = null; - - const nextScrollLeft = scrollContainer.scrollLeft; - const nextWidth = scrollContainer.clientWidth; - - setCenterViewport((current) => { - if ( - current.scrollLeft === nextScrollLeft && - current.width === nextWidth - ) { - return current; - } - - return { - scrollLeft: nextScrollLeft, - width: nextWidth, - }; - }); + // Recompute synchronously on scroll. Scroll events already fire at most + // once per frame, and deferring the window update behind an animation + // frame plus a state update lets fast scrolling outrun the overscan area, + // which shows up as blank columns and jumpy repaints. + const handleViewportChange = () => { + recomputeCenterColumnWindow(); }; - const scheduleViewportUpdate = () => { - if (animationFrameId !== null) { - return; - } - - if (typeof window.requestAnimationFrame !== "function") { - updateViewport(); - return; - } - - animationFrameId = window.requestAnimationFrame(updateViewport); - }; - - scheduleViewportUpdate(); - scrollContainer.addEventListener("scroll", scheduleViewportUpdate, { + handleViewportChange(); + scrollContainer.addEventListener("scroll", handleViewportChange, { passive: true, }); - window.addEventListener("resize", scheduleViewportUpdate); + window.addEventListener("resize", handleViewportChange); let resizeObserver: ResizeObserver | null = null; if (typeof ResizeObserver !== "undefined") { - resizeObserver = new ResizeObserver(scheduleViewportUpdate); + resizeObserver = new ResizeObserver(handleViewportChange); resizeObserver.observe(scrollContainer); } return () => { - if ( - animationFrameId !== null && - typeof window.cancelAnimationFrame === "function" - ) { - window.cancelAnimationFrame(animationFrameId); - } - - scrollContainer.removeEventListener("scroll", scheduleViewportUpdate); - window.removeEventListener("resize", scheduleViewportUpdate); + scrollContainer.removeEventListener("scroll", handleViewportChange); + window.removeEventListener("resize", handleViewportChange); resizeObserver?.disconnect(); }; - }, [columnDefinitionIdentityKey]); + }, [columnDefinitionIdentityKey, recomputeCenterColumnWindow]); useEffect(() => { const tableElement = tableRef.current; diff --git a/ui/studio/grid/DataGrid.virtualization.test.tsx b/ui/studio/grid/DataGrid.virtualization.test.tsx index a381ed16..e83d6f3c 100644 --- a/ui/studio/grid/DataGrid.virtualization.test.tsx +++ b/ui/studio/grid/DataGrid.virtualization.test.tsx @@ -10,7 +10,8 @@ import { computeColumnVirtualizationWindow } from "./column-virtualization"; import { DataGrid } from "./DataGrid"; import { createReadOnlyColumns, defaultRows } from "./test-utils"; -vi.mock("./column-virtualization", () => ({ +vi.mock("./column-virtualization", async (importOriginal) => ({ + ...(await importOriginal()), computeColumnVirtualizationWindow: vi.fn(), })); @@ -167,6 +168,60 @@ describe("DataGrid column virtualization", () => { cleanup(); }); + it("updates the virtualization window synchronously on scroll without animation frames", () => { + const requestAnimationFrameSpy = vi + .spyOn(window, "requestAnimationFrame") + .mockImplementation(() => 0); + + vi.mocked(computeColumnVirtualizationWindow).mockImplementation( + ({ scrollLeft }) => + scrollLeft >= 500 + ? { + enabled: true, + startIndex: 2, + endIndex: 2, + hiddenStartCount: 2, + hiddenEndCount: 0, + hiddenStartWidth: 400, + hiddenEndWidth: 0, + } + : { + enabled: true, + startIndex: 0, + endIndex: 0, + hiddenStartCount: 0, + hiddenEndCount: 2, + hiddenStartWidth: 0, + hiddenEndWidth: 400, + }, + ); + + const { cleanup, container, scrollContainer } = renderGrid(); + + expect( + container.querySelector('td[data-grid-column-id="id"]'), + ).not.toBeNull(); + expect( + container.querySelector('td[data-grid-column-id="title"]'), + ).toBeNull(); + + act(() => { + scrollContainer.scrollLeft = 600; + scrollContainer.dispatchEvent(new Event("scroll")); + }); + + // The window must follow the scroll position even though no animation + // frame callback ever ran; deferring the update behind rAF lets fast + // scrolling outrun the rendered window. + expect(container.querySelector('td[data-grid-column-id="id"]')).toBeNull(); + expect( + container.querySelector('td[data-grid-column-id="title"]'), + ).not.toBeNull(); + + requestAnimationFrameSpy.mockRestore(); + cleanup(); + }); + it("passes column widths and virtualization constants into the window computation", () => { vi.mocked(computeColumnVirtualizationWindow).mockReturnValue({ enabled: false, diff --git a/ui/studio/grid/column-virtualization.test.ts b/ui/studio/grid/column-virtualization.test.ts index 170c9289..16d5948f 100644 --- a/ui/studio/grid/column-virtualization.test.ts +++ b/ui/studio/grid/column-virtualization.test.ts @@ -1,6 +1,10 @@ import { describe, expect, it } from "vitest"; -import { computeColumnVirtualizationWindow } from "./column-virtualization"; +import { + columnVirtualizationWindowsAreEqual, + computeColumnVirtualizationWindow, + DISABLED_COLUMN_VIRTUALIZATION_WINDOW, +} from "./column-virtualization"; describe("computeColumnVirtualizationWindow", () => { it("returns an empty disabled window when there are no columns", () => { @@ -183,3 +187,74 @@ describe("computeColumnVirtualizationWindow", () => { }); }); }); + +describe("computeColumnVirtualizationWindow at the maximum scroll position", () => { + it("keeps the last column inside the window so the end of the grid is reachable", () => { + const columnWidths = Array.from({ length: 61 }, () => 200); + const totalWidth = 61 * 200; + const viewportWidth = 1027; + + const window = computeColumnVirtualizationWindow({ + columnWidths, + minColumnCount: 16, + overscanPx: 320, + scrollLeft: totalWidth - viewportWidth, + viewportWidth, + }); + + expect(window.endIndex).toBe(60); + expect(window.hiddenEndCount).toBe(0); + expect(window.hiddenEndWidth).toBe(0); + expect( + window.hiddenStartWidth + 200 * (window.endIndex - window.startIndex + 1), + ).toBe(totalWidth); + }); +}); + +describe("columnVirtualizationWindowsAreEqual", () => { + it("returns true for identical windows", () => { + const window = { + enabled: true, + startIndex: 2, + endIndex: 8, + hiddenStartCount: 2, + hiddenEndCount: 4, + hiddenStartWidth: 400, + hiddenEndWidth: 800, + }; + + expect(columnVirtualizationWindowsAreEqual(window, { ...window })).toBe( + true, + ); + expect( + columnVirtualizationWindowsAreEqual( + DISABLED_COLUMN_VIRTUALIZATION_WINDOW, + { ...DISABLED_COLUMN_VIRTUALIZATION_WINDOW }, + ), + ).toBe(true); + }); + + it("returns false when any window field differs", () => { + const window = { + enabled: true, + startIndex: 2, + endIndex: 8, + hiddenStartCount: 2, + hiddenEndCount: 4, + hiddenStartWidth: 400, + hiddenEndWidth: 800, + }; + + for (const key of Object.keys(window) as (keyof typeof window)[]) { + const changed = { ...window }; + + if (key === "enabled") { + changed.enabled = !changed.enabled; + } else { + changed[key] = changed[key] + 1; + } + + expect(columnVirtualizationWindowsAreEqual(window, changed)).toBe(false); + } + }); +}); diff --git a/ui/studio/grid/column-virtualization.ts b/ui/studio/grid/column-virtualization.ts index fe0323c4..1187e2f1 100644 --- a/ui/studio/grid/column-virtualization.ts +++ b/ui/studio/grid/column-virtualization.ts @@ -8,6 +8,32 @@ export interface ColumnVirtualizationWindow { hiddenEndWidth: number; } +export const DISABLED_COLUMN_VIRTUALIZATION_WINDOW: ColumnVirtualizationWindow = + { + enabled: false, + startIndex: 0, + endIndex: -1, + hiddenStartCount: 0, + hiddenEndCount: 0, + hiddenStartWidth: 0, + hiddenEndWidth: 0, + }; + +export function columnVirtualizationWindowsAreEqual( + left: ColumnVirtualizationWindow, + right: ColumnVirtualizationWindow, +): boolean { + return ( + left.enabled === right.enabled && + left.startIndex === right.startIndex && + left.endIndex === right.endIndex && + left.hiddenStartCount === right.hiddenStartCount && + left.hiddenEndCount === right.hiddenEndCount && + left.hiddenStartWidth === right.hiddenStartWidth && + left.hiddenEndWidth === right.hiddenEndWidth + ); +} + export interface ComputeColumnVirtualizationWindowArgs { columnWidths: number[]; minColumnCount: number; From 55041992b1ca65fe91915d439f559c6de9167092 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20Bramer=20Schmidt?= Date: Sat, 18 Jul 2026 15:05:19 +0700 Subject: [PATCH 2/2] fix: address PR review feedback on virtualization key and pending focus - Serialize the center-column virtualization inputs key structurally with JSON.stringify instead of delimiter joining, so column ids containing the delimiter characters cannot produce colliding keys. - Keep a focused-cell auto-scroll pending while its prerequisites are missing (no scroll container, unknown column, or an unmeasured/hidden grid) instead of marking it handled up-front. Column changes and a new viewport-readiness tick re-trigger the effect when layout becomes ready; neither signal changes on plain scrolling, so the at-most-once-per-focus guarantee against fighting user scrolling is preserved. Co-Authored-By: Claude Fable 5 --- ui/studio/grid/DataGrid.interactions.test.tsx | 89 +++++++++++++++++++ ui/studio/grid/DataGrid.tsx | 50 ++++++++--- 2 files changed, 128 insertions(+), 11 deletions(-) diff --git a/ui/studio/grid/DataGrid.interactions.test.tsx b/ui/studio/grid/DataGrid.interactions.test.tsx index 702e1f5d..18d380b2 100644 --- a/ui/studio/grid/DataGrid.interactions.test.tsx +++ b/ui/studio/grid/DataGrid.interactions.test.tsx @@ -788,6 +788,95 @@ describe("DataGrid interactions", () => { } }); + it("runs the focused-cell auto-scroll once the grid becomes measurable after being hidden", async () => { + const clientWidthDescriptor = Object.getOwnPropertyDescriptor( + HTMLElement.prototype, + "clientWidth", + ); + let mockedClientWidth = 0; + + Object.defineProperty(HTMLElement.prototype, "clientWidth", { + configurable: true, + get() { + return mockedClientWidth; + }, + }); + + let cleanupGrid: (() => void) | null = null; + + try { + const columnIds = Array.from( + { length: 20 }, + (_, index) => `col_${index}`, + ); + const row: GridRow = { __ps_rowid: "row_1" }; + + for (const columnId of columnIds) { + row[columnId] = `${columnId} value`; + } + + const { cleanup, container, setFocusedCell } = renderGrid({ + columnDefs: createReadOnlyColumns({ columnIds }), + focusedCell: null, + manageFocusedCell: true, + rows: [row], + }); + cleanupGrid = cleanup; + + const scrollContainer = container.querySelector( + '[data-grid-scroll-container="true"]', + ); + + if (!(scrollContainer instanceof HTMLDivElement)) { + throw new Error("Could not find table scroll container"); + } + + await flushMicrotasks(); + + // Focus arrives while the grid is hidden (clientWidth 0), so the + // auto-scroll cannot run yet and must stay pending. + setFocusedCell({ + columnId: "col_10", + rowIndex: 0, + }); + await flushMicrotasks(); + + expect(scrollContainer.scrollLeft).toBe(0); + + // The grid becomes visible and layout observers fire. + mockedClientWidth = 400; + act(() => { + window.dispatchEvent(new Event("resize")); + }); + await flushMicrotasks(); + + // The pending focused-cell auto-scroll now runs exactly once: + // columnEnd (11 * 200) minus the 400px viewport. + expect(scrollContainer.scrollLeft).toBe(1800); + + // Subsequent user scrolling still wins. + act(() => { + scrollContainer.scrollLeft = 2600; + scrollContainer.dispatchEvent(new Event("scroll")); + }); + await flushMicrotasks(); + + expect(scrollContainer.scrollLeft).toBe(2600); + } finally { + cleanupGrid?.(); + + if (clientWidthDescriptor) { + Object.defineProperty( + HTMLElement.prototype, + "clientWidth", + clientWidthDescriptor, + ); + } else { + Reflect.deleteProperty(HTMLElement.prototype, "clientWidth"); + } + } + }); + it("loads more rows when infinite scroll reaches the bottom threshold", async () => { const onLoadMoreRows = vi.fn(); const { cleanup, container } = renderGrid({ diff --git a/ui/studio/grid/DataGrid.tsx b/ui/studio/grid/DataGrid.tsx index 8c93e798..e3a8405a 100644 --- a/ui/studio/grid/DataGrid.tsx +++ b/ui/studio/grid/DataGrid.tsx @@ -1064,6 +1064,8 @@ export function DataGrid(props: DataGridProps) { leftPinnedWidth: 0, rightPinnedWidth: 0, }); + const viewportMeasurableRef = useRef(false); + const [viewportReadyTick, setViewportReadyTick] = useState(0); const [contextMenuTarget, setContextMenuTarget] = useState(null); const [activeColumnDragState, setActiveColumnDragState] = @@ -1206,9 +1208,11 @@ export function DataGrid(props: DataGridProps) { const rightPinnedWidth = table .getRightVisibleLeafColumns() .reduce((total, column) => total + column.getSize(), 0); - const centerVirtualizationInputsKey = `${centerVisibleLeafColumns - .map((column) => `${column.id}:${column.getSize()}`) - .join("|")}|left:${leftPinnedWidth}|right:${rightPinnedWidth}`; + const centerVirtualizationInputsKey = JSON.stringify([ + centerVisibleLeafColumns.map((column) => [column.id, column.getSize()]), + leftPinnedWidth, + rightPinnedWidth, + ]); // Recomputes the center-column virtualization window from the live scroll // position. State only changes when the window itself changes, so plain @@ -1220,6 +1224,19 @@ export function DataGrid(props: DataGridProps) { return; } + // Track when the grid gains a measurable viewport (e.g. a hidden tab + // becomes visible). Scrolling never changes this, so the tick only + // re-triggers effects on layout-readiness transitions. + const isViewportMeasurable = scrollContainer.clientWidth > 0; + + if (isViewportMeasurable !== viewportMeasurableRef.current) { + viewportMeasurableRef.current = isViewportMeasurable; + + if (isViewportMeasurable) { + setViewportReadyTick((current) => current + 1); + } + } + const inputs = centerVirtualizationInputsRef.current; // The virtualization window is computed in center-column coordinates. // Left-pinned columns occupy the start of the scrollable row and overlay @@ -1807,16 +1824,13 @@ export function DataGrid(props: DataGridProps) { return; } - // Mark the focused cell as handled before scrolling so the auto-scroll - // runs at most once per focused-cell change. Retrying while the focused - // column is outside the virtualization window (its cell element is not - // rendered) would re-apply the computed scrollLeft on every scroll update - // and fight user-initiated scrolling. - autoScrolledFocusedCellRef.current = focusedCell; - const tableElement = tableRef.current; const scrollContainer = tableElement?.parentElement; + // Leave the focused cell unhandled while its prerequisites are missing + // (no scroll container, unknown column, or a hidden/unmeasured grid), so + // the auto-scroll still runs once column and layout readiness re-trigger + // this effect. if (!(scrollContainer instanceof HTMLDivElement)) { return; } @@ -1827,6 +1841,17 @@ export function DataGrid(props: DataGridProps) { return; } + if (scrollContainer.clientWidth <= 0) { + return; + } + + // Mark the focused cell as handled before scrolling so the auto-scroll + // runs at most once per focused-cell change. Retrying while the focused + // column is outside the virtualization window (its cell element is not + // rendered) would re-apply the computed scrollLeft on every scroll update + // and fight user-initiated scrolling. + autoScrolledFocusedCellRef.current = focusedCell; + const currentLeftPinnedWidth = table .getLeftVisibleLeafColumns() .reduce((total, column) => total + column.getSize(), 0); @@ -1889,7 +1914,10 @@ export function DataGrid(props: DataGridProps) { scrollContainer.scrollLeft = nextScrollLeft; scrollContainer.dispatchEvent(new Event("scroll")); } - }, [focusedCell, table]); + // centerVirtualizationInputsKey and viewportReadyTick re-trigger this + // effect when columns or the grid layout become ready; neither changes on + // plain scrolling, so user scrolling can never re-run the auto-scroll. + }, [centerVirtualizationInputsKey, focusedCell, table, viewportReadyTick]); useEffect(() => { const tableElement = tableRef.current;