From 04f61d431f3a3dde78b72f581521225c1d01b020 Mon Sep 17 00:00:00 2001 From: rcourtman <8825017+rcourtman@users.noreply.github.com> Date: Thu, 8 Oct 2026 00:24:23 +0100 Subject: [PATCH] Remove the unreachable Workloads chart visibility toggle WorkloadsFilter rendered ChartVisibilityToggleButton only when a surface passed onChartsToggle, and none has since the workload summary chart section was deleted with the summary components. The button had no other production consumer, so the shared primitive, its two shared-template registry entries, the filter-model props and the tests that pinned them described a Show/Hide charts control that nobody could reach. Delete them. The guardrail that required the primitive now keeps the exact retired registry ids, export and props out: a returning summary section is a product decision that should register its own control. Contract prose in frontend-primitives, performance-and-scalability and unified-resources now says no chart show/hide control exists instead of describing the deleted one. No visible change: the Workloads View disclosure on the Proxmox and vSphere overviews shows the same sections and controls before and after, at desktop and phone widths. --- .../subsystems/frontend-primitives.md | 28 ++++---- .../subsystems/performance-and-scalability.md | 12 ++-- .../internal/subsystems/unified-resources.md | 10 +-- .../2096a46724dee366.json | 39 +++++++++++ .../scripts/shared-template-registry.json | 34 ---------- .../components/Workloads/WorkloadsFilter.tsx | 19 +----- .../__tests__/WorkloadsFilter.test.tsx | 31 --------- .../Workloads/workloadsFilterModel.ts | 2 - .../components/shared/FilterToolbar.test.tsx | 29 --------- .../src/components/shared/FilterToolbar.tsx | 28 -------- .../SharedPrimitives.guardrails.test.ts | 64 +++++-------------- 11 files changed, 82 insertions(+), 214 deletions(-) create mode 100644 frontend-modern/browser-verification/2096a46724dee366.json diff --git a/docs/release-control/v6/internal/subsystems/frontend-primitives.md b/docs/release-control/v6/internal/subsystems/frontend-primitives.md index e78920a444..572cd035db 100644 --- a/docs/release-control/v6/internal/subsystems/frontend-primitives.md +++ b/docs/release-control/v6/internal/subsystems/frontend-primitives.md @@ -730,8 +730,8 @@ history, and must not discard the saved note. Platform-owned workload controls extend the shared `WorkloadsFilter` view options rather than creating page-local toolbar shells. Persistent presentation 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 +primary filter rail: layout, metric style, row hover, memory basis, inventory +totals, 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. @@ -2638,12 +2638,17 @@ not a replacement status card, CTA band, or page-local nested card. breakpoint, so Recovery events, Workloads, Storage, Infrastructure, and future dense toolbars do not strand Filter/Columns/Reset actions as an isolated second-row fragment. Shared `FilterToolbarPanel` owns - default filter-popover geometry, and `FilterToolbar` owns the shared chart - visibility display action: Workloads, Storage, Infrastructure, and future - summary-bearing pages must use `ChartVisibilityToggleButton` so the - affordance exposes one `Show charts` / `Hide charts` pressed-state contract - instead of rebuilding a one-option segmented control or an in-summary - collapse chevron page by page. Feature state hooks under + default filter-popover geometry. The workload summary chart section that the + shared chart show/hide action toggled is deleted and no other page exposes + one, so the shared toolbar carries no chart show/hide action: the + `ChartVisibilityToggleButton` primitive, its + `chart-visibility-toggle-button` rule, and its + `chart-visibility-local-toggle-labels` pattern guard are retired, and + `SharedPrimitives.guardrails.test.ts` keeps them out. A summary section that + returns through a governed product decision must register one shared + show/hide primitive with an explicit pressed state rather than a page-local + one-option segmented control or in-summary collapse chevron. + Feature state hooks under `frontend-modern/src/features/` own route-backed query state, selected item state, and data-window selection for their product surfaces; shared primitives and reusable presentation helpers may own viewport-safe chrome, @@ -5764,11 +5769,6 @@ compose that primitive instead of copying rounded-full active selector styling. `ResourcePicker` report-domain filters are part of that boundary: the picker owns the reportable resource categories and labels, but the type selector shell must come from `FilterButtonGroup`. -Chart visibility display actions are registry-backed too. -`ChartVisibilityToggleButton` owns the `Show charts` / `Hide charts` label, -pressed-state, title, icon, and toolbar action styling for summary-bearing -filter surfaces. Pages must compose that primitive instead of recreating local -chart visibility buttons or one-option segmented controls. Column visibility controls are registry-backed too. `ColumnPicker` owns the column chooser trigger, panel title, reset action, empty-state copy, hidden count badge, dropdown width, and outside-click lifecycle through the shared @@ -5958,7 +5958,7 @@ of `PageControls`. Each page declares a `FilterDef[]` catalog (label, options, value, defaultValue, group); `FilterBar` renders chips for active filters and exposes the rest behind a "+ Filter" menu, with type-ahead at both the menu and chip popovers (`AddFilterMenu` and `FilterChip`). Low-frequency view options -(grouping segmented control, charts toggle, columns picker, sort key) compose +(grouping segmented control, columns picker, sort key) compose the shared `ViewOptionsDisclosure` through `FilterBar`'s `viewOptions` prop instead of remaining as permanent toolbar controls. `FilterBar` owns the View trigger and inline disclosure; feature consumers pass only panel content and 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 26a57d60c5..35f744c537 100644 --- a/docs/release-control/v6/internal/subsystems/performance-and-scalability.md +++ b/docs/release-control/v6/internal/subsystems/performance-and-scalability.md @@ -2809,12 +2809,12 @@ 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 column header after the View disclosure closes. No workload summary chart -section renders today, so the chart-visibility control in -`frontend-modern/src/components/Workloads/WorkloadsFilter.tsx` stays hidden -(no surface passes `onChartsToggle`). If a summary section returns, that -control must expose explicit `Show charts` / `Hide charts` pressed state, and -hiding charts must remove the summary section rather than leaving an empty -collapsed summary band on screen. +section renders today, and `WorkloadsFilter` carries no chart-visibility +control or `onChartsToggle` / `chartsCollapsed` props. A summary section that +returns through a governed product decision must bring its own visibility +control with explicit `Show charts` / `Hide charts` pressed state, and hiding +charts must remove the summary section rather than leaving an empty collapsed +summary band on screen. The Workloads-owned filter-config assembly now lives in `frontend-modern/src/components/Workloads/useWorkloadsState.ts`, so future filter runtime changes must extend through those owners instead of diff --git a/docs/release-control/v6/internal/subsystems/unified-resources.md b/docs/release-control/v6/internal/subsystems/unified-resources.md index 7ed66c3921..9e2477f000 100644 --- a/docs/release-control/v6/internal/subsystems/unified-resources.md +++ b/docs/release-control/v6/internal/subsystems/unified-resources.md @@ -2601,10 +2601,12 @@ application resource-provider or WebSocket lifecycle. in place through that shared runtime and the root app-shell restore path, instead of looking like a page refresh or remount. 12. Keep summary-chart visibility a display preference, not a unified-resource - filter. Platform/runtime pages may hide or restore chart sections through - shared presentation controls, but those controls must not mutate resource - identity, table membership, source scope, or summary-hover state. The - retired top-level `/infrastructure` page, its summary chart strip, and its + filter. The workload and infrastructure summary chart sections are + retired and no platform page exposes a chart show/hide control today. If a + governed product decision brings one back, hiding or restoring it must not + mutate resource identity, table membership, source scope, or summary-hover + state. The retired top-level + `/infrastructure` page, its summary chart strip, and its saved-view/route-state machinery must not be reintroduced for this purpose. 15. Keep operator-local resource search on the operator's own names. Resource-policy redaction is a transmission boundary (`docs/PRIVACY.md`), diff --git a/frontend-modern/browser-verification/2096a46724dee366.json b/frontend-modern/browser-verification/2096a46724dee366.json new file mode 100644 index 0000000000..6808f31593 --- /dev/null +++ b/frontend-modern/browser-verification/2096a46724dee366.json @@ -0,0 +1,39 @@ +{ + "version": 1, + "base_sha": "fd36869223864faae2dd0969a758cde6232f011c", + "verified_at": "2026-10-07T23:02:23Z", + "result": "passed", + "changed_paths": [ + "frontend-modern/src/components/Workloads/WorkloadsFilter.tsx", + "frontend-modern/src/components/Workloads/workloadsFilterModel.ts", + "frontend-modern/src/components/shared/FilterToolbar.tsx" + ], + "content_sha256": { + "frontend-modern/src/components/Workloads/WorkloadsFilter.tsx": "54f320df506ff23e31ebfb05c927d4ba1bf3760620962200dd19fad72636eeeb", + "frontend-modern/src/components/Workloads/workloadsFilterModel.ts": "09e9c57fc2358564c5f929f06ac28daced329ef961dff3c52df9554f9359c0c2", + "frontend-modern/src/components/shared/FilterToolbar.tsx": "8b7d3b4ffd7218042a2c24384f4175ffb0c9d2080f2a02cbca801b48b097bed6" + }, + "routes": ["/proxmox/overview", "/vmware/overview"], + "viewports": [ + { + "width": 1440, + "height": 900 + }, + { + "width": 390, + "height": 844 + } + ], + "states": [ + "Production vite builds of the change (after) and of main's three runtime files (before) served by vite preview on pulse-dev, proxied to the 50-node mock perf rig; the after bundle carries no 'Show charts' copy, the before bundle does", + "Proxmox overview, 929 guests: Workloads View preferences disclosure open on desktop (Layout, Metrics, Row hover, Memory relative to, Inventory totals, Table) and phone (Layout, Metrics, Memory relative to, Inventory totals, Table)", + "vSphere overview: View preferences disclosure open on desktop (Layout, Metrics, Row hover, Inventory totals, Table) and phone (Layout, Metrics, Inventory totals, Table)", + "Before and after identical in every case: same section headings, same controls and pressed states, no Summary heading, no Charts / Show charts / Hide charts button, no horizontal page overflow, no console or page errors" + ], + "interactions": [ + "Signed in to the dev rig, opened /proxmox/overview and /vmware/overview at each viewport", + "Desktop: clicked View to open the inline View preferences disclosure", + "Phone: opened Filters, then View, to reach the same disclosure", + "Captured every visible heading, button and pressed state inside the disclosure plus page-wide Charts buttons and Summary labels, then diffed before against after" + ] +} diff --git a/frontend-modern/scripts/shared-template-registry.json b/frontend-modern/scripts/shared-template-registry.json index 44cb11003f..b66bda34e3 100644 --- a/frontend-modern/scripts/shared-template-registry.json +++ b/frontend-modern/scripts/shared-template-registry.json @@ -3241,21 +3241,6 @@ "scripts/shared-template-audit.mjs" ] }, - { - "id": "chart-visibility-toggle-button", - "category": "display-toggle", - "summary": "Summary-bearing filter surfaces must compose ChartVisibilityToggleButton for chart show/hide display actions instead of recreating one-off chart toggle buttons.", - "canonical": { - "path": "src/components/shared/FilterToolbar.tsx", - "export": "ChartVisibilityToggleButton" - }, - "requiredConsumers": [{ "path": "src/components/Workloads/WorkloadsFilter.tsx" }], - "proof": [ - "src/components/shared/SharedPrimitives.guardrails.test.ts", - "src/components/shared/FilterToolbar.test.tsx", - "scripts/shared-template-audit.mjs" - ] - }, { "id": "column-picker-shell", "category": "column-picker", @@ -4787,25 +4772,6 @@ "scripts/shared-template-audit.mjs" ] }, - { - "id": "chart-visibility-local-toggle-labels", - "category": "display-toggle", - "summary": "Summary-bearing surfaces must not copy the Show charts / Hide charts display-toggle labels owned by ChartVisibilityToggleButton.", - "canonical": { - "path": "src/components/shared/FilterToolbar.tsx", - "export": "ChartVisibilityToggleButton" - }, - "scopes": ["src/components/Workloads", "src/features", "src/pages"], - "extensions": [".tsx"], - "allPatterns": ["Show charts", "Hide charts"], - "legacyReason": "Retired migration debt. Chart visibility display actions must use ChartVisibilityToggleButton; local show/hide chart toggle copies are not allowed.", - "allowedPaths": [], - "ignoredPaths": ["src/components/Workloads/__tests__/WorkloadsFilter.test.tsx"], - "proof": [ - "src/components/shared/SharedPrimitives.guardrails.test.ts", - "scripts/shared-template-audit.mjs" - ] - }, { "id": "column-picker-local-column-chooser", "category": "column-picker", diff --git a/frontend-modern/src/components/Workloads/WorkloadsFilter.tsx b/frontend-modern/src/components/Workloads/WorkloadsFilter.tsx index 34d226b784..fee0c80fb0 100644 --- a/frontend-modern/src/components/Workloads/WorkloadsFilter.tsx +++ b/frontend-modern/src/components/Workloads/WorkloadsFilter.tsx @@ -10,11 +10,7 @@ import { type FilterDef, type FilterSelectOption, } from '@/components/shared/FilterBar'; -import { - ChartVisibilityToggleButton, - FilterActionButton, - FilterSegmentedControl, -} from '@/components/shared/FilterToolbar'; +import { FilterActionButton, FilterSegmentedControl } from '@/components/shared/FilterToolbar'; import { GroupedTableModeSegmentedControl } from '@/components/shared/GroupedTableModeSegmentedControl'; import { useBreakpoint } from '@/hooks/useBreakpoint'; import { usePersistentSignal } from '@/hooks/usePersistentSignal'; @@ -426,19 +422,6 @@ export const WorkloadsFilter: Component = (props) => { - -
-
- Summary -
- props.onChartsToggle?.()} - /> -
- - {(visibility) => (
diff --git a/frontend-modern/src/components/Workloads/__tests__/WorkloadsFilter.test.tsx b/frontend-modern/src/components/Workloads/__tests__/WorkloadsFilter.test.tsx index d74fcddb71..b8644fb976 100644 --- a/frontend-modern/src/components/Workloads/__tests__/WorkloadsFilter.test.tsx +++ b/frontend-modern/src/components/Workloads/__tests__/WorkloadsFilter.test.tsx @@ -931,37 +931,6 @@ describe('WorkloadsFilter', () => { }); }); - describe('charts toggle', () => { - it('renders the Charts button when onChartsToggle is provided', () => { - render(() => ( - false), - onChartsToggle: vi.fn(), - })} - /> - )); - expect(screen.queryByRole('button', { name: 'Hide charts' })).not.toBeInTheDocument(); - expect( - within(openViewPreferences()).getByRole('button', { name: 'Hide charts' }), - ).toBeInTheDocument(); - }); - - it('labels the Charts button as a show action when charts are collapsed', () => { - render(() => ( - true), - onChartsToggle: vi.fn(), - })} - /> - )); - expect( - within(openViewPreferences()).getByRole('button', { name: 'Show charts' }), - ).toBeInTheDocument(); - }); - }); - describe('column picker', () => { it('renders ColumnPicker when columnVisibility is provided', () => { render(() => ( diff --git a/frontend-modern/src/components/Workloads/workloadsFilterModel.ts b/frontend-modern/src/components/Workloads/workloadsFilterModel.ts index 87c0ece4be..766269bc0b 100644 --- a/frontend-modern/src/components/Workloads/workloadsFilterModel.ts +++ b/frontend-modern/src/components/Workloads/workloadsFilterModel.ts @@ -75,8 +75,6 @@ export interface WorkloadsFilterProps { namespaceFilter?: WorkloadsToolbarFilterConfig; clusterFilter?: WorkloadsToolbarFilterConfig; containerRuntimeFilter?: WorkloadsToolbarFilterConfig; - chartsCollapsed?: () => boolean; - onChartsToggle?: () => void; metricDisplayMode?: () => WorkloadsMetricDisplayMode; setMetricDisplayMode?: (value: WorkloadsMetricDisplayMode) => void; metricHoverMode?: () => WorkloadsMetricHoverMode; diff --git a/frontend-modern/src/components/shared/FilterToolbar.test.tsx b/frontend-modern/src/components/shared/FilterToolbar.test.tsx index 5c70e44310..0eddf84091 100644 --- a/frontend-modern/src/components/shared/FilterToolbar.test.tsx +++ b/frontend-modern/src/components/shared/FilterToolbar.test.tsx @@ -2,7 +2,6 @@ import { cleanup, render, screen, waitFor } from '@solidjs/testing-library'; import { For, createSignal } from 'solid-js'; import { afterEach, describe, expect, it, vi } from 'vitest'; import { - ChartVisibilityToggleButton, FilterHeader, FilterPopoverTrigger, FilterToolbarPanel, @@ -124,34 +123,6 @@ describe('FilterHeader', () => { ).toBe(false); }); - it('keeps chart visibility as an explicit display toggle action', () => { - const [collapsed, setCollapsed] = createSignal(false); - const onToggle = vi.fn(() => setCollapsed((current) => !current)); - - render(() => ( - - )); - - const hideButton = screen.getByRole('button', { name: 'Hide charts' }); - expect(hideButton).toHaveTextContent('Charts'); - expect(hideButton).toHaveAttribute('aria-pressed', 'true'); - expect(hideButton).toHaveAttribute('title', 'Hide charts'); - expect(hideButton).toHaveClass('hidden'); - expect(hideButton).toHaveClass('lg:inline-flex'); - - hideButton.click(); - - const showButton = screen.getByRole('button', { name: 'Show charts' }); - expect(onToggle).toHaveBeenCalledTimes(1); - expect(showButton).toHaveTextContent('Charts'); - expect(showButton).toHaveAttribute('aria-pressed', 'false'); - expect(showButton).toHaveAttribute('title', 'Show charts'); - }); - it('keeps shared toggle behavior on shell, runtime, and model owners', () => { expect(toggleSource).toContain('useToggleState'); expect(toggleSource).toContain('getToggleTrackClass'); diff --git a/frontend-modern/src/components/shared/FilterToolbar.tsx b/frontend-modern/src/components/shared/FilterToolbar.tsx index 8545a6bb4a..17809c161e 100644 --- a/frontend-modern/src/components/shared/FilterToolbar.tsx +++ b/frontend-modern/src/components/shared/FilterToolbar.tsx @@ -1,5 +1,4 @@ import { Component, For, JSX, Show, splitProps } from 'solid-js'; -import BarChartIcon from 'lucide-solid/icons/bar-chart'; import ChevronDownIcon from 'lucide-solid/icons/chevron-down'; import ListFilterIcon from 'lucide-solid/icons/list-filter'; import { FormSelect } from './FormSelect'; @@ -410,33 +409,6 @@ export const FilterPopoverTrigger: Component = (props ); }; -interface ChartVisibilityToggleButtonProps extends Omit< - JSX.ButtonHTMLAttributes, - 'aria-label' | 'aria-pressed' | 'children' | 'onClick' | 'title' -> { - collapsed: boolean; - onToggle: () => void; -} - -export const ChartVisibilityToggleButton: Component = (props) => { - const [local, buttonProps] = splitProps(props, ['collapsed', 'onToggle', 'class']); - const label = () => (local.collapsed ? 'Show charts' : 'Hide charts'); - return ( - local.onToggle()} - > - - Charts - - ); -}; - interface FilterToolbarPanelProps extends JSX.HTMLAttributes { children: JSX.Element; widthClass?: string; diff --git a/frontend-modern/src/components/shared/SharedPrimitives.guardrails.test.ts b/frontend-modern/src/components/shared/SharedPrimitives.guardrails.test.ts index c90210a1e6..e4eab0423f 100644 --- a/frontend-modern/src/components/shared/SharedPrimitives.guardrails.test.ts +++ b/frontend-modern/src/components/shared/SharedPrimitives.guardrails.test.ts @@ -1573,56 +1573,25 @@ describe('shared primitive guardrails', () => { expect(storagePoolsTableSource).not.toContain('HEADER_SORT_BUTTON'); }); - it('keeps chart visibility display actions on the shared toolbar toggle', () => { + it('keeps the retired summary chart-visibility toggle out of the shared toolbar', () => { + // The workload summary chart section this toggle controlled is deleted and + // no other page exposes a chart show/hide control. A returning summary + // section is a governed product decision that must register its own + // primitive. const registry = JSON.parse(sharedTemplateRegistrySource) as { - rules?: Array<{ - id: string; - canonical?: { path?: string; export?: string }; - requiredConsumers?: Array<{ path?: string }>; - }>; - patternGuards?: Array<{ - id: string; - canonical?: { path?: string; export?: string }; - allPatterns?: string[]; - scopes?: string[]; - pathIncludes?: string[]; - pathExcludes?: string[]; - allowedPaths?: string[]; - ignoredPaths?: string[]; - }>; + rules?: Array<{ id: string; canonical?: { export?: string } }>; + patternGuards?: Array<{ id: string; canonical?: { export?: string } }>; }; - const registeredRule = registry.rules?.find( - (rule) => rule.id === 'chart-visibility-toggle-button', - ); - const registeredGuard = registry.patternGuards?.find( - (guard) => guard.id === 'chart-visibility-local-toggle-labels', - ); + const retiredIds = ['chart-visibility-toggle-button', 'chart-visibility-local-toggle-labels']; + for (const entry of [...(registry.rules ?? []), ...(registry.patternGuards ?? [])]) { + expect(retiredIds).not.toContain(entry.id); + expect(entry.canonical?.export).not.toBe('ChartVisibilityToggleButton'); + } - expect(registeredRule?.canonical?.path).toBe('src/components/shared/FilterToolbar.tsx'); - expect(registeredRule?.canonical?.export).toBe('ChartVisibilityToggleButton'); - expect(registeredRule?.requiredConsumers?.map((consumer) => consumer.path)).toEqual([ - 'src/components/Workloads/WorkloadsFilter.tsx', - ]); - expect(registeredGuard?.canonical?.path).toBe('src/components/shared/FilterToolbar.tsx'); - expect(registeredGuard?.canonical?.export).toBe('ChartVisibilityToggleButton'); - expect(registeredGuard?.allPatterns).toEqual(['Show charts', 'Hide charts']); - expect(registeredGuard?.allowedPaths ?? []).toHaveLength(0); - expect(registeredGuard?.ignoredPaths).toEqual([ - 'src/components/Workloads/__tests__/WorkloadsFilter.test.tsx', - ]); - expect(registeredGuard?.scopes).toEqual( - expect.arrayContaining(['src/components/Workloads', 'src/features', 'src/pages']), - ); - - expect(filterToolbarSource).toContain('export const ChartVisibilityToggleButton'); - expect(filterToolbarSource).toContain("local.collapsed ? 'Show charts' : 'Hide charts'"); - expect(filterToolbarSource).toContain('active={!local.collapsed}'); - expect(filterToolbarSource).toContain('aria-pressed={!local.collapsed}'); - expect(filterToolbarSource).toContain('title={label()}'); - - expect(workloadsFilterSource).toContain('ChartVisibilityToggleButton'); - expect(workloadsFilterSource).not.toContain('Show charts'); - expect(workloadsFilterSource).not.toContain('Hide charts'); + expect(filterToolbarSource).not.toContain('ChartVisibilityToggleButton'); + expect(workloadsFilterSource).not.toContain('ChartVisibilityToggleButton'); + expect(workloadsFilterSource).not.toContain('onChartsToggle'); + expect(workloadsFilterSource).not.toContain('chartsCollapsed'); }); it('keeps grouped/list table-mode controls on one shared presentation contract', () => { @@ -8667,7 +8636,6 @@ describe('shared primitive guardrails', () => { expect(workloadsFilterSource).toContain('trailingControls={'); expect(workloadsFilterSource).not.toContain('ViewOptionsMenu'); expect(workloadsFilterSource).toContain('GroupedTableModeSegmentedControl'); - expect(workloadsFilterSource).toContain('ChartVisibilityToggleButton'); expect(workloadsFilterSource).toContain('