From da9800ff2f6d1064556c25394e976c9cfa40dc64 Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Fri, 4 Sep 2026 00:23:56 +0100 Subject: [PATCH] Keep infrastructure dialog dismissal in place The stable-row focus fallback ran after the shared dialog cleanup and could scroll a lower Manage trigger into view. Preserve the existing focus return without changing the operator's viewport, and cover the real desktop and narrow browser path. Contract-Neutral: frontend focus-restoration bugfix; no API or persisted-data change. Change-source: pulse-maintainer --- .../v6/internal/subsystems/agent-lifecycle.md | 6 +++ .../subsystems/frontend-primitives.md | 5 ++ frontend-modern/browser-verification.json | 30 +++++------ .../Settings/InfrastructureWorkspace.tsx | 6 ++- .../InfrastructureWorkspace.test.tsx | 5 +- .../91-proxmox-node-display-names.spec.ts | 52 ++++++++++++++++++- 6 files changed, 84 insertions(+), 20 deletions(-) diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index e2005b53e..0b4f5da6e 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -2995,6 +2995,12 @@ explicitly mapped administrator reaches the lifecycle handlers. scrolling just to reach primary controls. Dedicated collection and last-activity columns may return only once the workspace has enough width to show the full ledger without clipping headers or row actions. + Closing a connection's governed Manage dialog must return keyboard focus to + the originating ledger action without moving the infrastructure viewport. + If ledger refresh replaces the original row while the dialog is open, + `InfrastructureWorkspace.tsx` may resolve the stable replacement action, + but that delayed fallback must also focus with `preventScroll` rather than + revealing the row through an implicit or explicit page scroll. 10. Keep post-install lifecycle completion explicit inside `frontend-modern/src/components/Settings/InfrastructureInstallerSection.tsx` and `frontend-modern/src/components/Settings/useInfrastructureInstallState.tsx`. diff --git a/docs/release-control/v6/internal/subsystems/frontend-primitives.md b/docs/release-control/v6/internal/subsystems/frontend-primitives.md index 30148078b..db477cc97 100644 --- a/docs/release-control/v6/internal/subsystems/frontend-primitives.md +++ b/docs/release-control/v6/internal/subsystems/frontend-primitives.md @@ -1066,6 +1066,11 @@ shell listeners before it closes. Feature-owned dialogs may provide a stable fallback target when their original virtual row has unmounted, but they must use the same scroll-neutral focus contract rather than compensating with page-level scroll writes. +`InfrastructureWorkspace.tsx` exercises that extension point after its shared +Manage dialog closes: both the shared captured-trigger restoration and its +delayed stable-row fallback must use `preventScroll`, preserve the app scroll +offset, and leave focus on the originating Manage action at desktop and narrow +viewports. Assistant shell entry changes must keep Assistant contextual rather than generic: `AppLayout.tsx` and the command palette may expose a compact launcher, diff --git a/frontend-modern/browser-verification.json b/frontend-modern/browser-verification.json index c5ae5f224..916740a36 100644 --- a/frontend-modern/browser-verification.json +++ b/frontend-modern/browser-verification.json @@ -1,18 +1,16 @@ { "version": 1, - "base_sha": "93c765a7eca670c46d93684e37af76ea9232686d", - "verified_at": "2026-09-03T21:29:58Z", + "base_sha": "fa006785c59a442da1f988d4f916bd97286d564d", + "verified_at": "2026-09-03T23:23:14Z", "result": "passed", "changed_paths": [ - "frontend-modern/src/components/shared/InlineDetailTableRow.tsx", - "frontend-modern/src/components/shared/Subtabs.tsx" + "frontend-modern/src/components/Settings/InfrastructureWorkspace.tsx" ], "content_sha256": { - "frontend-modern/src/components/shared/InlineDetailTableRow.tsx": "e3150f2bb67a6e4324bdc2b5cc6fa9915d4a17a059ace5a4571551169b203e36", - "frontend-modern/src/components/shared/Subtabs.tsx": "0f92b5a0be6904c717c6520ec9576beca02c3db4fb6a54b640d4513e8f3ba28b" + "frontend-modern/src/components/Settings/InfrastructureWorkspace.tsx": "ed65c454d88801bf7a24d8d063bea38fca248c4b90b29f083b81ba51ba4181bb" }, "routes": [ - "/proxmox/backups (managed local backend with deterministic Proxmox and PBS fixtures)" + "/settings/infrastructure (managed local backend with deterministic Proxmox cluster and connection-ledger fixtures)" ], "viewports": [ { @@ -25,16 +23,16 @@ } ], "states": [ - "Expanded PBS server row with Overview selected at desktop and narrow widths", - "Expanded PBS server row with Manage selected at desktop and narrow widths", - "Backup health content in view while the expanded drawer tab strip remained above the viewport", - "Collapsed PBS detail after focus restoration at desktop and narrow widths" + "Expanded Proxmox cluster members with the production cluster Manage trigger visible at desktop and narrow widths", + "Manage production dialog showing endpoint and cluster-member fields at desktop and narrow widths", + "Infrastructure workspace after dialog dismissal with focus returned to Manage at both widths", + "Reopened Manage dialog with the display-name save path available at both widths" ], "interactions": [ - "Opened a PBS server detail row from the Backups table", - "Selected Manage and returned to Overview while the drawer was visible", - "Moved below the expanded drawer, selected Manage programmatically, and verified no vertical scrollIntoView request occurred", - "Closed the drawer from focused tab content and verified focus restoration used preventScroll", - "Inspected Overview, Manage, and below-drawer screenshots for placement, clipping, overflow, and fixed-navigation overlap at both widths" + "Expanded the deterministic Proxmox cluster and opened its Manage dialog", + "Closed Manage and verified both the shared dialog and delayed stable-row focus restorations used preventScroll", + "Verified the application scroll offset did not change and keyboard focus returned to the originating Manage trigger", + "Reopened Manage, edited a cluster member display name, and saved through the immutable node identity path", + "Inspected open and dismissed screenshots for placement, clipping, overflow, focus indication, and fixed-navigation overlap at both widths" ] } diff --git a/frontend-modern/src/components/Settings/InfrastructureWorkspace.tsx b/frontend-modern/src/components/Settings/InfrastructureWorkspace.tsx index 7d9f2847c..5f085f0ad 100644 --- a/frontend-modern/src/components/Settings/InfrastructureWorkspace.tsx +++ b/frontend-modern/src/components/Settings/InfrastructureWorkspace.tsx @@ -321,7 +321,11 @@ const InfrastructureWorkspaceContent: Component = (element) => element.getAttribute('data-infrastructure-manage-id') === focusReturnConnectionId, ); - focusTarget?.focus(); + // The shared dialog restores its captured trigger without scrolling, + // but this delayed lookup covers rows recreated while the dialog was + // open. Keep that fallback from pulling a lower infrastructure row + // into view after the operator closes Manage. + focusTarget?.focus({ preventScroll: true }); }, 250); } }; diff --git a/frontend-modern/src/components/Settings/__tests__/InfrastructureWorkspace.test.tsx b/frontend-modern/src/components/Settings/__tests__/InfrastructureWorkspace.test.tsx index 87aa493d7..481cbc48e 100644 --- a/frontend-modern/src/components/Settings/__tests__/InfrastructureWorkspace.test.tsx +++ b/frontend-modern/src/components/Settings/__tests__/InfrastructureWorkspace.test.tsx @@ -1024,8 +1024,11 @@ describe('InfrastructureWorkspace', () => { expect(screen.getByText('Manage zeus')).toBeInTheDocument(); expect(screen.getByTestId('proxmox-section')).toBeInTheDocument(); + const focusSpy = vi.spyOn(manageButton, 'focus'); fireEvent.click(screen.getByRole('button', { name: 'Close edit infrastructure dialog' })); - await waitFor(() => expect(manageButton).toHaveFocus()); + await waitFor(() => expect(focusSpy).toHaveBeenCalledTimes(2)); + expect(focusSpy.mock.calls).toEqual([[{ preventScroll: true }], [{ preventScroll: true }]]); + expect(manageButton).toHaveFocus(); }); it('keeps the mounted node editor across connection-ledger refreshes', async () => { diff --git a/tests/integration/tests/91-proxmox-node-display-names.spec.ts b/tests/integration/tests/91-proxmox-node-display-names.spec.ts index 1180934ef..1c4754beb 100644 --- a/tests/integration/tests/91-proxmox-node-display-names.spec.ts +++ b/tests/integration/tests/91-proxmox-node-display-names.spec.ts @@ -183,7 +183,7 @@ const test = base.extend<{}, WorkerFixtures>({ test.describe('Proxmox cluster node display names', () => { test.setTimeout(180_000); - test('keeps native diagnostics while saving by immutable identity', async ({ page }) => { + test('keeps native diagnostics while saving by immutable identity', async ({ page }, testInfo) => { let updatePayload: Record | undefined; await routeClusterNodeDisplayNames(page, (payload) => { updatePayload = payload; @@ -197,7 +197,8 @@ test.describe('Proxmox cluster node display names', () => { await expect(page.getByText('Render East', { exact: true })).toBeVisible(); await expect(page.getByText('Production Cluster (pve1)', { exact: true })).toHaveCount(0); - await page.getByRole('button', { name: 'Manage', exact: true }).click(); + const manageButton = page.getByRole('button', { name: 'Manage', exact: true }); + await manageButton.click(); await expect( page.getByText( 'Give each node an optional display name for Pulse. This never changes its Proxmox name, identity, credentials, or connection address.', @@ -207,6 +208,53 @@ test.describe('Proxmox cluster node display names', () => { await expect(page.getByText('Proxmox node: pve1', { exact: true })).toBeVisible(); await expect(page.getByLabel('Display name for pve1')).toHaveValue('Render East'); + await page.screenshot({ path: testInfo.outputPath('manage-dialog.png') }); + const scrollTopBeforeClose = await page.evaluate(() => { + const shell = document.querySelector('.app-scroll-shell'); + return shell?.scrollTop ?? window.scrollY; + }); + await manageButton.evaluate((element) => { + const state = window as Window & { __infrastructureManageFocusCalls?: FocusOptions[] }; + state.__infrastructureManageFocusCalls = []; + const target = element as HTMLButtonElement; + const originalFocus = target.focus.bind(target); + target.focus = (options?: FocusOptions) => { + state.__infrastructureManageFocusCalls?.push(options ?? {}); + originalFocus(options); + }; + }); + await page.getByRole('button', { name: 'Close edit infrastructure dialog' }).click(); + await expect(page.getByRole('dialog')).toHaveCount(0); + await expect + .poll( + () => + page.evaluate( + () => + (window as Window & { __infrastructureManageFocusCalls?: FocusOptions[] }) + .__infrastructureManageFocusCalls?.length ?? 0, + ), + { timeout: 2_000 }, + ) + .toBe(2); + expect( + await page.evaluate( + () => + (window as Window & { __infrastructureManageFocusCalls?: FocusOptions[] }) + .__infrastructureManageFocusCalls, + ), + ).toEqual([{ preventScroll: true }, { preventScroll: true }]); + expect( + await page.evaluate(() => { + const shell = document.querySelector('.app-scroll-shell'); + return shell?.scrollTop ?? window.scrollY; + }), + ).toBe(scrollTopBeforeClose); + await expect(manageButton).toBeFocused(); + await page.screenshot({ path: testInfo.outputPath('manage-dialog-closed.png') }); + + await manageButton.click(); + await expect(page.getByRole('dialog')).toBeVisible(); + // Equal display labels are intentionally valid presentation. The write // target remains the second member's immutable identity. await page.getByLabel('Display name for pve2').fill('Render East');