From 4040a79f4f5ffc465131fb75c31146d93e45c8d5 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Sun, 23 Aug 2026 15:51:40 +0100 Subject: [PATCH] Fix workload virtualization scroll ownership --- .../subsystems/performance-and-scalability.md | 10 ++++ frontend-modern/browser-verification.json | 42 +++++++------ .../useWorkloadViewportSync.test.tsx | 59 +++++++++++++------ .../Workloads/useWorkloadViewportSync.ts | 9 ++- 4 files changed, 79 insertions(+), 41 deletions(-) diff --git a/docs/release-control/v6/internal/subsystems/performance-and-scalability.md b/docs/release-control/v6/internal/subsystems/performance-and-scalability.md index f5b152b94..e68ac5e47 100644 --- a/docs/release-control/v6/internal/subsystems/performance-and-scalability.md +++ b/docs/release-control/v6/internal/subsystems/performance-and-scalability.md @@ -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 diff --git a/frontend-modern/browser-verification.json b/frontend-modern/browser-verification.json index 779a1f304..75428e206 100644 --- a/frontend-modern/browser-verification.json +++ b/frontend-modern/browser-verification.json @@ -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" ] } diff --git a/frontend-modern/src/components/Workloads/__tests__/useWorkloadViewportSync.test.tsx b/frontend-modern/src/components/Workloads/__tests__/useWorkloadViewportSync.test.tsx index ea47c2bb0..35838397e 100644 --- a/frontend-modern/src/components/Workloads/__tests__/useWorkloadViewportSync.test.tsx +++ b/frontend-modern/src/components/Workloads/__tests__/useWorkloadViewportSync.test.tsx @@ -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 (
{ + 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' }} > - - { - 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); - }} - /> -
+
{ + horizontalTableWrapper = element; + Object.defineProperty(element, 'clientHeight', { configurable: true, value: 2800 }); + Object.defineProperty(element, 'scrollHeight', { configurable: true, value: 2800 }); + }} + style={{ 'overflow-x': 'auto' }} + > + + { + 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); + }} + /> +
+
); }; @@ -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); }); }); diff --git a/frontend-modern/src/components/Workloads/useWorkloadViewportSync.ts b/frontend-modern/src/components/Workloads/useWorkloadViewportSync.ts index 46a6d0675..73e5d0e94 100644 --- a/frontend-modern/src/components/Workloads/useWorkloadViewportSync.ts +++ b/frontend-modern/src/components/Workloads/useWorkloadViewportSync.ts @@ -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;