diff --git a/docs/release-control/v6/internal/subsystems/frontend-primitives.md b/docs/release-control/v6/internal/subsystems/frontend-primitives.md index f3c90c8383..f15ae6a895 100644 --- a/docs/release-control/v6/internal/subsystems/frontend-primitives.md +++ b/docs/release-control/v6/internal/subsystems/frontend-primitives.md @@ -3050,7 +3050,7 @@ Agent`), with the plain-language source phrase available through accessible when the normalized guest table has no corresponding match. 8. Keep summary interaction identity on one shared helper. Pages that expose row-hover, group-hover, or route-focus-driven summary emphasis, today the Workloads and Storage tables through `useSummaryPageInteractionState` in `frontend-modern/src/components/shared/summaryTableFocus.ts`, must derive page/group/entity scope through `frontend-modern/src/components/shared/summaryCardInteraction.ts` rather than letting each surface read `hovered || focused` from its own page-local ID source. Hovering or pinning a storage pool-group header must scope the matching rows through that same shared contract instead of forking a page-local summary filter path. Workloads grouped host rows are static identity dividers and pass no group scope to the bridge; a grouped table that wants group hover or pin again needs a demand-ledger entry and must use this shared scope rather than reviving a Workloads-local path. No summary card or chart strip renders today, and the bridge carries no chart-hover input; one that returns must consume that same resolved scope, and the chart under the pointer must enter its hovered series into `resolveSummaryScopeState` as a preview input that outranks row hover instead of keeping a chart-local hover island. Recovery is explicitly outside this interaction dialect: its retired posture-card strip must not return with row/group/chart hover behavior without a separate governed product decision. 9. Keep any page summary page-scoped when table rows enter contextual focus. No infrastructure, workloads, or storage summary cards render today. If one returns, route-backed row selection may add a focused label and shared series emphasis, but the summary must continue to render the page-level series set instead of collapsing down to the selected row or replacing the global trend view with row-local empty states. -10. Keep contextual row focus on the shared primitives. Same-route table drill-ins must reuse `frontend-modern/src/components/shared/contextualFocus.ts` for local scroll preservation, off-screen reveal, inline-detail lookup, and deliberate inline-detail reveal, and must resolve the active series through `frontend-modern/src/components/shared/summaryCardInteraction.ts` behind `frontend-modern/src/components/shared/summaryTableFocus.ts`, instead of rebuilding page-local `Set` filters, focused-label scans, drawer-aware scroll math, or ad hoc scroll restoration in each surface. `frontend-modern/src/components/shared/summaryTableFocus.ts` measures and scrolls rows only for deliberate focus, a focused row's inline detail or a pinned group header, so it must not keep window-level scroll or resize listeners or re-measure the hovered row on hover changes and scroll frames; an off-screen affordance for the active row needs a rendered consumer before that cost returns. +10. Keep contextual row focus on the shared primitives. Same-route table drill-ins must reuse `frontend-modern/src/components/shared/contextualFocus.ts` for local scroll preservation, off-screen reveal, inline-detail lookup, and deliberate inline-detail reveal, and must resolve the active series through `frontend-modern/src/components/shared/summaryCardInteraction.ts` behind `frontend-modern/src/components/shared/summaryTableFocus.ts`, instead of rebuilding page-local `Set` filters, focused-label scans, drawer-aware scroll math, or ad hoc scroll restoration in each surface. `frontend-modern/src/components/shared/summaryTableFocus.ts` measures and scrolls rows only for deliberate focus, a focused row's inline detail or a pinned group header, and reveals a focused row once per focus change: its reveal effect subscribes to the focused series id and table root alone (a remounted table root starts a new reveal for the same focus) and runs the consumer's `revealActiveSeries` callback and the inline-detail scroll untracked, so page state the callback reads (live group lookups, expanded groups) cannot start another reveal, scroll the page back to the row, or reopen a group the operator collapsed. A reveal ends when its row and detail settle in view or its bounded retry window lapses, and filter changes do not re-run the consumer callback, so a row that stays filtered out past that window is not revealed when it returns. It must not keep window-level scroll or resize listeners or re-measure the hovered row on hover changes and scroll frames; an off-screen affordance for the active row needs a rendered consumer before that cost returns. 11. Keep summary-linked table row emphasis on the shared primitive contract. Workloads, infrastructure, and storage rows that mirror the active summary entity must expose that state through `data-summary-row-active` and let the shared presentation in `frontend-modern/src/index.css` render the row emphasis, rather than carrying page-local sky or blue fill classes inside each row renderer. Group-scoped preview and pin must use that same shared presentation boundary: child rows that belong to a hovered or pinned summary group should expose `data-summary-group-member-active="preview|pinned"` so the block-level emphasis stays subtle, consistent, and reversible instead of each table inventing its own outline, badge, or full-strength fill treatment. Static grouped row headers on workloads, infrastructure, storage, recovery, and future grouped tables must use `frontend-modern/src/components/shared/groupedTableRowPresentation.ts` plus the `.grouped-table-row` CSS contract in `frontend-modern/src/index.css`, rather than rebuilding local `bg-surface-alt` variants with subtly different light/dark behavior or page-local left-accent markers. That shared grouped-table primitive owns the subgroup cell padding, typography, small metadata, and badge treatment as well as the row background token, so a future adjustment to the subgroup visual language changes every grouped product table from one owner. Inline table detail rows on platform, workload, and infrastructure tables must compose `frontend-modern/src/components/shared/InlineDetailTableRow.tsx` for the full-width row, surface-alt cell, detail padding, and row-click containment instead of rebuilding page-local `TableRow` / `TableCell` / `div` shells around each drawer. Storage-backed reusable row presenters under `frontend-modern/src/features/storageBackups/` must also keep row height and alert accents on class/data-attribute presentation instead of runtime inline style maps, so the shared table contract stays CSP-safe on both steady-state and alert-highlighted routes. 12. Keep retained-value data loading honest at the ownership boundary. Helpers that prevent a feature surface from falling through the app-level Suspense diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index c78f5dee98..d238080f6b 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -4388,7 +4388,26 @@ which reopens a focused pool's collapsed owning group, and a pinned pool-group header that sits off-screen scrolls into view through the same bridge. Focus only ever names a row in the active pools or disks view, so the callback never switches views; the view-switching branches that served the retired jump to -the active row are gone. +the active row are gone. Reveal runs once per deliberate focus change: the +bridge starts a reveal only when the focused series id or the table root +element changes, and runs the callback and the inline-detail scroll untracked, +so the storage state the callback reads (the pool-to-group lookup that every +live update rebuilds, the expanded group set) never starts another. A live +update therefore cannot pull the page back to an open pool the operator +scrolled away from, and collapsing the open pool's group does not bounce it +back open. Filter changes do not re-run the callback either, so a focused +pool whose row stays filtered out past the reveal's short retry window is not +revealed when the filter later clears; storage filters live in the URL and a +storage link without filter parameters resets them, so only a link carrying a +filter that excludes its own pool reaches that case. +`frontend-modern/src/components/Storage/useStorageExpansionState.ts` stores +only the groups the operator collapsed and derives the expanded set from the +current group keys, so a group that appears later opens by default and a +collapsed group stays collapsed across live updates until the operator reopens +it, focus moves to one of its pools, or the grouping changes. Group keys are +bare labels, so a grouping change clears the collapsed set rather than letting +a label carry collapse state into another grouping or hide the headerless +ungrouped `All` group. That same reveal contract now also owns inline-detail expansion. When a pool or disk row is deliberately focused and its inline detail opens on the storage page, the detail row must publish the same canonical summary series ID through diff --git a/frontend-modern/browser-verification/a3cfe76a64f0d148.json b/frontend-modern/browser-verification/a3cfe76a64f0d148.json new file mode 100644 index 0000000000..8c2bd8ede3 --- /dev/null +++ b/frontend-modern/browser-verification/a3cfe76a64f0d148.json @@ -0,0 +1,56 @@ +{ + "version": 1, + "base_sha": "1a094646d7f21217eda051af6ba0252e208f283e", + "verified_at": "2026-10-07T23:22:00Z", + "result": "passed", + "changed_paths": [ + "frontend-modern/src/components/Storage/storagePageState.ts", + "frontend-modern/src/components/Storage/useStorageExpansionState.ts", + "frontend-modern/src/components/Storage/useStoragePageModel.ts", + "frontend-modern/src/components/shared/summaryTableFocus.ts" + ], + "content_sha256": { + "frontend-modern/src/components/Storage/storagePageState.ts": "63b20e139475680eda3e637eedc9ce2628108c6a91d0d2fab7648c67fd679f16", + "frontend-modern/src/components/Storage/useStorageExpansionState.ts": "b0b012ef7c386a6377dfe0852b5ed81be056efdc6acd2ab244b2c175bc696605", + "frontend-modern/src/components/Storage/useStoragePageModel.ts": "fe35c36b9478d03f1de21427d842b6f6f795b8965fc5293fff12aa4d4dae1431", + "frontend-modern/src/components/shared/summaryTableFocus.ts": "8ba44d5fb5d782dd96d1cec8bcb57a6696425d4eb5ab7fd546f77c4a073640d7" + }, + "routes": [ + "/proxmox/storage", + "/proxmox/storage?group=node", + "/proxmox/storage?group=type", + "/proxmox/storage?tab=disks", + "/proxmox/storage?resource=ceph-rbd", + "/proxmox/storage?group=node&resource=ceph-rbd", + "/proxmox/storage?group=node&summaryGroup=storage:node:West Production F", + "/proxmox", + "/proxmox?resource=" + ], + "viewports": [ + { + "width": 1440, + "height": 900 + }, + { + "width": 390, + "height": 844 + } + ], + "states": [ + "Private 12-node mock stack (PULSE_MOCK_MODE, live websocket updates) served by Vite from this branch; headless Chromium on pulse-dev through an SSH tunnel, same stack swapped between the PR 2709 code (before) and this change (after)", + "Pools view with an open pool after scrolling the app shell to the top: app-shell scrollTop sampled every 500 ms for 12 s. Before: 0, 0, 1008, 1111 and 1111 for the remaining 20 samples (yanked back to the open pool). After: 0 for all 24 samples. Physical Disks view and the Workloads table read 0 for all 24 samples both before and after", + "Storage grouped by node with an open pool, its group collapsed by the Collapse button and, separately, by a group-header click: group state sampled every 500 ms. Before: open in all 12 samples (bounced straight back). Bridge fix alone: the header-collapsed group reopened about 1.5 s later and console probes showed the expansion sync re-adding the collapsed key on a live update. After: shut in all 32 samples (16 s) for both paths", + "Regrouping after a collapse: node grouping with one group collapsed shows 33 of 35 pool rows; switching to type, then ungrouped, then back to node shows all 35 rows", + "Final pass on the rebased tree (PR 2709 merged): the scroll and collapse samples above unchanged, no page errors at either width" + ], + "interactions": [ + "Click a lower pool row: inline detail opens, row header and top of the detail in view, then Escape closes it (1440x900 and 390x844)", + "Click a lower physical disk row on ?tab=disks: detail opens with the row header and detail top in view (both widths)", + "Click a lower Workloads guest row on /proxmox: detail opens in view and Escape closes it; ?resource= deep link opens the guest detail in view (both widths)", + "Load ?resource=ceph-rbd: the Ceph pool detail opens in view (both widths)", + "With ?group=node, collapse the shared group with its disclosure button, then navigate in-app to ?group=node&resource=ceph-rbd: the group reopens and the Ceph detail is in view (both widths)", + "Load ?group=node&summaryGroup=: the pinned header scrolls into view and is marked active (scrollTop 1189 desktop and 1103 phone, as before the change)", + "Hover eight Storage and Workloads rows: rows mark active with no change in window scroll listeners or style reads versus before", + "Known unchanged edge recorded: with a pool already open and its group collapsed, an in-app link to that same pool does not reopen the group, because focus does not change" + ] +} diff --git a/frontend-modern/src/components/Storage/__tests__/Storage.test.tsx b/frontend-modern/src/components/Storage/__tests__/Storage.test.tsx index 67a75c3807..9da5e5a709 100644 --- a/frontend-modern/src/components/Storage/__tests__/Storage.test.tsx +++ b/frontend-modern/src/components/Storage/__tests__/Storage.test.tsx @@ -75,6 +75,8 @@ let wsState: any = { const reconnectSpy = vi.fn(); let hookResources: Resource[] = []; +// Bump to deliver hookResources as a live update to an already rendered page. +const [hookResourcesVersion, setHookResourcesVersion] = createSignal(0); let hookLoading = false; let hookError: unknown = undefined; let alertsDetectionEnabled = true; @@ -344,8 +346,12 @@ vi.mock('@/hooks/useUnifiedResources', () => ({ mutate: vi.fn(), }), useUnifiedResources: (options?: { cacheKey?: string }) => ({ - resources: () => - options?.cacheKey === 'storage-page' ? [...nodeResources, ...hookResources] : nodeResources, + resources: () => { + hookResourcesVersion(); + return options?.cacheKey === 'storage-page' + ? [...nodeResources, ...hookResources] + : nodeResources; + }, loading: () => (options?.cacheKey === 'storage-page' ? hookLoading : false), error: () => (options?.cacheKey === 'storage-page' ? hookError : undefined), refetch: vi.fn(), @@ -936,6 +942,73 @@ describe('Storage', () => { ); }); + it('does not scroll back to an open pool when a live update rebuilds the pools', async () => { + const scrollTo = vi.fn(); + const originalScrollTo = Object.getOwnPropertyDescriptor(window, 'scrollTo'); + Object.defineProperty(window, 'scrollTo', { configurable: true, value: scrollTo }); + hookResources = [ + buildStorageResource('storage-1', 'Node-Store', 'pve1'), + buildStorageResource('storage-2', 'Edge-Store', 'pve2'), + ]; + mockLocationSearch = '?group=node'; + + try { + render(() => ); + + fireEvent.click(await screen.findByRole('button', { name: 'Expand Node-Store' })); + await waitFor(() => { + expect(document.querySelector('tr[data-inline-detail-for]')).toBeTruthy(); + expect(scrollTo).toHaveBeenCalled(); + }); + // Count only once the reveal's settle frames stop scrolling. + let previousScrolls = -1; + while (scrollTo.mock.calls.length !== previousScrolls) { + previousScrolls = scrollTo.mock.calls.length; + await new Promise((resolve) => setTimeout(resolve, 50)); + } + const scrollsAfterFocus = scrollTo.mock.calls.length; + + hookResources = hookResources.map((resource) => ({ ...resource, lastSeen: Date.now() })); + setHookResourcesVersion((version) => version + 1); + await new Promise((resolve) => setTimeout(resolve, 100)); + + expect(screen.getByRole('button', { name: 'Collapse Node-Store' })).toBeInTheDocument(); + expect(scrollTo).toHaveBeenCalledTimes(scrollsAfterFocus); + } finally { + if (originalScrollTo) { + Object.defineProperty(window, 'scrollTo', originalScrollTo); + } + } + }); + + it('keeps the group of an open pool collapsed across live updates', async () => { + hookResources = [ + buildStorageResource('storage-1', 'Node-Store', 'pve1'), + buildStorageResource('storage-2', 'Edge-Store', 'pve2'), + ]; + mockLocationSearch = '?group=node'; + + render(() => ); + + fireEvent.click(await screen.findByRole('button', { name: 'Expand Node-Store' })); + await waitFor(() => { + expect(document.querySelector('tr[data-inline-detail-for]')).toBeTruthy(); + }); + + fireEvent.click(screen.getByRole('button', { name: 'Collapse pve1' })); + await new Promise((resolve) => setTimeout(resolve, 100)); + + expect(screen.queryByText('Node-Store')).not.toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Expand pve1' })).toBeInTheDocument(); + + hookResources = hookResources.map((resource) => ({ ...resource, lastSeen: Date.now() })); + setHookResourcesVersion((version) => version + 1); + await new Promise((resolve) => setTimeout(resolve, 100)); + + expect(screen.queryByText('Node-Store')).not.toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Expand pve1' })).toBeInTheDocument(); + }); + it('clears pinned storage group scope from the content-card header action', async () => { hookResources = [ buildStorageResource('storage-1', 'Node-Store', 'pve1'), diff --git a/frontend-modern/src/components/Storage/__tests__/storagePageState.branchcov0723pm.test.ts b/frontend-modern/src/components/Storage/__tests__/storagePageState.branchcov0723pm.test.ts index 2f94d228a8..f8554b78e3 100644 --- a/frontend-modern/src/components/Storage/__tests__/storagePageState.branchcov0723pm.test.ts +++ b/frontend-modern/src/components/Storage/__tests__/storagePageState.branchcov0723pm.test.ts @@ -4,7 +4,6 @@ import { getActiveStorageNodeOptions, getStorageFilterGroupBy, storageResourceMatchesSourceFilter, - syncExpandedStorageGroups, toStorageHealthFilterValue, type StoragePageNodeOption, type StorageStatusFilterValue, @@ -134,21 +133,4 @@ describe('storagePageState branch coverage 0723pm', () => { expect(storageResourceMatchesSourceFilter(node, 'proxmox-pve')).toBe(false); }); }); - - describe('syncExpandedStorageGroups (changed=false arm)', () => { - it('returns the previous set unchanged (same reference) when every key is already present', () => { - // No new keys => `changed` stays false => the `: previous` arm fires and - // the input reference is returned verbatim. - const previous = new Set(['A', 'B']); - const result = syncExpandedStorageGroups(previous, ['A', 'B']); - expect(result).toBe(previous); - expect(result).toEqual(new Set(['A', 'B'])); - }); - - it('returns previous unchanged when allKeys is empty but previous is non-empty', () => { - const previous = new Set(['A']); - const result = syncExpandedStorageGroups(previous, []); - expect(result).toBe(previous); - }); - }); }); diff --git a/frontend-modern/src/components/Storage/__tests__/storagePageState.test.ts b/frontend-modern/src/components/Storage/__tests__/storagePageState.test.ts index f5101350a4..7c7743c31a 100644 --- a/frontend-modern/src/components/Storage/__tests__/storagePageState.test.ts +++ b/frontend-modern/src/components/Storage/__tests__/storagePageState.test.ts @@ -36,8 +36,9 @@ import { normalizeStorageView, readStorageRouteValue, storageResourceMatchesSourceFilter, - syncExpandedStorageGroups, - toggleExpandedStorageGroup, + haveSameStorageGroups, + resolveExpandedStorageGroups, + toggleCollapsedStorageGroup, toStorageHealthFilterValue, writeStorageRouteValue, } from '@/components/Storage/storagePageState'; @@ -207,10 +208,14 @@ describe('storagePageState', () => { }); it('keeps expanded storage groups canonical across data refreshes', () => { - expect(syncExpandedStorageGroups(new Set(), ['A', 'B'])).toEqual(new Set(['A', 'B'])); - expect(syncExpandedStorageGroups(new Set(['A']), ['A', 'B'])).toEqual(new Set(['A', 'B'])); - expect(toggleExpandedStorageGroup(new Set(['A']), 'A')).toEqual(new Set()); - expect(toggleExpandedStorageGroup(new Set(['A']), 'B')).toEqual(new Set(['A', 'B'])); + expect(resolveExpandedStorageGroups(['A', 'B'], new Set())).toEqual(new Set(['A', 'B'])); + expect(resolveExpandedStorageGroups(['A', 'B'], new Set(['B']))).toEqual(new Set(['A'])); + expect(resolveExpandedStorageGroups(['A', 'B'], new Set(['A', 'B']))).toEqual(new Set()); + expect(toggleCollapsedStorageGroup(new Set(['A']), 'A')).toEqual(new Set()); + expect(toggleCollapsedStorageGroup(new Set(['A']), 'B')).toEqual(new Set(['A', 'B'])); + expect(haveSameStorageGroups(new Set(['A', 'B']), new Set(['B', 'A']))).toBe(true); + expect(haveSameStorageGroups(new Set(['A']), new Set(['B']))).toBe(false); + expect(haveSameStorageGroups(new Set(['A']), new Set(['A', 'B']))).toBe(false); }); it('derives active storage filters canonically', () => { diff --git a/frontend-modern/src/components/Storage/__tests__/useStorageExpansionState.test.ts b/frontend-modern/src/components/Storage/__tests__/useStorageExpansionState.test.ts index 77b4e6f517..a9a25f9c8d 100644 --- a/frontend-modern/src/components/Storage/__tests__/useStorageExpansionState.test.ts +++ b/frontend-modern/src/components/Storage/__tests__/useStorageExpansionState.test.ts @@ -2,14 +2,17 @@ import { renderHook } from '@solidjs/testing-library'; import { createSignal } from 'solid-js'; import { describe, expect, it } from 'vitest'; import { useStorageExpansionState } from '@/components/Storage/useStorageExpansionState'; +import type { StorageGroupKey } from '@/components/Storage/useStorageModel'; describe('useStorageExpansionState', () => { it('syncs expanded groups from grouped keys and toggles them', () => { const [groupedKeys] = createSignal(['alpha', 'beta']); const [view] = createSignal<'pools' | 'disks'>('pools'); + const [groupBy] = createSignal('node'); const { result } = renderHook(() => useStorageExpansionState({ + groupBy, groupedKeys, view, }), @@ -21,12 +24,67 @@ describe('useStorageExpansionState', () => { expect([...result.expandedGroups()]).toEqual(['beta']); }); - it('clears expanded pool state when switching away from pools', () => { - const [groupedKeys] = createSignal(['alpha']); - const [view, setView] = createSignal<'pools' | 'disks'>('pools'); + it('keeps collapsed groups collapsed when live updates rebuild the group keys', () => { + const [groupedKeys, setGroupedKeys] = createSignal(['alpha', 'beta']); + const [view] = createSignal<'pools' | 'disks'>('pools'); + const [groupBy] = createSignal('node'); const { result } = renderHook(() => useStorageExpansionState({ + groupBy, + groupedKeys, + view, + }), + ); + + result.toggleGroup('alpha'); + setGroupedKeys(['alpha', 'beta']); + expect([...result.expandedGroups()]).toEqual(['beta']); + + // A group that appears later opens by default. + setGroupedKeys(['alpha', 'beta', 'gamma']); + expect([...result.expandedGroups()]).toEqual(['beta', 'gamma']); + + // Collapsing every group sticks too. + result.toggleGroup('beta'); + result.toggleGroup('gamma'); + setGroupedKeys(['alpha', 'beta', 'gamma']); + expect([...result.expandedGroups()]).toEqual([]); + + result.toggleGroup('alpha'); + expect([...result.expandedGroups()]).toEqual(['alpha']); + }); + + it('opens every group when the grouping changes', () => { + // A node labelled like the headerless ungrouped container must not hide it. + const [groupedKeys, setGroupedKeys] = createSignal(['All', 'pve1']); + const [groupBy, setGroupBy] = createSignal('node'); + const [view] = createSignal<'pools' | 'disks'>('pools'); + + const { result } = renderHook(() => + useStorageExpansionState({ + groupBy, + groupedKeys, + view, + }), + ); + + result.toggleGroup('All'); + expect([...result.expandedGroups()]).toEqual(['pve1']); + + setGroupBy('none'); + setGroupedKeys(['All']); + expect([...result.expandedGroups()]).toEqual(['All']); + }); + + it('clears expanded pool state when switching away from pools', () => { + const [groupedKeys] = createSignal(['alpha']); + const [view, setView] = createSignal<'pools' | 'disks'>('pools'); + const [groupBy] = createSignal('node'); + + const { result } = renderHook(() => + useStorageExpansionState({ + groupBy, groupedKeys, view, }), diff --git a/frontend-modern/src/components/Storage/storagePageState.ts b/frontend-modern/src/components/Storage/storagePageState.ts index 4c7d1b9221..922734d50a 100644 --- a/frontend-modern/src/components/Storage/storagePageState.ts +++ b/frontend-modern/src/components/Storage/storagePageState.ts @@ -416,24 +416,17 @@ export const buildStorageNodeOnlineByLabel = (nodes: Resource[]): Map, - allKeys: string[], -): Set => { - if (previous.size === 0) return new Set(allKeys); +// Groups open by default and only the groups the operator collapsed are stored, +// so a live update that rebuilds the group list never reopens a collapsed one. +export const resolveExpandedStorageGroups = ( + allKeys: readonly string[], + collapsed: ReadonlySet, +): Set => new Set(allKeys.filter((key) => !collapsed.has(key))); - const next = new Set(previous); - let changed = false; - for (const key of allKeys) { - if (!next.has(key)) { - next.add(key); - changed = true; - } - } - return changed ? next : previous; -}; +export const haveSameStorageGroups = (a: ReadonlySet, b: ReadonlySet): boolean => + a.size === b.size && [...a].every((key) => b.has(key)); -export const toggleExpandedStorageGroup = (previous: Set, key: string): Set => { +export const toggleCollapsedStorageGroup = (previous: Set, key: string): Set => { const next = new Set(previous); if (next.has(key)) next.delete(key); else next.add(key); diff --git a/frontend-modern/src/components/Storage/useStorageExpansionState.ts b/frontend-modern/src/components/Storage/useStorageExpansionState.ts index 95780e5e55..62499c1c97 100644 --- a/frontend-modern/src/components/Storage/useStorageExpansionState.ts +++ b/frontend-modern/src/components/Storage/useStorageExpansionState.ts @@ -1,21 +1,32 @@ -import { createEffect, createSignal, type Accessor } from 'solid-js'; +import { createEffect, createMemo, createSignal, type Accessor } from 'solid-js'; import { - syncExpandedStorageGroups, - toggleExpandedStorageGroup, + haveSameStorageGroups, + resolveExpandedStorageGroups, + toggleCollapsedStorageGroup, type StorageView, } from './storagePageState'; +import type { StorageGroupKey } from './useStorageModel'; type UseStorageExpansionStateOptions = { + groupBy: Accessor; groupedKeys: Accessor; view: Accessor; }; export const useStorageExpansionState = (options: UseStorageExpansionStateOptions) => { - const [expandedGroups, setExpandedGroups] = createSignal>(new Set()); + const [collapsedGroups, setCollapsedGroups] = createSignal>(new Set()); const [expandedPoolId, setExpandedPoolId] = createSignal(null); + const expandedGroups = createMemo( + () => resolveExpandedStorageGroups(options.groupedKeys(), collapsedGroups()), + undefined, + { equals: haveSameStorageGroups }, + ); + // Group keys are bare labels, so collapse state belongs to one grouping: + // regrouping opens every group, including the headerless ungrouped one. createEffect(() => { - setExpandedGroups((prev) => syncExpandedStorageGroups(prev, options.groupedKeys())); + options.groupBy(); + setCollapsedGroups(new Set()); }); createEffect(() => { @@ -25,7 +36,7 @@ export const useStorageExpansionState = (options: UseStorageExpansionStateOption }); const toggleGroup = (key: string) => { - setExpandedGroups((prev) => toggleExpandedStorageGroup(prev, key)); + setCollapsedGroups((prev) => toggleCollapsedStorageGroup(prev, key)); }; return { diff --git a/frontend-modern/src/components/Storage/useStoragePageModel.ts b/frontend-modern/src/components/Storage/useStoragePageModel.ts index 3cc8e78cec..76df466b99 100644 --- a/frontend-modern/src/components/Storage/useStoragePageModel.ts +++ b/frontend-modern/src/components/Storage/useStoragePageModel.ts @@ -165,6 +165,7 @@ export const useStoragePageModel = (options: UseStoragePageModelOptions = {}) => setExpandedPoolId: setExpandedPoolIdRaw, toggleGroup, } = useStorageExpansionState({ + groupBy, groupedKeys: () => groupedRecords().map((group) => group.key), view, }); diff --git a/frontend-modern/src/components/shared/SharedPrimitives.guardrails.test.ts b/frontend-modern/src/components/shared/SharedPrimitives.guardrails.test.ts index a03aa77e49..818d157bbd 100644 --- a/frontend-modern/src/components/shared/SharedPrimitives.guardrails.test.ts +++ b/frontend-modern/src/components/shared/SharedPrimitives.guardrails.test.ts @@ -1365,6 +1365,9 @@ describe('shared primitive guardrails', () => { expect(summaryTableFocusSource).toContain('findInlineDetailElement'); expect(summaryTableFocusSource).toContain('revealInlineDetailInViewport'); expect(summaryTableFocusSource).toContain('MutationObserver'); + expect(summaryTableFocusSource).toContain( + 'untrack(() => revealFocusedSeries(root, focusedId))', + ); expect(summaryTableFocusSource).toContain('clearPinnedScope?: () => void;'); expect(summaryTableFocusSource).toContain('onEscapeClear?: () => void;'); expect(summaryTableFocusSource).toContain('setClearSurfaceRootRef'); diff --git a/frontend-modern/src/components/shared/__tests__/summaryTableFocus.test.tsx b/frontend-modern/src/components/shared/__tests__/summaryTableFocus.test.tsx index 1448e4de40..80e1ea0712 100644 --- a/frontend-modern/src/components/shared/__tests__/summaryTableFocus.test.tsx +++ b/frontend-modern/src/components/shared/__tests__/summaryTableFocus.test.tsx @@ -145,6 +145,96 @@ describe('useSummaryPageInteractionState', () => { expect(scrollTo).toHaveBeenCalledWith({ top: 456, behavior: 'smooth' }); }); + it('reveals once per focus change, not when state its reveal callback reads changes', () => { + const [focusedSeriesId, setFocusedSeriesId] = createSignal('pool-alpha'); + const groupKeyEntries: Array<[string, string]> = [ + ['pool-alpha', 'node-a'], + ['pool-beta', 'node-b'], + ]; + const [groupKeys, setGroupKeys] = createSignal(new Map(groupKeyEntries)); + const [expandedGroups, setExpandedGroups] = createSignal(new Set(['node-a'])); + const scrollTo = vi.fn(); + const root = document.createElement('div'); + + Object.defineProperty(window, 'scrollTo', { + configurable: true, + value: scrollTo, + }); + vi.stubGlobal('scrollY', 0); + + for (const [index, id] of ['pool-alpha', 'pool-beta'].entries()) { + const row = document.createElement('div'); + const detail = document.createElement('div'); + row.setAttribute('data-summary-series-id', id); + row.getBoundingClientRect = vi.fn(() => buildRect(680 + index * 300, 40)); + detail.setAttribute('data-inline-detail-for', id); + detail.getBoundingClientRect = vi.fn(() => buildRect(724 + index * 300, 220)); + root.append(row, detail); + } + document.body.appendChild(root); + + // Mirrors Storage: reopen the focused row's collapsed group. + const revealActiveSeries = vi.fn((seriesId: string) => { + const groupKey = groupKeys().get(seriesId); + if (groupKey && !expandedGroups().has(groupKey)) { + setExpandedGroups((current) => new Set(current).add(groupKey)); + } + }); + + const { result } = renderHook(() => + useSummaryPageInteractionState({ + focusedSeriesId, + revealActiveSeries, + }), + ); + + result.setTableRootRef(root); + + expect(revealActiveSeries).toHaveBeenCalledTimes(1); + const scrollsAfterFocus = scrollTo.mock.calls.length; + expect(scrollsAfterFocus).toBeGreaterThan(0); + + // A live update rebuilds the lookup the callback read; focus is unchanged. + setGroupKeys(new Map(groupKeyEntries)); + + expect(revealActiveSeries).toHaveBeenCalledTimes(1); + expect(scrollTo).toHaveBeenCalledTimes(scrollsAfterFocus); + + // The operator collapses the focused row's group, and it stays collapsed. + setExpandedGroups(new Set()); + + expect(revealActiveSeries).toHaveBeenCalledTimes(1); + expect(expandedGroups().has('node-a')).toBe(false); + + setFocusedSeriesId('pool-beta'); + + expect(revealActiveSeries).toHaveBeenCalledTimes(2); + expect(revealActiveSeries).toHaveBeenLastCalledWith('pool-beta'); + expect(expandedGroups().has('node-b')).toBe(true); + expect(scrollTo.mock.calls.length).toBeGreaterThan(scrollsAfterFocus); + }); + + it('does not re-reveal when a derived focus accessor recomputes the same id', () => { + const [metricIds, setMetricIds] = createSignal(new Map([['row-a', 'pool-alpha']])); + const revealActiveSeries = vi.fn(); + const root = document.createElement('div'); + document.body.appendChild(root); + + const { result } = renderHook(() => + useSummaryPageInteractionState({ + focusedSeriesId: () => metricIds().get('row-a') ?? null, + revealActiveSeries, + }), + ); + + result.setTableRootRef(root); + expect(revealActiveSeries).toHaveBeenCalledTimes(1); + + setMetricIds(new Map([['row-a', 'pool-alpha']])); + + expect(revealActiveSeries).toHaveBeenCalledTimes(1); + }); + it('clears pinned scope when operators click table whitespace on a clear surface', () => { const [focusedSeriesId] = createSignal('workload-a'); const clearPinnedScope = vi.fn(); diff --git a/frontend-modern/src/components/shared/summaryTableFocus.ts b/frontend-modern/src/components/shared/summaryTableFocus.ts index 68d73100cb..d854027f4f 100644 --- a/frontend-modern/src/components/shared/summaryTableFocus.ts +++ b/frontend-modern/src/components/shared/summaryTableFocus.ts @@ -1,4 +1,11 @@ -import { createEffect, createMemo, createSignal, onCleanup, type Accessor } from 'solid-js'; +import { + createEffect, + createMemo, + createSignal, + onCleanup, + untrack, + type Accessor, +} from 'solid-js'; import { findInlineDetailElement, revealInlineDetailInViewport } from './contextualFocus'; import { resolveSummaryActiveSeriesId, @@ -163,13 +170,9 @@ export function useSummaryTableFocusBridge(options: UseSummaryTableFocusBridgeOp }); }); - createEffect(() => { - const focusedId = normalizeSeriesId(focusedSeriesId()); - const root = tableRoot(); - if (!focusedId || !root || typeof window === 'undefined') { - return; - } - + // Runs inside the reveal effect below with tracking off, so the cleanup it + // registers still belongs to that effect run. + const revealFocusedSeries = (root: HTMLElement, focusedId: string) => { options.revealActiveSeries?.(focusedId); let settled = false; @@ -187,6 +190,7 @@ export function useSummaryTableFocusBridge(options: UseSummaryTableFocusBridgeOp } observer?.disconnect(); }; + onCleanup(cleanup); const settleReveal = (remainingFrames: number) => { if (settled) { @@ -254,8 +258,21 @@ export function useSummaryTableFocusBridge(options: UseSummaryTableFocusBridgeOp timeoutId = window.setTimeout(() => { cleanup(); }, 1200); + }; - onCleanup(cleanup); + // Reveal once per deliberate focus change. The effect subscribes to the + // focused id and table root only: the consumer's reveal callback reads live + // table state (group maps, expanded groups) that changes on every update or + // collapse, and a re-run would pull the page back to the row or reopen a + // group the operator just closed. + const revealSeriesId = createMemo(() => normalizeSeriesId(focusedSeriesId())); + createEffect(() => { + const focusedId = revealSeriesId(); + const root = tableRoot(); + if (!focusedId || !root || typeof window === 'undefined') { + return; + } + untrack(() => revealFocusedSeries(root, focusedId)); }); createEffect(() => { diff --git a/frontend-modern/src/utils/__tests__/frontendResourceTypeBoundaries.test.ts b/frontend-modern/src/utils/__tests__/frontendResourceTypeBoundaries.test.ts index c4fbf37a5e..484ecd88c9 100644 --- a/frontend-modern/src/utils/__tests__/frontendResourceTypeBoundaries.test.ts +++ b/frontend-modern/src/utils/__tests__/frontendResourceTypeBoundaries.test.ts @@ -1558,8 +1558,8 @@ describe('frontend resource type boundaries', () => { expect(storageResourceHighlightSource).toContain('export const useStorageResourceHighlight'); expect(storageResourceHighlightSource).toContain('findHighlightedStorageRecord'); expect(storageExpansionStateSource).toContain('export const useStorageExpansionState'); - expect(storageExpansionStateSource).toContain('syncExpandedStorageGroups'); - expect(storageExpansionStateSource).toContain('toggleExpandedStorageGroup'); + expect(storageExpansionStateSource).toContain('resolveExpandedStorageGroups'); + expect(storageExpansionStateSource).toContain('toggleCollapsedStorageGroup'); expect(storagePageSource).not.toContain('const rowClass = createMemo(() => {'); expect(storagePageSource).not.toContain('const rowStyle = createMemo(() => {'); expect(storagePageSource).not.toContain('const isWaitingForData = createMemo('); @@ -1847,8 +1847,8 @@ describe('frontend resource type boundaries', () => { expect(storagePageStateSource).toContain('export const buildStorageNodeOptions'); expect(storagePageStateSource).toContain('export const filterStorageDiskNodeOptions'); expect(storagePageStateSource).toContain('export const buildStorageNodeOnlineByLabel'); - expect(storagePageStateSource).toContain('export const syncExpandedStorageGroups'); - expect(storagePageStateSource).toContain('export const toggleExpandedStorageGroup'); + expect(storagePageStateSource).toContain('export const resolveExpandedStorageGroups'); + expect(storagePageStateSource).toContain('export const toggleCollapsedStorageGroup'); expect(storagePageStateSource).toContain('export const DEFAULT_STORAGE_SELECTED_NODE_ID'); expect(storagePageStateSource).toContain('export const DEFAULT_STORAGE_SORT_OPTIONS'); expect(storagePageStateSource).toContain('export const STORAGE_STATUS_FILTER_OPTIONS');