From b976b97a843b6bde0fe8f063ec2d2eea94fdd1a5 Mon Sep 17 00:00:00 2001 From: Dave Page Date: Wed, 19 Aug 2026 13:12:46 +0100 Subject: [PATCH 1/2] 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/2] 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}`} ));