gsadmin/pulse
1
0
Fork 0
mirror of https://github.com/rcourtman/Pulse.git synced 2026-10-23 20:25:56 +00:00
Code Issues Packages Projects Releases Wiki Activity

Stop Workloads row focus staging a stale shell scroll restore

Browse Source 
Selecting a Workloads row still staged the app-shell scroll position for
App.tsx to restore. That is left over from when row focus wrote ?resource=
into the URL, which 86ac84f68e removed. Selection no longer navigates, so
the staged position waited for the next unrelated route change. The
same-path route-state scheduler only re-stages from a scrolled shell. So
after opening a guest drawer deep in the Proxmox overview, scrolling back
to the top and picking a Type filter jumped the page back down to that
row. This reproduced on a mock stack at 1440x900 and 390x844; the shell
went from 0 to 3000 about 100 ms after the URL changed.

Row focus already holds its position through the contextual-focus offset
preservation. Drop the capture, and with it the helper that has no callers
left and its coverage test. Add a regression test that fails on the old
hook, and correct the contract prose that still described the old
handoff.
This commit is contained in:
rcourtman
2026-10-08 00:57:13 +01:00
parent 03767f1e95
commit c99028bc82
7 changed files with 115 additions and 114 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/release-control/v6/internal/subsystems/performance-and-scalability.md
+10 -5
View File
@@ -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
docs/release-control/v6/internal/subsystems/unified-resources.md
+7 -4
View File
@@ -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
frontend-modern/browser-verification/b3c60348af17e8cc.json
+47
View File
@@ -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"
]
}
frontend-modern/src/components/Workloads/__tests__/useWorkloadSelectionState.test.ts
+46
View File
@@ -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<WorkloadGuest[]>([]);
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();
});
});
frontend-modern/src/components/Workloads/useWorkloadSelectionState.ts
+5 -9
View File
@@ -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);
}
});
frontend-modern/src/utils/__tests__/appShellScrollRestoration.branchcov0712c.test.ts
-84
View File
@@ -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);
});
});
frontend-modern/src/utils/appShellScrollRestoration.ts
-12
View File
@@ -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<HTMLElement>(APP_SCROLL_SHELL_SELECTOR);
if (!shell || shell.scrollTop <= 0) {
return;
}
schedulePendingAppShellRestoreTop(shell.scrollTop);
};
export const readPendingAppShellRestoreTop = (): number | null => pendingAppShellRestoreTop;
export const clearPendingAppShellRestoreTop = (): void => {