mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-09-09 18:15:50 +00:00
Keep Storage table headers plain
This commit is contained in:
rcourtman
@@ -913,10 +913,14 @@ the shared `TableHead` primitive and expose column state through `aria-sort`;
|
||||
the feature owner may define the sort keys and data comparator, but the header
|
||||
interaction must update that canonical owner state rather than forking
|
||||
table-local sort state or making header labels look clickable while inert.
|
||||
Storage pool headers are intentionally presentation-only rather than sortable
|
||||
headers: their one canonical sort interaction lives in `StoragePageControls`
|
||||
under `View` -> `Order`, so the table headings remain plain labels without a
|
||||
second hidden or competing sort affordance.
|
||||
Sortable header direction presentation belongs to
|
||||
`frontend-modern/src/components/shared/tableSortPresentation.ts`. Inactive
|
||||
sortable columns stay visually quiet; only the active column renders its
|
||||
ascending or descending marker. Storage, Workloads, backup, and shared platform
|
||||
ascending or descending marker. Workloads, backup, and shared platform
|
||||
tables must consume that helper rather than repeating dormant up/down icons in
|
||||
every header or defining page-local direction glyphs.
|
||||
|
||||
|
||||
@@ -3352,11 +3352,13 @@ storage-local raw `<select>` wrapper. Grouping, sort key, and sort direction are
|
||||
durable presentation preferences, so `StoragePageControls` supplies them
|
||||
through the frontend-primitives-owned `FilterBar.viewOptions` content boundary
|
||||
instead of leaving them as permanent filter-rail controls.
|
||||
Sortable storage and Proxmox backup headers keep their real button and
|
||||
`aria-sort` semantics, but their visible direction marker must come from the
|
||||
frontend-primitives-owned `tableSortPresentation.ts` helper. Inactive columns
|
||||
must not repeat dormant up/down icons across the header row; only the active
|
||||
sort column displays its direction.
|
||||
Storage pool table headings are presentation-only labels. Sorting remains
|
||||
available through the canonical `StoragePageControls` `View` -> `Order`
|
||||
controls, so those headings must not duplicate that interaction with buttons,
|
||||
`aria-sort`, or visible direction markers. Sortable Proxmox backup headers keep
|
||||
their real button and `aria-sort` semantics, but their visible direction marker
|
||||
must come from the frontend-primitives-owned `tableSortPresentation.ts` helper;
|
||||
only the active backup sort column displays its direction.
|
||||
Recovery all-history, all-item-type, and all-platform defaults follow the same
|
||||
shared filter-option contract through
|
||||
`frontend-modern/src/utils/recoveryTablePresentation.ts`, so recovery history
|
||||
|
||||
@@ -1,11 +1,17 @@
|
||||
{
|
||||
"version": 1,
|
||||
"base_sha": "3d77ac9cec60d55d3c1e732e748d21f60f32b1da",
|
||||
"verified_at": "2026-08-24T16:15:55Z",
|
||||
"base_sha": "baabed91a61acc7b4262c5170e3f5f5a6b457ab1",
|
||||
"verified_at": "2026-08-24T16:27:42Z",
|
||||
"result": "passed",
|
||||
"changed_paths": ["frontend-modern/src/components/Storage/StorageGroupRow.tsx"],
|
||||
"changed_paths": [
|
||||
"frontend-modern/src/components/Storage/Storage.tsx",
|
||||
"frontend-modern/src/components/Storage/StorageContentCard.tsx",
|
||||
"frontend-modern/src/components/Storage/StoragePoolsTable.tsx"
|
||||
],
|
||||
"content_sha256": {
|
||||
"frontend-modern/src/components/Storage/StorageGroupRow.tsx": "e6ef06dae2236e5c414de92aeab356b96fc2ed795f97d572d80e0e2f00674792"
|
||||
"frontend-modern/src/components/Storage/Storage.tsx": "68ec929f7daf54b809fb9fa508e35ed73613666e242df41cd2c147bdcdb761af",
|
||||
"frontend-modern/src/components/Storage/StorageContentCard.tsx": "4ec6d2d81673e02e8937695cfe59773f548b5c052554245e44deffc0d44f03af",
|
||||
"frontend-modern/src/components/Storage/StoragePoolsTable.tsx": "d23551a00a460075996ac78eed48d7cf190dcd40adace7fcfdaf2fa2034b2797"
|
||||
},
|
||||
"routes": ["/proxmox/storage"],
|
||||
"viewports": [
|
||||
@@ -19,13 +25,15 @@
|
||||
}
|
||||
],
|
||||
"states": [
|
||||
"Proxmox Storage grouped by node with expanded storage group rows",
|
||||
"Proxmox Storage grouped by node after collapsing a group through the row label"
|
||||
"Proxmox Storage with plain desktop table headings",
|
||||
"Proxmox Storage after sorting by usage and changing direction through View preferences",
|
||||
"Proxmox Storage compact phone table with Filters and View preferences open"
|
||||
],
|
||||
"interactions": [
|
||||
"opened mobile Filters and View preferences, then selected Group by node",
|
||||
"confirmed storage group disclosure buttons are visually hidden at 390 pixels while retaining accessible expand and collapse labels",
|
||||
"clicked the Analytics A group label and confirmed the whole row collapsed the group without a visible chevron",
|
||||
"confirmed ordinary storage pool row disclosure controls remain visually hidden at the mobile breakpoint"
|
||||
"confirmed every desktop Storage table heading contains no button, aria-sort state, or visible direction marker",
|
||||
"selected Usage in View -> Order and changed the sort direction, confirming the rendered row order changed while headings remained plain",
|
||||
"confirmed compact phone headings render Storage, State, Type, Host, Prot, and Used without direction markers",
|
||||
"opened phone Filters -> View and confirmed Sort by and Sort direction remain available",
|
||||
"confirmed the page has no horizontal overflow at 390 pixels"
|
||||
]
|
||||
}
|
||||
|
||||
@@ -148,10 +148,6 @@ const Storage: Component<StorageProps> = (props) => {
|
||||
search={search}
|
||||
groupedRecords={groupedRecords}
|
||||
groupBy={groupBy}
|
||||
sortKey={sortKey}
|
||||
setSortKey={setSortKey}
|
||||
sortDirection={sortDirection}
|
||||
setSortDirection={setSortDirection}
|
||||
expandedGroups={expandedGroups}
|
||||
toggleGroup={toggleGroup}
|
||||
expandedPoolId={expandedPoolId}
|
||||
|
||||
@@ -7,7 +7,7 @@ import { STORAGE_CONTENT_CARD_BODY_CLASS } from '@/features/storageBackups/stora
|
||||
import type { StorageCapacityDeltaPresentation } from '@/features/storageBackups/storageCapacityDeltaPresentation';
|
||||
import type { Resource } from '@/types/resource';
|
||||
import type { StorageHealthFilter } from '@/features/storageBackups/models';
|
||||
import type { StorageGroupKey, StorageGroupedRecords, StorageSortKey } from './useStorageModel';
|
||||
import type { StorageGroupKey, StorageGroupedRecords } from './useStorageModel';
|
||||
import type { StorageAlertRowState } from '@/features/storageBackups/storageAlertState';
|
||||
import type { StorageView } from './storagePageState';
|
||||
import { useStorageContentCardModel } from './useStorageContentCardModel';
|
||||
@@ -25,10 +25,6 @@ type StorageContentCardProps = {
|
||||
search: () => string;
|
||||
groupedRecords: () => StorageGroupedRecords[];
|
||||
groupBy: () => StorageGroupKey;
|
||||
sortKey: () => StorageSortKey;
|
||||
setSortKey: (value: StorageSortKey) => void;
|
||||
sortDirection: () => 'asc' | 'desc';
|
||||
setSortDirection: (value: 'asc' | 'desc') => void;
|
||||
expandedGroups: () => Set<string>;
|
||||
toggleGroup: (key: string) => void;
|
||||
expandedPoolId: () => string | null;
|
||||
@@ -97,10 +93,6 @@ export const StorageContentCard: Component<StorageContentCardProps> = (props) =>
|
||||
<StoragePoolsTable
|
||||
groupedRecords={props.groupedRecords()}
|
||||
groupBy={props.groupBy()}
|
||||
sortKey={props.sortKey()}
|
||||
setSortKey={props.setSortKey}
|
||||
sortDirection={props.sortDirection()}
|
||||
setSortDirection={props.setSortDirection}
|
||||
expandedGroups={props.expandedGroups()}
|
||||
toggleGroup={props.toggleGroup}
|
||||
expandedPoolId={props.expandedPoolId()}
|
||||
|
||||
@@ -7,7 +7,6 @@ import {
|
||||
TableHeader,
|
||||
TableRow,
|
||||
} from '@/components/shared/Table';
|
||||
import { getTableSortIndicator } from '@/components/shared/tableSortPresentation';
|
||||
import { getPlatformTableHeadClassForKind } from '@/features/platformPage/sharedPlatformPage';
|
||||
import {
|
||||
getStoragePoolColumnWidthPercent,
|
||||
@@ -29,8 +28,7 @@ import type { StorageAlertRowState } from '@/features/storageBackups/storageAler
|
||||
import type { Resource } from '@/types/resource';
|
||||
import { StorageGroupRow } from './StorageGroupRow';
|
||||
import { StoragePoolRow } from './StoragePoolRow';
|
||||
import { getDefaultStorageSortDirection } from './storagePageState';
|
||||
import type { StorageGroupedRecords, StorageGroupKey, StorageSortKey } from './useStorageModel';
|
||||
import type { StorageGroupedRecords, StorageGroupKey } from './useStorageModel';
|
||||
import { useStoragePoolsTableModel } from './useStoragePoolsTableModel';
|
||||
import type { SummarySeriesGroupScope } from '@/components/shared/summaryCardInteraction';
|
||||
import { resolveSummaryGroupMemberInteractionState } from '@/components/shared/summaryCardInteraction';
|
||||
@@ -40,10 +38,6 @@ import { useStoragePoolsTableWindowing } from './useStoragePoolsTableWindowing';
|
||||
type StoragePoolsTableProps = {
|
||||
groupedRecords: StorageGroupedRecords[];
|
||||
groupBy: StorageGroupKey;
|
||||
sortKey: StorageSortKey;
|
||||
setSortKey: (value: StorageSortKey) => void;
|
||||
sortDirection: 'asc' | 'desc';
|
||||
setSortDirection: (value: 'asc' | 'desc') => void;
|
||||
expandedGroups: Set<string>;
|
||||
toggleGroup: (key: string) => void;
|
||||
expandedPoolId: string | null;
|
||||
@@ -65,29 +59,6 @@ type StoragePoolsTableProps = {
|
||||
onHoverChange?: (recordId: string | null) => void;
|
||||
};
|
||||
|
||||
const STORAGE_POOL_HEADER_SORT_BUTTON_CLASS =
|
||||
'inline-flex min-w-0 max-w-full items-center gap-1 rounded-sm text-left outline-none transition-colors hover:text-base-content focus-visible:ring-2 focus-visible:ring-blue-500 focus-visible:ring-offset-1 focus-visible:ring-offset-surface';
|
||||
|
||||
const getNextStorageColumnSortDirection = (
|
||||
currentSortKey: StorageSortKey,
|
||||
currentSortDirection: 'asc' | 'desc',
|
||||
columnSortKey: StorageSortKey,
|
||||
): 'asc' | 'desc' => {
|
||||
if (currentSortKey !== columnSortKey) {
|
||||
return getDefaultStorageSortDirection(columnSortKey);
|
||||
}
|
||||
return currentSortDirection === 'asc' ? 'desc' : 'asc';
|
||||
};
|
||||
|
||||
const getStorageColumnSortButtonLabel = (
|
||||
label: string,
|
||||
isSorted: boolean,
|
||||
direction: 'asc' | 'desc',
|
||||
): string => {
|
||||
if (!isSorted) return `Sort ${label} column`;
|
||||
return `Sort ${label} column ${direction === 'asc' ? 'descending' : 'ascending'}`;
|
||||
};
|
||||
|
||||
export const StoragePoolsTable: Component<StoragePoolsTableProps> = (props) => {
|
||||
const tableWidth = useObservedElementWidth();
|
||||
const layoutMode = createMemo(() =>
|
||||
@@ -114,13 +85,6 @@ export const StoragePoolsTable: Component<StoragePoolsTableProps> = (props) => {
|
||||
expandedPoolId: () => props.expandedPoolId,
|
||||
});
|
||||
|
||||
const handleSort = (sortKey: StorageSortKey) => {
|
||||
props.setSortDirection(
|
||||
getNextStorageColumnSortDirection(props.sortKey, props.sortDirection, sortKey),
|
||||
);
|
||||
props.setSortKey(sortKey);
|
||||
};
|
||||
|
||||
return (
|
||||
<Show
|
||||
when={props.isLoading}
|
||||
@@ -162,41 +126,14 @@ export const StoragePoolsTable: Component<StoragePoolsTableProps> = (props) => {
|
||||
)}
|
||||
data-storage-column={column.id}
|
||||
aria-label={column.label}
|
||||
aria-sort={
|
||||
props.sortKey === column.sortKey
|
||||
? props.sortDirection === 'asc'
|
||||
? 'ascending'
|
||||
: 'descending'
|
||||
: undefined
|
||||
}
|
||||
title={column.label}
|
||||
>
|
||||
<button
|
||||
type="button"
|
||||
class={STORAGE_POOL_HEADER_SORT_BUTTON_CLASS}
|
||||
onClick={() => handleSort(column.sortKey)}
|
||||
aria-label={getStorageColumnSortButtonLabel(
|
||||
column.label,
|
||||
props.sortKey === column.sortKey,
|
||||
props.sortDirection,
|
||||
)}
|
||||
title={getStorageColumnSortButtonLabel(
|
||||
column.label,
|
||||
props.sortKey === column.sortKey,
|
||||
props.sortDirection,
|
||||
)}
|
||||
<Show
|
||||
when={layoutMode() === 'full'}
|
||||
fallback={<span class="min-w-0 truncate">{column.compactLabel}</span>}
|
||||
>
|
||||
<Show
|
||||
when={layoutMode() === 'full'}
|
||||
fallback={<span class="min-w-0 truncate">{column.compactLabel}</span>}
|
||||
>
|
||||
<span class="min-w-0 truncate">{column.label}</span>
|
||||
</Show>
|
||||
{getTableSortIndicator(
|
||||
props.sortKey === column.sortKey,
|
||||
props.sortDirection,
|
||||
)}
|
||||
</button>
|
||||
<span class="min-w-0 truncate">{column.label}</span>
|
||||
</Show>
|
||||
</TableHead>
|
||||
)}
|
||||
</For>
|
||||
|
||||
@@ -541,7 +541,7 @@ describe('Storage', () => {
|
||||
storageSummarySpy.mockRestore();
|
||||
});
|
||||
|
||||
it('renders compact table columns and supports sort/group controls', async () => {
|
||||
it('renders plain compact table headings and supports View sort/group controls', async () => {
|
||||
hookResources = [
|
||||
buildStorageResource('storage-1', 'Alpha-Store', 'pve1', {
|
||||
current: 20,
|
||||
@@ -594,23 +594,27 @@ describe('Storage', () => {
|
||||
).toBe(true);
|
||||
|
||||
const usageHeader = screen.getByRole('columnheader', { name: 'Usage' });
|
||||
const sortableHeaderButtons = screen.getAllByRole('button', { name: /^Sort .* column/ });
|
||||
expect(within(usageHeader).getByRole('button')).not.toHaveTextContent(/[▲▼]/);
|
||||
|
||||
fireEvent.click(within(usageHeader).getByRole('button', { name: 'Sort Usage column' }));
|
||||
expect(usageHeader).toHaveAttribute('aria-sort', 'descending');
|
||||
expect(within(usageHeader).getByRole('button')).toHaveTextContent('▼');
|
||||
expect(within(usageHeader).queryByRole('button')).not.toBeInTheDocument();
|
||||
expect(usageHeader).not.toHaveAttribute('aria-sort');
|
||||
expect(screen.queryByRole('button', { name: /^Sort .* column/ })).not.toBeInTheDocument();
|
||||
expect(
|
||||
sortableHeaderButtons.filter((button) => /[▲▼]/.test(button.textContent ?? '')),
|
||||
).toHaveLength(1);
|
||||
screen.getAllByRole('columnheader').some((header) => /[▲▼]/.test(header.textContent ?? '')),
|
||||
).toBe(false);
|
||||
|
||||
const viewOptions = openStorageViewOptions();
|
||||
setStorageViewOption(viewOptions, 'Sort by', 'usage');
|
||||
expect((within(viewOptions).getByLabelText('Sort by') as HTMLSelectElement).value).toBe(
|
||||
'usage',
|
||||
);
|
||||
|
||||
fireEvent.click(
|
||||
within(usageHeader).getByRole('button', { name: 'Sort Usage column ascending' }),
|
||||
);
|
||||
await waitFor(() => {
|
||||
const orderedRowIds = Array.from(document.querySelectorAll('tr[data-row-id]')).map((row) =>
|
||||
row.getAttribute('data-row-id'),
|
||||
);
|
||||
expect(orderedRowIds.slice(0, 2)).toEqual(['storage-2', 'storage-1']);
|
||||
});
|
||||
|
||||
fireEvent.click(within(viewOptions).getByRole('button', { name: 'Sort direction' }));
|
||||
|
||||
await waitFor(() => {
|
||||
const orderedRowIds = Array.from(document.querySelectorAll('tr[data-row-id]')).map((row) =>
|
||||
@@ -897,6 +901,11 @@ describe('Storage', () => {
|
||||
'/proxmox/storage?group=node&summaryGroup=storage%3Anode%3Apve1',
|
||||
ROUTE_STATE_REPLACE_OPTIONS,
|
||||
);
|
||||
});
|
||||
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Expand pve1' }));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(
|
||||
document.querySelectorAll('tr[data-summary-group-member-active="pinned"]'),
|
||||
).toHaveLength(1);
|
||||
@@ -938,6 +947,7 @@ describe('Storage', () => {
|
||||
render(() => <Storage />);
|
||||
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Expand tank' }));
|
||||
fireEvent.click(await screen.findByRole('tab', { name: 'History' }));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(metricsHistorySpy).toHaveBeenCalledWith(
|
||||
|
||||
@@ -1456,16 +1456,16 @@ describe('shared primitive guardrails', () => {
|
||||
for (const source of [
|
||||
sharedPlatformPageSource,
|
||||
workloadTableHeaderSource,
|
||||
storagePoolsTableSource,
|
||||
proxmoxBackupsTableSharedSource,
|
||||
]) {
|
||||
expect(source).toContain('getTableSortIndicator');
|
||||
}
|
||||
|
||||
for (const source of [storagePoolsTableSource, proxmoxBackupsTableSharedSource]) {
|
||||
expect(source).not.toContain('ArrowUpDownIcon');
|
||||
expect(source).not.toContain('SORT_ICON_CLASS');
|
||||
}
|
||||
expect(proxmoxBackupsTableSharedSource).not.toContain('ArrowUpDownIcon');
|
||||
expect(proxmoxBackupsTableSharedSource).not.toContain('SORT_ICON_CLASS');
|
||||
expect(storagePoolsTableSource).not.toContain('getTableSortIndicator');
|
||||
expect(storagePoolsTableSource).not.toContain('aria-sort');
|
||||
expect(storagePoolsTableSource).not.toContain('HEADER_SORT_BUTTON');
|
||||
});
|
||||
|
||||
it('keeps chart visibility display actions on the shared toolbar toggle', () => {
|
||||
|
||||
Reference in New Issue
Block a user