mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-09-09 18:15:50 +00:00
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
This commit is contained in:
@@ -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`.
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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"
|
||||
]
|
||||
}
|
||||
|
||||
@@ -321,7 +321,11 @@ const InfrastructureWorkspaceContent: Component<InfrastructureWorkspaceProps> =
|
||||
(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);
|
||||
}
|
||||
};
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
@@ -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<string, unknown> | 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<HTMLElement>('.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<HTMLElement>('.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');
|
||||
|
||||
Reference in New Issue
Block a user