Fix workload virtualization scroll ownership

2026-09-11 14:00:29 +00:00 · 2026-08-23 15:51:40 +01:00
parent 3bc613c915
commit 4040a79f4f
4 changed files with 79 additions and 41 deletions
@@ -784,6 +784,16 @@ change may globally weaken the Task 03 lifecycle-state idempotency invariant.
    `prefers-reduced-motion`; hot-path Workloads components must not re-create
    independent number animation loops.
 23. Extend grouped workload row windowing, reveal-index clamping, overscan math, and per-group visible-slice derivation through `frontend-modern/src/components/Workloads/useGroupedTableWindowing.ts`, and extend viewport event wiring through `frontend-modern/src/components/Workloads/useWorkloadViewportSync.ts` rather than rebuilding scroll handlers, mounted-row budgets, viewport listeners, or group-slice math inside `frontend-modern/src/components/Workloads/useWorkloadsDerivedState.ts`
+    Viewport sync may select an ancestor when it explicitly owns
+    `overflow-y: scroll`, including during the mount frame before children
+    contribute their final height, or when an `auto` / `overlay` ancestor has a
+    real vertical scroll range. A horizontally scrollable table shell may
+    compute `overflow-y: auto` through CSS overflow-axis coupling, but when its
+    scroll height equals its client height it is not the workload viewport
+    owner and must be skipped. The listener must continue outward to the
+    bounded app scroll container, or fall back to the window, so scrolling a
+    large mobile estate advances the mounted row window and removes the final
+    bottom spacer when the operator reaches the end.
 25. Extend workload table shell ownership through `frontend-modern/src/components/Workloads/WorkloadTableHeader.tsx` and `frontend-modern/src/components/Workloads/WorkloadPanel.tsx` rather than rebuilding sortable header markup, grouped node rows, row expansion, or guest-drawer rendering inside `frontend-modern/src/components/Workloads/WorkloadsTable.tsx`
    `WorkloadPanel` owns the mutually exclusive host/guest drawer handoff:
    clicking a grouped host row while a guest drawer is open must clear the
@@ -1,43 +1,41 @@
 {
  "version": 1,
-  "base_sha": "6869612c669ff640f52becfc3f66ce66415195c1",
-  "verified_at": "2026-08-23T14:05:18Z",
+  "base_sha": "3bc613c9157d1c6769ca25a0a5c3eec9c6694887",
+  "verified_at": "2026-08-23T14:51:12Z",
  "result": "passed",
  "changed_paths": [
-    "frontend-modern/src/components/Workloads/useGroupedTableWindowing.ts",
-    "frontend-modern/src/components/Workloads/useWorkloadsState.ts",
-    "frontend-modern/src/features/proxmox/ProxmoxPageSurface.tsx",
-    "frontend-modern/src/hooks/useWorkloads.ts"
+    "frontend-modern/src/components/Workloads/useWorkloadViewportSync.ts"
  ],
  "content_sha256": {
-    "frontend-modern/src/components/Workloads/useGroupedTableWindowing.ts": "861e07273c9e7aa30a565889903224df596dcddd20b988b40c79c4ee108fa0a7",
-    "frontend-modern/src/components/Workloads/useWorkloadsState.ts": "85182febda02e646fa354c6efd973222ef64360efaebc8b7b51c4e80821daa5b",
-    "frontend-modern/src/features/proxmox/ProxmoxPageSurface.tsx": "66dcc020434ee4ea4622fc1007c96eeb0b16432058ec1ade459c78530d26a75e",
-    "frontend-modern/src/hooks/useWorkloads.ts": "6ca20e750675f7016617366dfb23c3cfbe42394046fded71702903f0368f004c"
+    "frontend-modern/src/components/Workloads/useWorkloadViewportSync.ts": "5c5fd2650df7dfe6228e14cb033e8f22ec9866b3b5310cb6bb076757a4d09a1b"
  },
  "routes": [
    "/proxmox/overview"
  ],
  "viewports": [
-    {
-      "width": 2560,
-      "height": 1199
-    },
    {
      "width": 375,
      "height": 812
+    },
+    {
+      "width": 1440,
+      "height": 900
    }
  ],
  "states": [
-    "Fresh authenticated Proxmox overview renders separate Nodes and Guests regions with the guest count and workload table present",
-    "Desktop inspection shows the Proxmox guest region populated and the page remains within the desktop viewport width",
-    "Phone-width 375x812 inspection keeps the Guests heading, filters, and guest table visible with body scrollWidth equal to the 375px viewport",
-    "Fresh desktop and phone loads reported no browser console errors"
+    "The authenticated Proxmox overview renders 50 nodes and 929 guests from the large local mock estate",
+    "At 375x812 the initial virtual window keeps one trailing spacer while the first 140 guest rows are mounted",
+    "At the absolute bottom of the app scroll shell the virtual spacer is the leading row at index zero, the final workload row is last, and no trailing spacer remains",
+    "The Nodes card and footer follow the final guest row without a blank virtualized section",
+    "At 1440x900 the 929-guest table also reaches its final wireguard-edge-78 row with only a leading spacer and no trailing blank section",
+    "The verified phone load reported no browser console errors"
  ],
  "interactions": [
-    "Opened the local demo entrypoint and signed in to the authenticated demo session",
-    "Navigated to /proxmox/overview and inspected the Nodes and Guests table regions",
-    "Applied a 375x812 viewport override, reloaded the Proxmox overview, and rechecked guest visibility and horizontal overflow",
-    "Reset the temporary viewport override and rechecked the fresh tab console"
+    "Claimed the open local Proxmox overview and applied a temporary 375x812 viewport override",
+    "Reloaded the 929-guest mock estate and measured the initial virtual spacer and mounted row count",
+    "Scrolled the app-owned vertical container to its maximum scroll position and inspected spacer row placement plus the final workload row",
+    "Captured the bottom viewport showing the final guest rows followed immediately by the Nodes card and footer",
+    "Repeated the absolute-bottom spacer and final-row inspection at 1440x900",
+    "Reset the temporary viewport override after verification"
  ]
 }
