From b976b97a843b6bde0fe8f063ec2d2eea94fdd1a5 Mon Sep 17 00:00:00 2001 From: Dave Page Date: Wed, 19 Aug 2026 13:12:46 +0100 Subject: [PATCH 1/3] fix: avoid re-measuring every row when a hidden DataGridView tab is shown (#10143) --- .../js/SchemaView/DataGridView/grid.jsx | 42 ++++++++++++++----- .../js/components/PgReactTableStyled.jsx | 8 ++++ .../SchemaView/SchemaDialogView.spec.js | 35 ++++++++++++++++ 3 files changed, 74 insertions(+), 11 deletions(-) diff --git a/web/pgadmin/static/js/SchemaView/DataGridView/grid.jsx b/web/pgadmin/static/js/SchemaView/DataGridView/grid.jsx index 8366cd44ccd..d497c90811e 100644 --- a/web/pgadmin/static/js/SchemaView/DataGridView/grid.jsx +++ b/web/pgadmin/static/js/SchemaView/DataGridView/grid.jsx @@ -122,12 +122,25 @@ export default function DataGridView({ ) ).includes(true); + // Virtualising a small grid buys nothing (there's no offscreen window to + // skip rendering) but still pays for measureElement's per-row + // getBoundingClientRect on every mount/remeasure. That remeasure is + // exactly what fires when a dialog tab holding the grid is hidden via + // `display: none` and then shown again, since the scroll viewport + // momentarily measures 0 and the virtualizer's ResizeObserver treats + // that as a real resize. Below the threshold we skip virtualisation + // entirely and render every row in normal document flow, so showing a + // hidden tab is a pure CSS toggle again. + const virtualiseThreshold = viewHelperProps.virtualiseThreshold ?? 100; + const shouldVirtualise = rows.length > virtualiseThreshold; + const virtualizer = useVirtualizer({ count: rows.length, getScrollElement: () => tableEleRef.current, estimateSize: () => 50, measureElement: - typeof window !== 'undefined' && + shouldVirtualise && + typeof window !== 'undefined' && navigator.userAgent.indexOf('Firefox') === -1 ? element => element?.getBoundingClientRect().height : undefined, @@ -152,22 +165,29 @@ export default function DataGridView({ ref={tableEleRef} table={table} data-test="data-grid-view" tableClassName='DataGridView-table'> - + { - virtualizer.getVirtualItems().map((virtualRow) => { + ( + shouldVirtualise + ? virtualizer.getVirtualItems() + : rows.map((_row, index) => ({index, start: 0})) + ).map((virtualRow) => { const row = rows[virtualRow.index]; return ( virtualizer.measureElement(node)} - style={{ - // This should always be a `style` as it changes on - // scroll. - transform: `translateY(${virtualRow.start}px)`, - }} + ref={shouldVirtualise ? node => virtualizer.measureElement(node) : undefined} + className={shouldVirtualise ? undefined : 'pgrt-row--static'} + style={ + shouldVirtualise ? { + // This should always be a `style` as it changes + // on scroll. + transform: `translateY(${virtualRow.start}px)`, + } : undefined + } > ({ position: 'absolute', width: '100%', + // Opted out of the virtualizer's absolute positioning for grids + // small enough that virtualisation isn't used. Keeps the row in + // normal document flow so a hidden/shown dialog tab is a pure CSS + // toggle instead of triggering a virtualizer remeasure. + '&.pgrt-row--static': { + position: 'static', + }, + '& .pgrt-row-content': { display: 'flex', minHeight: 0, diff --git a/web/regression/javascript/SchemaView/SchemaDialogView.spec.js b/web/regression/javascript/SchemaView/SchemaDialogView.spec.js index 9c4c944138e..fbfad366241 100644 --- a/web/regression/javascript/SchemaView/SchemaDialogView.spec.js +++ b/web/regression/javascript/SchemaView/SchemaDialogView.spec.js @@ -172,6 +172,41 @@ describe('SchemaView', ()=>{ await user.type(ctrl.container.querySelectorAll('[name="field5"]')[1], 'rval51'); expect(ctrl.container.querySelector('[data-test="notifier-message"]')).toHaveTextContent('Field5 in FieldColl must be unique.'); }); + + it('does not virtualise a small grid, rendering rows in static flow', async ()=>{ + await simulateValidData(); + + const dataRows = ctrl.container.querySelectorAll('[data-test="data-table-row"]'); + expect(dataRows.length).toBe(2); + + // Every row should be fully mounted and opted out of the + // virtualizer's absolute positioning, so a hidden dialog tab is a + // pure CSS toggle rather than something the virtualizer has to + // remeasure when the tab is shown again. + const pgrtRows = ctrl.container.querySelectorAll('.pgrt-row'); + expect(pgrtRows.length).toBe(2); + pgrtRows.forEach((rowEl)=>{ + expect(rowEl.classList.contains('pgrt-row--static')).toBe(true); + expect(rowEl.style.transform).toBe(''); + }); + }); + + it('virtualises a large grid, mounting only a window of rows', async ()=>{ + const manyRows = Array.from({length: 150}, (_, i)=>( + {field3: i, field4: 'field4val', field5: `field5val${i}`} + )); + + await ctrlMount({ + getInitData: ()=>Promise.resolve({fieldcoll: manyRows}), + }); + + const pgrtRows = ctrl.container.querySelectorAll('.pgrt-row'); + expect(pgrtRows.length).toBeGreaterThan(0); + expect(pgrtRows.length).toBeLessThan(manyRows.length); + pgrtRows.forEach((rowEl)=>{ + expect(rowEl.classList.contains('pgrt-row--static')).toBe(false); + }); + }); }); describe('SQL tab', ()=>{ From df1260279786f727dff2c6dedb4868abc3d7fe76 Mon Sep 17 00:00:00 2001 From: Dave Page Date: Thu, 27 Aug 2026 10:20:56 +0100 Subject: [PATCH 2/3] fix: scale DataGridView virtualisation threshold by visible column count Render cost tracks total cells (rows * cols), not row count alone, so a flat row threshold under-virtualises wide grids. Scale the default threshold by visible column count instead, clamped to [25, 400]. Formula and bounds adapted from VIBVEL47's independent fix for the same issue in #10146. --- web/pgadmin/static/js/SchemaView/DataGridView/grid.jsx | 10 +++++++++- .../javascript/SchemaView/SchemaDialogView.spec.js | 5 ++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/web/pgadmin/static/js/SchemaView/DataGridView/grid.jsx b/web/pgadmin/static/js/SchemaView/DataGridView/grid.jsx index d497c90811e..0724dd0b1ce 100644 --- a/web/pgadmin/static/js/SchemaView/DataGridView/grid.jsx +++ b/web/pgadmin/static/js/SchemaView/DataGridView/grid.jsx @@ -131,7 +131,15 @@ export default function DataGridView({ // that as a real resize. Below the threshold we skip virtualisation // entirely and render every row in normal document flow, so showing a // hidden tab is a pure CSS toggle again. - const virtualiseThreshold = viewHelperProps.virtualiseThreshold ?? 100; + // + // The threshold scales with visible column count rather than being a + // flat row count, since render cost tracks total cells (rows * cols), + // not rows alone: formula and bounds from VIBVEL47's PR #10146. + const visibleColCount = table.getVisibleLeafColumns().length; + const virtualiseThreshold = viewHelperProps.virtualiseThreshold ?? + (visibleColCount > 0 + ? Math.min(400, Math.max(25, Math.round(700 / visibleColCount))) + : 100); const shouldVirtualise = rows.length > virtualiseThreshold; const virtualizer = useVirtualizer({ diff --git a/web/regression/javascript/SchemaView/SchemaDialogView.spec.js b/web/regression/javascript/SchemaView/SchemaDialogView.spec.js index fbfad366241..19d0f0be89a 100644 --- a/web/regression/javascript/SchemaView/SchemaDialogView.spec.js +++ b/web/regression/javascript/SchemaView/SchemaDialogView.spec.js @@ -192,7 +192,10 @@ describe('SchemaView', ()=>{ }); it('virtualises a large grid, mounting only a window of rows', async ()=>{ - const manyRows = Array.from({length: 150}, (_, i)=>( + // The default threshold scales with visible column count (capped at + // 400), so this needs to comfortably clear that cap regardless of + // how many columns FieldColl renders. + const manyRows = Array.from({length: 450}, (_, i)=>( {field3: i, field4: 'field4val', field5: `field5val${i}`} )); From 38d6e911fd5d12c6d5b01ed8d248ae137becc8f3 Mon Sep 17 00:00:00 2001 From: Dave Page Date: Tue, 1 Sep 2026 11:51:49 +0100 Subject: [PATCH 3/3] Stop the virtualisation test timing out on the Windows runners 'virtualises a large grid, mounting only a window of rows' mounted 450 rows so as to clear the default threshold whatever the visible column count happened to be, which took around sixteen seconds on Linux and tipped over Jest's eighteen second budget on the Windows runners, failing the job and dragging the following test down with it through the act() warnings the aborted render left behind. The grid only needs enough rows to clear whatever threshold is in force, so the test now pins the threshold low through viewHelperProps and mounts sixty rows, which exercises the same windowing in about five seconds. The scaling of the default threshold is no longer incidental coverage of a slow DOM test: the formula is now getVirtualiseThreshold() in its own right, with tests covering the scaling, both bounds and the no-columns-yet case, and runs in a fraction of a second. --- .../js/SchemaView/DataGridView/grid.jsx | 22 ++++++++---- .../SchemaView/SchemaDialogView.spec.js | 10 +++--- .../SchemaView/getVirtualiseThreshold.spec.js | 34 +++++++++++++++++++ 3 files changed, 55 insertions(+), 11 deletions(-) create mode 100644 web/regression/javascript/SchemaView/getVirtualiseThreshold.spec.js diff --git a/web/pgadmin/static/js/SchemaView/DataGridView/grid.jsx b/web/pgadmin/static/js/SchemaView/DataGridView/grid.jsx index 0724dd0b1ce..0147ad79ca9 100644 --- a/web/pgadmin/static/js/SchemaView/DataGridView/grid.jsx +++ b/web/pgadmin/static/js/SchemaView/DataGridView/grid.jsx @@ -46,6 +46,20 @@ import { FeatureSet } from './features'; import { createGridColumns, GRID_STATE } from './utils'; +// The row count above which a grid is worth virtualising. It scales with +// the visible column count rather than being a flat row count, since +// render cost tracks total cells (rows * cols), not rows alone: formula +// and bounds from VIBVEL47's PR #10146. A grid reporting no columns yet +// gets a middling default rather than the 25 row floor, so that a grid +// still settling its columns is not virtualised on the strength of a +// momentary zero. +export function getVirtualiseThreshold(visibleColCount) { + if(!visibleColCount) return 100; + + return Math.min(400, Math.max(25, Math.round(700 / visibleColCount))); +} + + export default function DataGridView({ field, viewHelperProps, accessPath, dataDispatch, containerClassName }) { @@ -131,15 +145,9 @@ export default function DataGridView({ // that as a real resize. Below the threshold we skip virtualisation // entirely and render every row in normal document flow, so showing a // hidden tab is a pure CSS toggle again. - // - // The threshold scales with visible column count rather than being a - // flat row count, since render cost tracks total cells (rows * cols), - // not rows alone: formula and bounds from VIBVEL47's PR #10146. const visibleColCount = table.getVisibleLeafColumns().length; const virtualiseThreshold = viewHelperProps.virtualiseThreshold ?? - (visibleColCount > 0 - ? Math.min(400, Math.max(25, Math.round(700 / visibleColCount))) - : 100); + getVirtualiseThreshold(visibleColCount); const shouldVirtualise = rows.length > virtualiseThreshold; const virtualizer = useVirtualizer({ diff --git a/web/regression/javascript/SchemaView/SchemaDialogView.spec.js b/web/regression/javascript/SchemaView/SchemaDialogView.spec.js index 19d0f0be89a..fd4eb33c626 100644 --- a/web/regression/javascript/SchemaView/SchemaDialogView.spec.js +++ b/web/regression/javascript/SchemaView/SchemaDialogView.spec.js @@ -192,14 +192,16 @@ describe('SchemaView', ()=>{ }); it('virtualises a large grid, mounting only a window of rows', async ()=>{ - // The default threshold scales with visible column count (capped at - // 400), so this needs to comfortably clear that cap regardless of - // how many columns FieldColl renders. - const manyRows = Array.from({length: 450}, (_, i)=>( + // Clearing the default threshold would need several hundred rows, + // which is slow enough to time out on the Windows runners, so the + // threshold is pinned low instead and the scaling of the default + // is covered directly by getVirtualiseThreshold's own tests. + const manyRows = Array.from({length: 60}, (_, i)=>( {field3: i, field4: 'field4val', field5: `field5val${i}`} )); await ctrlMount({ + viewHelperProps: {mode: 'create', virtualiseThreshold: 25}, getInitData: ()=>Promise.resolve({fieldcoll: manyRows}), }); diff --git a/web/regression/javascript/SchemaView/getVirtualiseThreshold.spec.js b/web/regression/javascript/SchemaView/getVirtualiseThreshold.spec.js new file mode 100644 index 00000000000..77e55b1c2a9 --- /dev/null +++ b/web/regression/javascript/SchemaView/getVirtualiseThreshold.spec.js @@ -0,0 +1,34 @@ +///////////////////////////////////////////////////////////// +// +// pgAdmin 4 - PostgreSQL Tools +// +// Copyright (C) 2013 - 2026, The pgAdmin Development Team +// This software is released under the PostgreSQL Licence +// +////////////////////////////////////////////////////////////// + +import { getVirtualiseThreshold } + from 'sources/SchemaView/DataGridView/grid'; + +describe('getVirtualiseThreshold', ()=>{ + it('scales inversely with the visible column count', ()=>{ + expect(getVirtualiseThreshold(4)).toBe(175); + expect(getVirtualiseThreshold(7)).toBe(100); + expect(getVirtualiseThreshold(14)).toBe(50); + }); + + it('never goes above 400, however few the columns', ()=>{ + expect(getVirtualiseThreshold(1)).toBe(400); + expect(getVirtualiseThreshold(2)).toBe(350); + }); + + it('never goes below 25, however many the columns', ()=>{ + expect(getVirtualiseThreshold(28)).toBe(25); + expect(getVirtualiseThreshold(200)).toBe(25); + }); + + it('uses a middling default when no columns are reported yet', ()=>{ + expect(getVirtualiseThreshold(0)).toBe(100); + expect(getVirtualiseThreshold(undefined)).toBe(100); + }); +});