mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-10-23 20:25:56 +00:00
Stop Storage scrolling back to open pools and reopening groups (#2733)
Follow-up to #2709. ## Problem On Proxmox > Storage (pools view): 1. With a pool open, scrolling the page back to the top pulled it back down to the open pool a second or two later, and kept doing so. 2. With `?group=node`, collapsing the open pool's group (Collapse button or header click) bounced it straight back open. ## Causes and fixes - **Bridge reveal re-ran on unrelated state.** `useSummaryTableFocusBridge` called the consumer's `revealActiveSeries` inside a tracked effect, so Storage's reads (the pool-to-group lookup rebuilt on every live update, the expanded group set) re-ran the reveal and its inline-detail scroll. The effect now depends only on a memo of the focused series id and the table root, and runs the callback and scroll untracked (`summaryTableFocus.ts`). Cleanup is registered before the first attempt, so a focus change or unmount cancels pending settle frames. - **Expansion state reopened collapsed groups.** `syncExpandedStorageGroups` re-added every group key missing from the expanded set whenever live data rebuilt the groups, so any collapsed group reopened within ~10 s. The bounce above had been hiding this. `useStorageExpansionState` now stores only the collapsed groups and derives the expanded set, so new groups still open by default. Group keys are bare labels, so a grouping change clears the collapsed set. That stops a label carrying collapse state into another grouping or hiding the headerless `All` group. ## Evidence 12-node mock stack, Chromium at 1440x900 and 390x844, same stack swapped between #2709's code and this change: | Check | Before | After | |---|---|---| | App-shell scrollTop after scrolling away from an open pool (every 0.5 s, 12 s) | 0, 0, 1008, then 1111 ×21 | 0 ×24 | | Collapse open pool's group via button, group state (0.5 s samples) | open ×12 | shut ×32 (16 s) | | Same via header click | open ×12 | shut ×32 (16 s) | | Disks view / Workloads table scroll-away | 0 ×24 | 0 ×24 | Unchanged: lower pool, disk and guest rows open their detail with the row header and detail top in view; Escape closes it; `?resource=ceph-rbd` opens in view; collapsing `shared` and then following an in-app `?group=node&resource=ceph-rbd` link reopens the group and shows the detail; a `?summaryGroup=` deep link scrolls to the pinned header; regrouping node → type → none → node shows every pool. Tests: new bridge tests (`summaryTableFocus.test.tsx`), Storage-level live-update and collapse tests (`Storage.test.tsx`), expansion hook tests, and a guardrail pin that the reveal runs untracked. All six new tests fail on #2709's code. The three expansion tests also fail with the new bridge but the old expansion state. ## Known limit Filter changes do not re-run the reveal. A focused pool whose row stays filtered out past the reveal's ~1.2 s retry window is not revealed when the filter 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 this. Likewise, with a pool already open and its group collapsed, an in-app link to that same pool leaves the group collapsed, because focus does not change.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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=<guest>"
|
||||
],
|
||||
"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=<guest> 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=<last group>: 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"
|
||||
]
|
||||
}
|
||||
@@ -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(() => <Storage />);
|
||||
|
||||
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(() => <Storage />);
|
||||
|
||||
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'),
|
||||
|
||||
-18
@@ -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);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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', () => {
|
||||
|
||||
@@ -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<StorageGroupKey>('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<StorageGroupKey>('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<StorageGroupKey>('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<StorageGroupKey>('node');
|
||||
|
||||
const { result } = renderHook(() =>
|
||||
useStorageExpansionState({
|
||||
groupBy,
|
||||
groupedKeys,
|
||||
view,
|
||||
}),
|
||||
|
||||
@@ -416,24 +416,17 @@ export const buildStorageNodeOnlineByLabel = (nodes: Resource[]): Map<string, bo
|
||||
return map;
|
||||
};
|
||||
|
||||
export const syncExpandedStorageGroups = (
|
||||
previous: Set<string>,
|
||||
allKeys: string[],
|
||||
): Set<string> => {
|
||||
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<string>,
|
||||
): Set<string> => 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<string>, b: ReadonlySet<string>): boolean =>
|
||||
a.size === b.size && [...a].every((key) => b.has(key));
|
||||
|
||||
export const toggleExpandedStorageGroup = (previous: Set<string>, key: string): Set<string> => {
|
||||
export const toggleCollapsedStorageGroup = (previous: Set<string>, key: string): Set<string> => {
|
||||
const next = new Set(previous);
|
||||
if (next.has(key)) next.delete(key);
|
||||
else next.add(key);
|
||||
|
||||
@@ -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<StorageGroupKey>;
|
||||
groupedKeys: Accessor<string[]>;
|
||||
view: Accessor<StorageView>;
|
||||
};
|
||||
|
||||
export const useStorageExpansionState = (options: UseStorageExpansionStateOptions) => {
|
||||
const [expandedGroups, setExpandedGroups] = createSignal<Set<string>>(new Set());
|
||||
const [collapsedGroups, setCollapsedGroups] = createSignal<Set<string>>(new Set());
|
||||
const [expandedPoolId, setExpandedPoolId] = createSignal<string | null>(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<string>());
|
||||
});
|
||||
|
||||
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 {
|
||||
|
||||
@@ -165,6 +165,7 @@ export const useStoragePageModel = (options: UseStoragePageModelOptions = {}) =>
|
||||
setExpandedPoolId: setExpandedPoolIdRaw,
|
||||
toggleGroup,
|
||||
} = useStorageExpansionState({
|
||||
groupBy,
|
||||
groupedKeys: () => groupedRecords().map((group) => group.key),
|
||||
view,
|
||||
});
|
||||
|
||||
@@ -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');
|
||||
|
||||
@@ -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<string | null>('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<string>());
|
||||
|
||||
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<string | null>('workload-a');
|
||||
const clearPinnedScope = vi.fn();
|
||||
|
||||
@@ -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(() => {
|
||||
|
||||
@@ -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');
|
||||
|
||||
Reference in New Issue
Block a user