fix: consolidate virtual scroll height lock (WC-3333, WC-3390)

yordan-st · yordan-st · commit 02523661df31 · 2026-05-22T15:52:56.000+02:00
- Replace parallel lockedAtX guards with single layoutKey cache
- Keep old height alive on invalidation to prevent flicker
- Fix overflow detection: clientHeight not scrollHeight — many-column
  grids now correctly receive the synthetic scroll offset
- Guard bumpPage behind scrollTop &gt; 0
diff --git a/packages/pluggableWidgets/datagrid-web/src/model/containers/Datagrid.container.ts b/packages/pluggableWidgets/datagrid-web/src/model/containers/Datagrid.container.ts
@@ -79,7 +79,14 @@ const _01_coreBindings: BindingGroup = {
             DG.exportProgressService,
             SA_TOKENS.selectionDialogVM
         );
-        injected(GridSizeStore, CORE.atoms.hasMoreItems, DG.paginationConfig, DG.setPageAction, DG.pageSize);
+        injected(
+            GridSizeStore,
+            CORE.atoms.hasMoreItems,
+            DG.paginationConfig,
+            DG.setPageAction,
+            DG.pageSize,
+            CORE.atoms.visibleColumnsCount
+        );
     },
     define(container: Container) {
         container.bind(DG.basicDate).toInstance(GridBasicData).inSingletonScope();
diff --git a/packages/pluggableWidgets/datagrid-web/src/model/hooks/useInfiniteControl.tsx b/packages/pluggableWidgets/datagrid-web/src/model/hooks/useInfiniteControl.tsx
@@ -32,8 +32,9 @@ export function useInfiniteControl(): [trackTableScrolling: ((e: any) => void) |
              * causing mismatch by 1 pixel point, thus, add magic number 2 as buffer.
              */
             const bottom =
+                target.scrollTop > 0 &&
                 Math.floor(target.scrollHeight - VIRTUAL_SCROLLING_OFFSET - target.scrollTop) <=
-                Math.floor(target.clientHeight) + 2;
+                    Math.floor(target.clientHeight) + 2;
             if (bottom) {
                 gridSizeStore.bumpPage();
             }
diff --git a/packages/pluggableWidgets/datagrid-web/src/model/stores/GridSize.store.ts b/packages/pluggableWidgets/datagrid-web/src/model/stores/GridSize.store.ts
@@ -13,25 +13,30 @@ export class GridSizeStore {
 
     gridContainerHeight?: number;
 
-    private lockedAtPageSize?: number;
+    private lockedAtLayoutKey?: string;
 
     constructor(
         private readonly hasMoreItemsAtom: ComputedAtom<boolean | undefined>,
         private readonly paginationConfig: PaginationConfig,
         private readonly setPageAction: SetPageAction,
-        private readonly pageSizeAtom: ComputedAtom<number>
+        private readonly pageSizeAtom: ComputedAtom<number>,
+        private readonly visibleColumnsCountAtom: ComputedAtom<number>
     ) {
-        makeAutoObservable<GridSizeStore, "lockedAtPageSize">(this, {
+        makeAutoObservable<GridSizeStore, "lockedAtLayoutKey">(this, {
             gridContainerRef: false,
             gridBodyRef: false,
             gridHeaderRef: false,
-            lockedAtPageSize: false,
+            lockedAtLayoutKey: false,
 
             gridContainerHeight: observable,
             lockGridContainerHeight: action
         });
     }
 
+    private get layoutKey(): string {
+        return `${this.pageSizeAtom.get()}-${this.visibleColumnsCountAtom.get()}`;
+    }
+
     get hasMoreItems(): boolean {
         return this.hasMoreItemsAtom.get() ?? false;
     }
@@ -80,37 +85,33 @@ export class GridSizeStore {
             return;
         }
 
-        // Reset the locked height when page size changes so layout is recomputed
-        // for the new number of rows (e.g. switching from 10 → 5 rows).
-        const currentPageSize = this.pageSizeAtom.get();
-        if (this.gridContainerHeight !== undefined && this.lockedAtPageSize !== currentPageSize) {
-            this.gridContainerHeight = undefined;
-            this.lockedAtPageSize = undefined;
+        // Single cache key encodes all layout inputs. Any change (page size, column count,
+        // or future inputs) invalidates the lock in one place.
+        const currentKey = this.layoutKey;
+        if (this.lockedAtLayoutKey !== currentKey) {
+            this.lockedAtLayoutKey = undefined;
         }
 
         const gridContainer = this.gridContainerRef.current;
-        if (!gridContainer || this.gridContainerHeight !== undefined) {
+        if (!gridContainer || this.lockedAtLayoutKey !== undefined) {
             return;
         }
 
         const bodyViewportHeight = this.computeBodyViewport();
         const headerViewportHeight = this.computeHeaderViewport();
 
-        // Don't lock height before the grid body has rendered content.
-        // clientHeight is 0 when the element has no layout yet, which would
-        // produce a negative height and break scrolling.
+        // Don't lock before the grid body has rendered content — clientHeight is 0
+        // before layout, which would produce a negative height and break scrolling.
         if (bodyViewportHeight <= 0) {
             return;
         }
 
         const fullHeight = bodyViewportHeight + headerViewportHeight;
 
-        // If content already overflows the container (fixed-height grid), do not subtract the
-        // pre-fetch offset — that would hide the last rows and trigger the next page too early.
-        // Only subtract the offset when the grid does not yet overflow (auto-height grid) so
-        // that we create a small synthetic overflow that makes the body scrollable.
-        const overflows = gridContainer.scrollHeight > fullHeight;
+        // clientHeight is vertical-only (excludes horizontal scrollbar overflow).
+        // scrollHeight would be inflated by many-column grids and produce false positives.
+        const overflows = gridContainer.clientHeight < fullHeight;
         this.gridContainerHeight = fullHeight - (overflows ? 0 : VIRTUAL_SCROLLING_OFFSET);
-        this.lockedAtPageSize = currentPageSize;
+        this.lockedAtLayoutKey = currentKey;
     }
 }
diff --git a/packages/pluggableWidgets/datagrid-web/src/model/stores/__tests__/GridSize.store.spec.ts b/packages/pluggableWidgets/datagrid-web/src/model/stores/__tests__/GridSize.store.spec.ts
@@ -0,0 +1,191 @@
+import { MutableRefObject } from "react";
+import { computed, configure, observable } from "mobx";
+import { SetPageAction } from "@mendix/widget-plugin-grid/pagination/main";
+import { PaginationConfig } from "../../../features/pagination/pagination.config";
+import { GridSizeStore, VIRTUAL_SCROLLING_OFFSET } from "../GridSize.store";
+
+configure({ enforceActions: "never" });
+
+function makePaginationConfig(pagination: "virtualScrolling" | "buttons" = "virtualScrolling"): PaginationConfig {
+    return {
+        pagination,
+        pagingPosition: "bottom",
+        paginationKind: "virtualScrolling.always",
+        showPagingButtons: "always",
+        showNumberOfRows: false,
+        constPageSize: 10,
+        initPageSize: 10,
+        isLimitBased: false,
+        dynamicPageSizeEnabled: false,
+        dynamicPageEnabled: false,
+        customPaginationEnabled: false,
+        requestTotalCount: false
+    };
+}
+
+interface StoreOptions {
+    hasMoreItems?: boolean;
+    pageSize?: number;
+    columnCount?: number;
+    pagination?: "virtualScrolling" | "buttons";
+}
+
+function buildStore(options: StoreOptions = {}): {
+    store: GridSizeStore;
+    hasMoreItemsBox: ReturnType<typeof observable.box<boolean | undefined>>;
+    pageSizeBox: ReturnType<typeof observable.box<number>>;
+    columnCountBox: ReturnType<typeof observable.box<number>>;
+    setPageAction: jest.Mock;
+} {
+    const { hasMoreItems = true, pageSize = 10, columnCount = 3, pagination = "virtualScrolling" } = options;
+
+    const hasMoreItemsBox = observable.box<boolean | undefined>(hasMoreItems);
+    const pageSizeBox = observable.box(pageSize);
+    const columnCountBox = observable.box(columnCount);
+    const setPageAction = jest.fn() as unknown as SetPageAction;
+
+    const store = new GridSizeStore(
+        computed(() => hasMoreItemsBox.get()),
+        makePaginationConfig(pagination),
+        setPageAction,
+        computed(() => pageSizeBox.get()),
+        computed(() => columnCountBox.get())
+    );
+
+    return {
+        store,
+        hasMoreItemsBox,
+        pageSizeBox,
+        columnCountBox,
+        setPageAction: setPageAction as unknown as jest.Mock
+    };
+}
+
+function createMockRows(count: number, rowHeight: number): HTMLElement[] {
+    return Array.from({ length: count }, () => {
+        const row = document.createElement("div");
+        const cell = document.createElement("div");
+        Object.defineProperty(cell, "clientHeight", { value: rowHeight, configurable: true });
+        row.appendChild(cell);
+        return row;
+    });
+}
+
+interface RefsOptions {
+    rowHeight?: number;
+    rowCount?: number;
+    headerHeight?: number;
+    containerClientHeight?: number;
+    bodyScrollHeight?: number;
+    bodyClientHeight?: number;
+}
+
+function setupRefs(store: GridSizeStore, options: RefsOptions = {}): void {
+    const {
+        rowHeight = 40,
+        rowCount = 10,
+        headerHeight = 50,
+        containerClientHeight = 1000,
+        bodyScrollHeight,
+        bodyClientHeight
+    } = options;
+
+    const container = document.createElement("div");
+    Object.defineProperty(container, "clientHeight", { value: containerClientHeight, configurable: true });
+    (store.gridContainerRef as MutableRefObject<HTMLDivElement>).current = container;
+
+    const body = document.createElement("div");
+    createMockRows(rowCount, rowHeight).forEach(r => body.appendChild(r));
+    Object.defineProperty(body, "scrollHeight", {
+        value: bodyScrollHeight ?? rowHeight * rowCount,
+        configurable: true
+    });
+    Object.defineProperty(body, "clientHeight", {
+        value: bodyClientHeight ?? containerClientHeight - headerHeight,
+        configurable: true
+    });
+    (store.gridBodyRef as MutableRefObject<HTMLDivElement>).current = body;
+
+    const header = document.createElement("div");
+    const th = document.createElement("div");
+    th.className = "th";
+    Object.defineProperty(th, "offsetHeight", { value: headerHeight, configurable: true });
+    header.appendChild(th);
+    (store.gridHeaderRef as MutableRefObject<HTMLDivElement>).current = header;
+}
+
+describe("GridSizeStore.lockGridContainerHeight()", () => {
+    it("locks container height on first call when virtual scrolling is active and hasMoreItems", () => {
+        const { store } = buildStore({ pageSize: 3, columnCount: 2 });
+        // bodyViewport = 3 rows × 40px = 120px, header = 50px → fullHeight = 170px
+        // containerClientHeight (1000) >= fullHeight (170) → no overflow → subtract VIRTUAL_SCROLLING_OFFSET
+        setupRefs(store, { rowHeight: 40, rowCount: 3, headerHeight: 50, containerClientHeight: 1000 });
+
+        store.lockGridContainerHeight();
+
+        expect(store.gridContainerHeight).toBe(170 - VIRTUAL_SCROLLING_OFFSET);
+    });
+
+    it("is a no-op when hasVirtualScrolling is false", () => {
+        const { store } = buildStore({ pagination: "buttons" });
+        setupRefs(store);
+
+        store.lockGridContainerHeight();
+
+        expect(store.gridContainerHeight).toBeUndefined();
+    });
+
+    it("is a no-op when hasMoreItems is false", () => {
+        const { store } = buildStore({ hasMoreItems: false });
+        setupRefs(store);
+
+        store.lockGridContainerHeight();
+
+        expect(store.gridContainerHeight).toBeUndefined();
+    });
+
+    it("does not double-lock on repeated calls with same layout inputs", () => {
+        const { store } = buildStore({ pageSize: 3, columnCount: 2 });
+        setupRefs(store, { rowHeight: 40, rowCount: 3, headerHeight: 50, containerClientHeight: 1000 });
+
+        store.lockGridContainerHeight();
+        const heightAfterFirst = store.gridContainerHeight;
+
+        // Mutate refs to simulate different measurements — if re-locking, height would change
+        setupRefs(store, { rowHeight: 99, rowCount: 3, headerHeight: 50, containerClientHeight: 1000 });
+        store.lockGridContainerHeight();
+
+        expect(store.gridContainerHeight).toBe(heightAfterFirst);
+    });
+
+    it("clears and recomputes height when page size changes", () => {
+        const { store, pageSizeBox } = buildStore({ pageSize: 3, columnCount: 2 });
+        setupRefs(store, { rowHeight: 40, rowCount: 3, headerHeight: 50, containerClientHeight: 1000 });
+        store.lockGridContainerHeight();
+        const heightWithPageSize3 = store.gridContainerHeight;
+
+        pageSizeBox.set(5);
+        setupRefs(store, { rowHeight: 40, rowCount: 5, headerHeight: 50, containerClientHeight: 1000 });
+        store.lockGridContainerHeight();
+
+        // bodyViewport = 5 × 40 = 200, header = 50 → fullHeight = 250, no overflow → subtract offset
+        expect(store.gridContainerHeight).toBe(250 - VIRTUAL_SCROLLING_OFFSET);
+        expect(store.gridContainerHeight).not.toBe(heightWithPageSize3);
+    });
+
+    it("clears and recomputes height when visible column count changes", () => {
+        const { store, columnCountBox } = buildStore({ pageSize: 3, columnCount: 2 });
+        setupRefs(store, { rowHeight: 40, rowCount: 3, headerHeight: 50, containerClientHeight: 1000 });
+        store.lockGridContainerHeight();
+        const heightWith2Columns = store.gridContainerHeight;
+
+        columnCountBox.set(1);
+        // Simulate rows getting shorter after hiding a column
+        setupRefs(store, { rowHeight: 20, rowCount: 3, headerHeight: 50, containerClientHeight: 1000 });
+        store.lockGridContainerHeight();
+
+        // bodyViewport = 3 × 20 = 60, header = 50 → fullHeight = 110, no overflow → subtract offset
+        expect(store.gridContainerHeight).toBe(110 - VIRTUAL_SCROLLING_OFFSET);
+        expect(store.gridContainerHeight).not.toBe(heightWith2Columns);
+    });
+});