@@ -81,6 +81,8 @@ describe('useWorkloadViewportSync', () => {

  it('tracks the app scroll container instead of leaving the initial spacer in place', async () => {
    const onScroll = vi.fn();
+    let appScrollContainer!: HTMLDivElement;
+    let horizontalTableWrapper!: HTMLDivElement;
    const groupedWindowing: UseGroupedTableWindowingResult = {
      endIndex: () => 150,
      getVisibleSlice: (_groupKey, guests) => guests,
@@ -104,7 +106,11 @@ describe('useWorkloadViewportSync', () => {
      return (
        <div
          ref={(element) => {
+            appScrollContainer = element;
            Object.defineProperty(element, 'clientHeight', { configurable: true, value: 400 });
+            // The explicit app scroll shell owns future vertical overflow even
+            // if the table rows have not contributed their final height yet.
+            Object.defineProperty(element, 'scrollHeight', { configurable: true, value: 400 });
            vi.spyOn(element, 'getBoundingClientRect').mockReturnValue({
              bottom: 400,
              height: 400,
@@ -119,24 +125,33 @@ describe('useWorkloadViewportSync', () => {
          }}
          style={{ 'overflow-y': 'scroll' }}
        >
-          <table>
-            <tbody
-              ref={(element) => {
-                vi.spyOn(element, 'getBoundingClientRect').mockReturnValue({
-                  bottom: -240,
-                  height: 320,
-                  left: 0,
-                  right: 800,
-                  toJSON: () => ({}),
-                  top: -560,
-                  width: 800,
-                  x: 0,
-                  y: -560,
-                } as DOMRect);
-                setBodyRef(element);
-              }}
-            />
-          </table>
+          <div
+            ref={(element) => {
+              horizontalTableWrapper = element;
+              Object.defineProperty(element, 'clientHeight', { configurable: true, value: 2800 });
+              Object.defineProperty(element, 'scrollHeight', { configurable: true, value: 2800 });
+            }}
+            style={{ 'overflow-x': 'auto' }}
+          >
+            <table>
+              <tbody
+                ref={(element) => {
+                  vi.spyOn(element, 'getBoundingClientRect').mockReturnValue({
+                    bottom: -240,
+                    height: 320,
+                    left: 0,
+                    right: 800,
+                    toJSON: () => ({}),
+                    top: -560,
+                    width: 800,
+                    x: 0,
+                    y: -560,
+                  } as DOMRect);
+                  setBodyRef(element);
+                }}
+              />
+            </table>
+          </div>
        </div>
      );
    };
@@ -146,5 +161,13 @@ describe('useWorkloadViewportSync', () => {
    await waitFor(() => {
      expect(onScroll).toHaveBeenCalledWith(560, 400, 32);
    });
+
+    appScrollContainer.dispatchEvent(new Event('scroll'));
+    await waitFor(() => {
+      expect(onScroll).toHaveBeenCalledTimes(2);
+    });
+
+    horizontalTableWrapper.dispatchEvent(new Event('scroll'));
+    expect(onScroll).toHaveBeenCalledTimes(2);
  });
 });
@@ -3,12 +3,19 @@ import { createEffect, onCleanup, type Accessor } from 'solid-js';
 import type { UseGroupedTableWindowingResult } from './useGroupedTableWindowing';

 const SCROLLABLE_OVERFLOW_PATTERN = /(?:auto|scroll|overlay)/;
+const MIN_VERTICAL_SCROLL_RANGE_PX = 1;

 const findScrollContainer = (element: HTMLElement): HTMLElement | null => {
  let parent = element.parentElement;
  while (parent && parent !== document.body && parent !== document.documentElement) {
    const styles = getComputedStyle(parent);
-    if (SCROLLABLE_OVERFLOW_PATTERN.test(`${styles.overflow} ${styles.overflowY}`)) {
+    const hasVerticalScrollRange =
+      parent.scrollHeight - parent.clientHeight > MIN_VERTICAL_SCROLL_RANGE_PX;
+    const ownsVerticalScrollBeforeOverflow = styles.overflowY === 'scroll';
+    if (
+      SCROLLABLE_OVERFLOW_PATTERN.test(styles.overflowY) &&
+      (ownsVerticalScrollBeforeOverflow || hasVerticalScrollRange)
+    ) {
      return parent;
    }
    parent = parent.parentElement;