From ccc25d269be99c040913d211607ec6cd36195378 Mon Sep 17 00:00:00 2001 From: rcourtman <8825017+rcourtman@users.noreply.github.com> Date: Wed, 7 Oct 2026 21:30:25 +0100 Subject: [PATCH] Correct stale Storage embed and History range contract prose The storage-recovery contract described an embedded StorageSurface that opts into its toolbar through a showFilterToolbar prop, keeps tableOnly to hide a storage summary section, and offers a chart-collapse control, alongside a RecoverySurface and a forcedPlatformFilter. None of that exists. Storage.tsx always renders StoragePageControls (only kiosk mode hides it), the Proxmox Storage tab is the only embed, and /truenas/storage renders TrueNASStorageTopologyTable rather than StorageSurface. The Workloads contracts said the sparkline range appears only while Trends is active and carries a "Trend range" label. WorkloadsFilter shows it in Trends and, in Bars, at desktop widths while Row hover is History, and labels it "History". A rendered Storage test now pins the embed behaviour the rewrite describes. The toolbar renders with no opt-in prop. The forced source outranks a URL source and is never written back. A URL node resets to all nodes. Neither lock surfaces as a chip, an Add filter option or a clear action. The History label was already pinned by WorkloadsFilter.test.tsx and the Workloads performance contract test. --- .../subsystems/frontend-primitives.md | 7 ++- .../subsystems/performance-and-scalability.md | 11 ++-- .../internal/subsystems/storage-recovery.md | 52 +++++++++------- .../Storage/__tests__/Storage.test.tsx | 61 ++++++++++++++++++- 4 files changed, 103 insertions(+), 28 deletions(-) diff --git a/docs/release-control/v6/internal/subsystems/frontend-primitives.md b/docs/release-control/v6/internal/subsystems/frontend-primitives.md index fd73a5370c..5cd905860e 100644 --- a/docs/release-control/v6/internal/subsystems/frontend-primitives.md +++ b/docs/release-control/v6/internal/subsystems/frontend-primitives.md @@ -725,8 +725,11 @@ choices compose the shared `ViewOptionsDisclosure` instead of occupying the primary filter rail: layout, metric style, chart visibility, memory basis, and columns remain discoverable behind one `View` trigger. The history range stays inline in both metric modes because bars now expose an intent-driven row -history lens and Trends keeps the same charts persistent. That inline range -must carry a visible contextual label. +history lens and Trends keeps the same charts persistent. In Bars the range +renders only at desktop widths while Row hover is set to `History`, the mode +that drives that lens; `Details` hover and the phone (`isMobile`) layout hide +it. The inline range carries the visible `History` label that +`WorkloadsFilter` passes to `MetricHistoryRangeSegmentedControl`. Controls inside the View disclosure must expand in place rather than opening nested absolute panels that can clip or create competing overlay stacks. The Proxmox page owns and persists the `Guest` / `Host` memory basis; diff --git a/docs/release-control/v6/internal/subsystems/performance-and-scalability.md b/docs/release-control/v6/internal/subsystems/performance-and-scalability.md index ef776a8f02..d11703eeff 100644 --- a/docs/release-control/v6/internal/subsystems/performance-and-scalability.md +++ b/docs/release-control/v6/internal/subsystems/performance-and-scalability.md @@ -2778,10 +2778,13 @@ counts wrap independently so a narrow toolbar cannot strand View on a line by itself or force page-level horizontal overflow. The compact Add filter control joins that cluster only in the default state where no active menu chip, contextual Clear action, or leading action needs its own filter row; active -scope editing keeps Add filter beside the chips. The sparkline range remains -inline only while trends are active because it is the frequently changed -analytical window, and its compact control must include a visible `Trend range` -label so the values are not an orphaned set of durations. Columns must expand +scope editing keeps Add filter beside the chips. The sparkline range is a +trailing inline control, not a View option, because it is the frequently +changed analytical window. `WorkloadsFilter` renders it whenever the metric +mode is Trends, and in Bars only at desktop widths while Row hover is `History` +(the default); Bars with `Details` hover, or Bars below the 768px `isMobile` +breakpoint, hides it. Its compact control carries the visible `History` label +so the values are not an orphaned set of durations. Columns must expand as a full-width responsive checkbox row inside the View disclosure instead of opening a nested absolute panel or a tall single-column desktop list. On Proxmox, the non-default Host basis must remain visible in the workload memory diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index d1199c9224..f7eee106ba 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -2430,11 +2430,13 @@ recovery scope, or a storage/recovery-owned secret source. Storage and recovery UI must keep sourcing those signals from their existing canonical page models instead of polling the connections ledger for per-datastore or per-backup truth. - Platform-first top-level pages may embed `StorageSurface` and - `RecoverySurface` with `embedded tableOnly` and forced source or - platform filters (e.g. `forcedSourceFilter`, `forcedPlatformFilter`) - so platform-scoped storage and recovery rows render through the same - canonical surfaces rather than a forked per-platform table. + Platform-first top-level pages may embed `StorageSurface` + (`frontend-modern/src/components/Storage/Storage.tsx`) with a forced + source filter (`forcedSourceFilter`) so platform-scoped storage rows + render through the same canonical surface rather than a forked + per-platform table. The surface has no `embedded` or `tableOnly` mode + and no `forcedPlatformFilter`, and no `RecoverySurface` component + exists to embed alongside it. `frontend-modern/src/App.tsx` may carry the platform-page route registrations that mount those embedded canonical surfaces, but the routes themselves must derive their paths from the canonical builders @@ -2444,22 +2446,30 @@ recovery scope, or a storage/recovery-owned secret source. canonical surface that actually populates. The canonical TrueNAS adapter already emits the top-level TrueNAS system as a unified `agent` row tagged with the `truenas` platform, so the platform - page defaults to `/truenas/overview` (the Systems sub-tab) and the - embedded `StorageSurface` lives at `/truenas/storage`. The Source - filter chip in `StoragePageControls` is also suppressed when a - platform page locks source scope through `forcedSourceFilter` (via - `suppressSourceFilter`, auto-applied whenever `forcedSourceFilter` - is set), so the user never sees the platform's name pinned as a - removable filter chip inside the embedded surface. - Platform pages that embed `StorageSurface` reuse the canonical - `StoragePageControls` toolbar through the `showFilterToolbar` prop on - `StorageProps`. The page keeps `tableOnly` to hide the storage summary - section but opts in to the shared search, status, grouping, sort, - node, view, and chart-collapse controls so platform operators get - dense-table storage controls on every embedded storage tab without - forking the toolbar. The source scope flows through - `forcedSourceFilter` as a typed page input; the source filter remains - available in the toolbar only when not forced. The seven-state Storage + page defaults to `/truenas/overview`, whose Overview sub-tab lists + those systems. `/truenas/storage` renders the TrueNAS-owned + `TrueNASStorageTopologyTable`, not `StorageSurface`; the Proxmox + Storage tab in `frontend-modern/src/features/proxmox/ProxmoxPageSurface.tsx` + is the only page that embeds `StorageSurface` today. + An embedding page gets the canonical `StoragePageControls` toolbar + without opting in: `Storage.tsx` always renders it (only kiosk mode + hides it), there is no `showFilterToolbar` prop, and there is no storage + summary section to hide. The Proxmox embed passes `forcedSourceFilter`, + its own `resourceSource`, `suppressNodeFilter`, `filterAriaLabel`, and + Proxmox search placeholder and empty-history copy. It passes no + `forcedView`, so the Storage / Physical Disks view tabs stay. + `forcedSourceFilter` is a typed page input that the route state reads + as the source value and never writes back to the URL, and `Storage.tsx` + sets `suppressSourceFilter` whenever it is present. `StoragePageControls` + then drops the Source chip, so the user never sees the platform's name + pinned as a removable filter chip, and the locked source does not count + as an active filter that surfaces the toolbar's clear action. + `suppressNodeFilter` drops the Node chip so search carries host and + node scoping, and `Storage.tsx` resets any selected node to all nodes + while it is set. The embedded toolbar otherwise keeps search, the + Status filter on the Storage view (Role and Group filters on Physical + Disks once there is more than one choice), and the View popover's + grouping and sort; it has no chart-collapse control. The seven-state Storage status catalog stays in the shared Add filter menu instead of occupying a permanent segmented rail; a non-default status surfaces through the canonical FilterBar chip. Grouping and sort remain durable presentation diff --git a/frontend-modern/src/components/Storage/__tests__/Storage.test.tsx b/frontend-modern/src/components/Storage/__tests__/Storage.test.tsx index 5b4b65d38e..09adf8dfef 100644 --- a/frontend-modern/src/components/Storage/__tests__/Storage.test.tsx +++ b/frontend-modern/src/components/Storage/__tests__/Storage.test.tsx @@ -464,6 +464,65 @@ describe('Storage', () => { expect(screen.queryByText('Local-LVM-PVE2')).not.toBeInTheDocument(); }); + it('gives a platform embed the canonical toolbar with source and node scope locked', async () => { + mockLocationSearch = '?node=node-2&source=truenas'; + hookResources = [ + buildStorageResource('storage-1', 'Local-LVM-PVE1', 'pve1', { + parentId: 'node-1', + parentName: 'pve1', + includePlatformNode: false, + }), + buildStorageResource('storage-2', 'Local-LVM-PVE2', 'pve2', { + parentId: 'node-2', + parentName: 'pve2', + includePlatformNode: false, + }), + buildStorageResource('storage-3', 'Tank-TrueNAS', 'truenas-1', { + platformId: 'truenas-main', + platformType: 'truenas', + }), + ]; + + render(() => ( + + )); + + // No prop opts the embed into the toolbar or keeps it out of the view tabs. + expect(screen.getByRole('group', { name: 'Proxmox storage filters' })).toBeInTheDocument(); + expect( + screen.getByPlaceholderText('Search Proxmox storage by pool, datastore, node, or device'), + ).toBeInTheDocument(); + expect(screen.getByRole('tablist', { name: 'Storage view' })).toBeInTheDocument(); + + // The forced source outranks the URL source, and the URL node resets to all. + await waitFor(() => { + expect(screen.getByText('Local-LVM-PVE1')).toBeInTheDocument(); + expect(screen.getByText('Local-LVM-PVE2')).toBeInTheDocument(); + }); + expect(screen.queryByText('Tank-TrueNAS')).not.toBeInTheDocument(); + expect(navigateSpy.mock.calls.some(([path]) => String(path).includes('source='))).toBe(false); + + // Neither lock surfaces as a chip, an Add filter option, or a clear action. + expect(queryStorageChip('Source')).toBeNull(); + expect(queryStorageChip('Node')).toBeNull(); + const addFilter = screen.getByRole('combobox', { name: 'Filter' }); + expect(within(addFilter).getByRole('option', { name: 'Status: Warning' })).toBeInTheDocument(); + expect( + within(addFilter).queryByRole('option', { name: /^(Source|Node):/ }), + ).not.toBeInTheDocument(); + expect(screen.queryByRole('button', { name: 'Clear filters' })).not.toBeInTheDocument(); + expect(screen.queryByRole('button', { name: /charts/i })).not.toBeInTheDocument(); + + const viewOptions = openStorageViewOptions(); + expect(within(viewOptions).getByLabelText('Group by')).toBeInTheDocument(); + expect(within(viewOptions).getByLabelText('Sort by')).toBeInTheDocument(); + }); + it('renders per-pool growth from the shared storage summary history contract', async () => { const gib = 1024 * 1024 * 1024; const storageSummarySpy = vi.spyOn(ChartsAPI, 'getStorageSummaryCharts').mockResolvedValue({ @@ -1343,7 +1402,7 @@ describe('Storage', () => { ); }); - it('keeps the storage view selector available in table-only embedding', async () => { + it('keeps the storage view selector available in a platform-page embed', async () => { mockLocationPath = '/proxmox/storage'; hookResources = [ buildPhysicalDiskResource('sda', 'node-1', 'pve1'),