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 325a67998f..f76ccd7c9a 100644 --- a/docs/release-control/v6/internal/subsystems/performance-and-scalability.md +++ b/docs/release-control/v6/internal/subsystems/performance-and-scalability.md @@ -2435,7 +2435,13 @@ through `preserveScrollableAncestorVerticalOffset` in opened detail through `summaryTableFocus.ts`, so opening a focused workload does not look like a full page reload. The root `frontend-modern/src/App.tsx` shell applies a pending `frontend-modern/src/utils/appShellScrollRestoration.ts` -restore only when the route itself changes. The shared same-path scheduler must also own cleanup +restore on the next route change, whatever caused it, so only the shared +same-path route-state scheduler may stage one, from the shell position at the +moment it navigates. Local row focus stages none: a position staged without a +navigation waits for an unrelated one, and before this rule a Type filter +change made at the top of the Proxmox overview jumped the shell back down to +the row toggled earlier, because the scheduler stages nothing from scrollTop 0. +The shared same-path scheduler must also own cleanup for every deferred scroll-restore timeout and animation frame it creates, so route-state cleanup cannot leave hot-path replay work running after the owning surface unmounts. @@ -2455,10 +2461,9 @@ That same hot-path ownership now also covers deliberate inline-detail reveal. When a focused workload or infrastructure row opens its inline detail, the hot path may preserve scroll across same-route state writes, but the actual reveal must still flow through the shared contextual-focus and summary-table helpers. -Direct row toggles that already have the row in view must capture the current -app-shell scroll position before the focus write and let the remounted root -shell restore that position, so the interaction stays anchored instead of -looking like a full refresh. Once the drawer is mounted, the shared reveal +Direct row toggles that already have the row in view stay anchored through +that contextual-focus offset preservation, not through a staged app-shell +restore, so the interaction does not look like a full refresh. Once the drawer is mounted, the shared reveal helper must still take over whenever the opened detail would land below the fold, marking that movement as deliberate so route-state restore does not replay over it and then scrolling only enough to keep the row header plus the diff --git a/docs/release-control/v6/internal/subsystems/unified-resources.md b/docs/release-control/v6/internal/subsystems/unified-resources.md index 94fdf005aa..81b3704d3e 100644 --- a/docs/release-control/v6/internal/subsystems/unified-resources.md +++ b/docs/release-control/v6/internal/subsystems/unified-resources.md @@ -4110,10 +4110,13 @@ inline drawer through local component state, most through while the Docker hosts and Proxmox nodes tables keep their own selection signal, and write no route state. An infrastructure table that moves drawer selection into route state must tag the opened detail with the canonical active resource -ID; direct toggles of a row already in view must capture the current -`.app-scroll-shell` position through -`frontend-modern/src/utils/appShellScrollRestoration.ts`, so the remounted root -shell in `frontend-modern/src/App.tsx` stays anchored, and then hand off to the +ID; its route write must go through the shared same-path route-state scheduler +in `frontend-modern/src/utils/routeStateNavigation.ts`, which alone stages the +`.app-scroll-shell` position in +`frontend-modern/src/utils/appShellScrollRestoration.ts` at the moment it +navigates, so the root shell in `frontend-modern/src/App.tsx` stays anchored. +The row toggle itself stages nothing: the root shell applies a staged position +on the next route change, whatever caused it. The table must then hand off to the shared summary-table/contextual-focus helpers when the opened drawer would fall below the fold. That reveal scrolls only enough to keep the row header plus the start of the detail visible, never leaving the drawer clipped or hard-centering diff --git a/frontend-modern/browser-verification/b3c60348af17e8cc.json b/frontend-modern/browser-verification/b3c60348af17e8cc.json new file mode 100644 index 0000000000..7ba28e5682 --- /dev/null +++ b/frontend-modern/browser-verification/b3c60348af17e8cc.json @@ -0,0 +1,47 @@ +{ + "version": 1, + "base_sha": "03767f1e956c68c7b826635990638ea45fd9ddbd", + "verified_at": "2026-10-08T00:45:00Z", + "result": "passed", + "changed_paths": [ + "frontend-modern/src/components/Workloads/useWorkloadSelectionState.ts", + "frontend-modern/src/utils/appShellScrollRestoration.ts" + ], + "content_sha256": { + "frontend-modern/src/components/Workloads/useWorkloadSelectionState.ts": "4b125b316052a801d9db7568cd584e7165b73995795c1943770bd9133c363f3c", + "frontend-modern/src/utils/appShellScrollRestoration.ts": "b20770a6edd2196ba436de1aadf1bb8dcdf84baf0da492afa6e5255ebfd91fd5" + }, + "routes": [ + "/proxmox/overview", + "/proxmox/overview?type=vm", + "/proxmox/overview?resource=Analytics%3Apve34%3A740", + "/proxmox/storage", + "/docker/overview" + ], + "viewports": [ + { + "width": 1440, + "height": 900 + }, + { + "width": 390, + "height": 844 + } + ], + "states": [ + "Embedded build of this exact tree (both runtime files checked by sha256 on the test host) on a mock DEMO_MODE stack on pulse-dev; baseline is an embedded build of origin/main 7f45148d15 run with the same probe script, which instruments every scripted scrollTop write on .app-scroll-shell", + "Baseline repro at 1440x900 and 390x844: guest drawer opened at shell scrollTop 3000 (URL unchanged), shell scrolled to 0, Type filter set to VMs; URL became ?type=vm and 74 ms (desktop) / 126 ms (phone) later a scripted write moved the shell to 3000", + "This tree, same steps at both widths: shell stays at 0 after ?type=vm, no scripted shell write", + "Controls unchanged on both builds: same scroll path without a drawer stays at 0; filter clicked at scrollTop 500 after a drawer toggle keeps 500 (phone: 41 after the Filters sheet opens); Proxmox Storage sub-tab (both widths) and Docker tab (desktop; on phones the tab is not in the visible nav) after a drawer toggle land at 0", + "Opening a guest drawer keeps the shell at 3000 with the URL unchanged (local selection, contextual-focus offset preservation)", + "?resource= deep link opens the matching guest drawer and reveals it (shell 2960 desktop, 3143 phone); closing it keeps that position", + "No page errors at either width" + ], + "interactions": [ + "Row disclosure button click (desktop) and row tap (phone) to open and close a guest drawer", + "Type filter VMs button (desktop inline group; phone via the toolbar Filters sheet)", + "Proxmox Storage sub-tab link and Docker platform tab link", + "Drawer close button on a deep-linked drawer", + "rAF sampling of .app-scroll-shell scrollTop and URL for 2.5 s after each action" + ] +} diff --git a/frontend-modern/src/components/Workloads/__tests__/useWorkloadSelectionState.test.ts b/frontend-modern/src/components/Workloads/__tests__/useWorkloadSelectionState.test.ts index cbf7f1b844..cb877cfbcd 100644 --- a/frontend-modern/src/components/Workloads/__tests__/useWorkloadSelectionState.test.ts +++ b/frontend-modern/src/components/Workloads/__tests__/useWorkloadSelectionState.test.ts @@ -3,6 +3,14 @@ import { createSignal } from 'solid-js'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import type { WorkloadGuest } from '@/types/workloads'; +import { + clearPendingAppShellRestoreTop, + readPendingAppShellRestoreTop, +} from '@/utils/appShellScrollRestoration'; +import { + ROUTE_STATE_REPLACE_OPTIONS, + createRouteStateNavigateScheduler, +} from '@/utils/routeStateNavigation'; import { resolveWorkloadResourceSelection } from '../workloadSelectionModel'; import { useWorkloadSelectionState } from '../useWorkloadSelectionState'; @@ -302,4 +310,42 @@ describe('useWorkloadSelectionState', () => { expect(result.selectedGuestId()).toBeNull(); expect(navigateSpy).not.toHaveBeenCalled(); }); + + it('stages no app-shell restore for a local row toggle that a later route change could replay', () => { + locationSearch = ''; + clearPendingAppShellRestoreTop(); + const shell = document.createElement('div'); + shell.className = 'app-scroll-shell'; + shell.scrollTop = 3000; + document.body.appendChild(shell); + const [filteredGuests] = createSignal([]); + + const { result } = renderHook(() => + useWorkloadSelectionState({ + filteredGuests, + }), + ); + + result.setSelectedGuestId('cluster-a:node-1:101'); + vi.runAllTimers(); + expect(result.selectedGuestId()).toBe('cluster-a:node-1:101'); + expect(readPendingAppShellRestoreTop()).toBeNull(); + + result.setSelectedGuestId(null); + vi.runAllTimers(); + expect(readPendingAppShellRestoreTop()).toBeNull(); + + // Back at the top, a filter change writes the URL. The scheduler stages a + // restore only from a scrolled shell, so App.tsx would replay anything the + // row toggle had staged and jump the page back to 3000. + shell.scrollTop = 0; + const navigate = vi.fn(); + const scheduler = createRouteStateNavigateScheduler(navigate, () => '/proxmox/overview'); + scheduler.schedule('/proxmox/overview?type=vm'); + vi.runAllTimers(); + + expect(navigate).toHaveBeenCalledWith('/proxmox/overview?type=vm', ROUTE_STATE_REPLACE_OPTIONS); + expect(readPendingAppShellRestoreTop()).toBeNull(); + expect(navigateSpy).not.toHaveBeenCalled(); + }); }); diff --git a/frontend-modern/src/components/Workloads/useWorkloadSelectionState.ts b/frontend-modern/src/components/Workloads/useWorkloadSelectionState.ts index 6ee13e7d68..0840dc6949 100644 --- a/frontend-modern/src/components/Workloads/useWorkloadSelectionState.ts +++ b/frontend-modern/src/components/Workloads/useWorkloadSelectionState.ts @@ -4,7 +4,6 @@ import { createEffect, createSignal, type Accessor } from 'solid-js'; import { preserveScrollableAncestorVerticalOffset } from '@/components/shared/contextualFocus'; import { useSummaryPageInteractionState } from '@/components/shared/summaryTableFocus'; import type { WorkloadGuest } from '@/types/workloads'; -import { capturePendingAppShellRestoreTop } from '@/utils/appShellScrollRestoration'; import { workloadsHasHoveredWorkload, resolveWorkloadResourceSelection, @@ -58,30 +57,27 @@ export function useWorkloadSelectionState(options: UseWorkloadsSelectionStateOpt summaryInteraction.setClearSurfaceRootRef(element); }; - const setSelectedGuestIdState = (id: string | null) => { + // Row focus is local: it never navigates, so it must not stage an app-shell + // restore that the next unrelated route change would replay. + const setSelectedGuestId = (id: string | null) => { preserveScrollableAncestorVerticalOffset(tableWrapperRef(), () => { setSelectedGuestIdRaw(id); }); }; - const setSelectedGuestId = (id: string | null) => { - capturePendingAppShellRestoreTop(); - setSelectedGuestIdState(id); - }; - createEffect(() => { if (options.routeStateEnabled && !options.routeStateEnabled()) return; const resourceId = resolveWorkloadResourceSelection(location.search); if (!resourceId) { if (handledResourceId() !== null) { - setSelectedGuestIdState(null); + setSelectedGuestId(null); setHandledResourceId(null); } return; } if (resourceId !== handledResourceId()) { - setSelectedGuestIdState(resourceId); + setSelectedGuestId(resourceId); setHandledResourceId(resourceId); } }); diff --git a/frontend-modern/src/utils/__tests__/appShellScrollRestoration.branchcov0712c.test.ts b/frontend-modern/src/utils/__tests__/appShellScrollRestoration.branchcov0712c.test.ts deleted file mode 100644 index da89478a76..0000000000 --- a/frontend-modern/src/utils/__tests__/appShellScrollRestoration.branchcov0712c.test.ts +++ /dev/null @@ -1,84 +0,0 @@ -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; - -import { - capturePendingAppShellRestoreTop, - clearPendingAppShellRestoreTop, - readPendingAppShellRestoreTop, -} from '@/utils/appShellScrollRestoration'; - -describe('capturePendingAppShellRestoreTop – branch coverage', () => { - beforeEach(() => { - clearPendingAppShellRestoreTop(); - document.body.innerHTML = ''; - }); - - afterEach(() => { - vi.unstubAllGlobals(); - document.body.innerHTML = ''; - }); - - it('returns early without capturing when document is undefined (SSR guard arm)', () => { - vi.stubGlobal('document', undefined); - - capturePendingAppShellRestoreTop(); - - expect(readPendingAppShellRestoreTop()).toBeNull(); - }); - - it('does not capture when no .app-scroll-shell element exists (!shell short-circuit arm)', () => { - capturePendingAppShellRestoreTop(); - - expect(readPendingAppShellRestoreTop()).toBeNull(); - }); - - it('does not capture when the shell is at the top (scrollTop === 0 hits the <= 0 boundary arm)', () => { - const shell = document.createElement('div'); - shell.className = 'app-scroll-shell'; - shell.scrollTop = 0; - document.body.appendChild(shell); - - capturePendingAppShellRestoreTop(); - - expect(readPendingAppShellRestoreTop()).toBeNull(); - }); - - it('does not capture when the shell reports a negative scrollTop (defensive <= 0 arm)', () => { - // Real browsers never report a negative scrollTop, so inject a stub - // element via querySelector to force the <= 0 guard's negative side and - // confirm the exact selector contract. - const stubShell = { scrollTop: -10 } as unknown as HTMLElement; - const querySpy = vi.spyOn(document, 'querySelector').mockReturnValue(stubShell); - - capturePendingAppShellRestoreTop(); - - expect(readPendingAppShellRestoreTop()).toBeNull(); - expect(querySpy).toHaveBeenCalledWith('.app-scroll-shell'); - querySpy.mockRestore(); - }); - - it('captures the exact scrollTop when the shell is scrolled past the top (happy path)', () => { - const shell = document.createElement('div'); - shell.className = 'app-scroll-shell'; - shell.scrollTop = 327; - document.body.appendChild(shell); - - capturePendingAppShellRestoreTop(); - - expect(readPendingAppShellRestoreTop()).toBe(327); - }); - - it('preserves a previously captured value when a later capture hits a guard arm (no-overwrite)', () => { - const scrolled = document.createElement('div'); - scrolled.className = 'app-scroll-shell'; - scrolled.scrollTop = 150; - document.body.appendChild(scrolled); - - capturePendingAppShellRestoreTop(); - expect(readPendingAppShellRestoreTop()).toBe(150); - - scrolled.remove(); - capturePendingAppShellRestoreTop(); - - expect(readPendingAppShellRestoreTop()).toBe(150); - }); -}); diff --git a/frontend-modern/src/utils/appShellScrollRestoration.ts b/frontend-modern/src/utils/appShellScrollRestoration.ts index 294fc38f24..b8f084d783 100644 --- a/frontend-modern/src/utils/appShellScrollRestoration.ts +++ b/frontend-modern/src/utils/appShellScrollRestoration.ts @@ -1,21 +1,9 @@ let pendingAppShellRestoreTop: number | null = null; -const APP_SCROLL_SHELL_SELECTOR = '.app-scroll-shell'; export const schedulePendingAppShellRestoreTop = (scrollTop: number): void => { pendingAppShellRestoreTop = Math.max(0, scrollTop); }; -export const capturePendingAppShellRestoreTop = (): void => { - if (typeof document === 'undefined') { - return; - } - const shell = document.querySelector(APP_SCROLL_SHELL_SELECTOR); - if (!shell || shell.scrollTop <= 0) { - return; - } - schedulePendingAppShellRestoreTop(shell.scrollTop); -}; - export const readPendingAppShellRestoreTop = (): number | null => pendingAppShellRestoreTop; export const clearPendingAppShellRestoreTop = (): void => {