From b22571f19629f1886f442f85a8770b7b3265606c Mon Sep 17 00:00:00 2001 From: rcourtman <8825017+rcourtman@users.noreply.github.com> Date: Sat, 3 Oct 2026 01:01:00 +0100 Subject: [PATCH] Let table callers own the cell padding they ask for TableCell and TableHead composed `px-2 sm:px-3 py-*` ahead of the caller's class, but Tailwind emits every padding utility at the same specificity in spacing-scale order, so a caller's `px-1` lost to the base `px-2` and `sm:px-1.5` lost to `sm:px-3` wherever it sat in the string. Measured on 2026-10-02: DiskList cells carried 16px of horizontal padding on a 390px phone although their class asked for 8px, and 24px at 640-1023px although it asked for 12px; the Workloads header cells sat at 8/12px above data cells at 6/8px; the platform helper's `px-1.5 sm:px-2` had never taken effect on a cell since it was introduced in May. PR #2409 and PR #2418 worked around it with layout-mode `!px-1` overrides. Apply the convention TableHeader already uses for borders and TableBody for dividers to padding: when the caller's class names a side's padding (`p-*`, `px-*`, `pl-*`, `pr-*`, `py-*`, `pt-*`, `pb-*`, with or without `!`), the base padding for that side is left out, so the caller's utility is the only one on the element. Sides the caller leaves alone keep the base, so a `pr-2` column still gets its left inset, and prefixed-only variants such as `lg:px-0` keep layering on the base as before. The base is unchanged when nothing is passed, and the class is resolved inside the JSX attribute so layout-mode class switches stay reactive. Platform tables pick up the helper's declared 6/8px cell padding, which lines data cells up under the 6px headers the stylesheet already pins, and their header rows drop to the helper's 2px vertical padding. --- .../subsystems/frontend-primitives.md | 11 ++ frontend-modern/browser-verification.json | 102 +++++++++---- .../src/components/shared/Table.tsx | 62 +++++++- .../shared/__tests__/PulseDataGrid.test.tsx | 31 ++++ .../shared/__tests__/Table.test.tsx | 134 ++++++++++++++++++ 5 files changed, 314 insertions(+), 26 deletions(-) diff --git a/docs/release-control/v6/internal/subsystems/frontend-primitives.md b/docs/release-control/v6/internal/subsystems/frontend-primitives.md index 5572c24252..9943a05f79 100644 --- a/docs/release-control/v6/internal/subsystems/frontend-primitives.md +++ b/docs/release-control/v6/internal/subsystems/frontend-primitives.md @@ -5526,6 +5526,17 @@ styling. `TableHeader` and `TableBody` may provide canonical default borders and dividers, but when a caller supplies explicit border or divide classes the shared primitive must defer to that local contract instead of silently forcing the default separator treatment back into the rendered DOM. +Cell padding follows the same ownership rule. `TableCell` and `TableHead` +provide the canonical `px-2 sm:px-3` and `py-*` defaults, but Tailwind emits +every padding utility at equal specificity in spacing-scale order, so a base +utility silently beats any smaller caller utility. When a caller's class names +a side's padding (`p-*`, `px-*`, `pl-*`, `pr-*`, `py-*`, `pt-*`, `pb-*`, with +or without `!`), the primitive omits its default for that side and keeps it for +the sides the caller left alone; prefixed-only variants such as `lg:px-0` keep +layering on the default. Tables therefore state the padding they want in their +own classes or presentation constants instead of carrying `!px-*` overrides to +beat the primitive. `Table.test.tsx` pins the base, single-side, important, +`p-*`, prefixed-only and reactive cases. That same shared table boundary now owns CSP-safe sizing for infrastructure tables and metric bars. `frontend-modern/src/components/Infrastructure/useUnifiedResourceTableState.ts` and `frontend-modern/src/components/Infrastructure/unifiedResourceTableStateModel.ts` diff --git a/frontend-modern/browser-verification.json b/frontend-modern/browser-verification.json index 4833520b01..db181fda93 100644 --- a/frontend-modern/browser-verification.json +++ b/frontend-modern/browser-verification.json @@ -1,46 +1,100 @@ { "version": 1, - "base_sha": "78796b411c1f37f57b303cae9fab5ddaa13e1121", - "verified_at": "2026-10-02T22:13:00Z", + "base_sha": "0e90236d3bb901dc84d4d07e70d4d2a09d5e6705", + "verified_at": "2026-10-02T23:55:48Z", "result": "passed", "changed_paths": [ - "frontend-modern/src/components/Settings/ReportingPanel.tsx", - "frontend-modern/src/components/Settings/reportingSchedulesModel.ts", - "frontend-modern/src/components/Settings/useReportingPanelState.ts" + "frontend-modern/src/components/shared/Table.tsx" ], "content_sha256": { - "frontend-modern/src/components/Settings/ReportingPanel.tsx": "fbf12a7025b520e1f4ba99144f88a35778e9360f982b72bba251797361504afb", - "frontend-modern/src/components/Settings/reportingSchedulesModel.ts": "69fe58abb763e528ac3e3dfe5c50b969038e905fcf46974209cac29d1bc77e83", - "frontend-modern/src/components/Settings/useReportingPanelState.ts": "cc102740cf6eec47b8f3e5b4c9d6c24e1017d7ffd14bd653d25bfd45fc88b1af" + "frontend-modern/src/components/shared/Table.tsx": "8e5029d5ee9dddc86a4733fa1e7249793cdbedb135f3634e75112f8fb3b327c9" }, "routes": [ - "/settings/support/reporting" + "/proxmox/overview", + "/proxmox/storage", + "/proxmox/replication", + "/proxmox/ceph", + "/proxmox/mail", + "/docker/overview", + "/docker/images", + "/docker/storage", + "/docker/networks", + "/docker/swarm", + "/kubernetes/overview", + "/kubernetes/nodes", + "/kubernetes/workloads", + "/kubernetes/services", + "/kubernetes/storage", + "/kubernetes/configuration", + "/kubernetes/events", + "/truenas/overview", + "/truenas/storage", + "/truenas/services", + "/truenas/apps", + "/truenas/vms", + "/truenas/shares", + "/truenas/protection", + "/vmware/overview", + "/vmware/storage", + "/vmware/networks", + "/vmware/health", + "/vmware/activity", + "/standalone/machines", + "/standalone/availability", + "/alerts/overview", + "/alerts/history", + "/alerts/thresholds", + "/actions", + "/settings/infrastructure", + "/settings/monitoring", + "/settings/pulse-intelligence", + "/settings/organization", + "/settings/security-users", + "/settings/security-webhooks" ], "viewports": [ + { + "width": 1440, + "height": 900 + }, { "width": 1280, "height": 900 }, { - "width": 375, - "height": 812 + "width": 900, + "height": 900 + }, + { + "width": 700, + "height": 900 + }, + { + "width": 560, + "height": 900 + }, + { + "width": 390, + "height": 900 } ], "states": [ - "Pro licence active with advanced_reporting; Scheduled reports header mentions the Patrol weekly summary because sessionCapabilities.assistantEnabled is true", - "Create schedule form opened: Report type select present with Performance report selected; cadence, format, resources, tag filter, delivery, retention, attach and save-copy controls visible", - "Report type switched to Patrol weekly summary: form reduced to name, timezone, weekday, time, email recipients and Enabled; help text explains the email and that nothing is saved to disk; name placeholder reads Patrol weekly summary", - "Saved schedule row: Weekly Patrol, Monday at 09:00, scope Patrol activity, last 7 days, delivery Email config recipients; API record carries kind patrol_digest and format email", - "Run now without an email destination: Last run shows Failed with the server message naming Settings > Notifications", - "Edit reopens the saved schedule with Report type Patrol weekly summary selected", - "375px: form fields stack full width, table shows name with cadence line, no horizontal clipping" + "Mock stack on origin/main plus this change; Playwright sweep of every table on the routes above at all six viewports, before and after, recording computed th/td padding per cell and leaf elements with scrollWidth > clientWidth under hidden overflow (455 table states compared)", + "Default TableCell/TableHead with no caller padding keep 8px/12px horizontal and 2px/6px vertical padding (Settings infrastructure source and agent tables unchanged at 12px/12px/6px/6px)", + "Platform tables (Proxmox nodes, Docker hosts and containers, Kubernetes clusters/nodes/deployments/pods, TrueNAS systems, vSphere, Machines): cells 12px -> 8px horizontal at 700px and up and 8px -> 6px at 560/390px; header cells keep the stylesheet 6px horizontal and drop to 2px vertical, header rows 29px -> 21px; row height 32px unchanged; clipped-cell count improved in 89 table states, unchanged in 362; the 4 states counted worse are live mock-value drift (relative-time and usage labels) with identical or wider content boxes", + "Physical Disks table (Proxmox > Storage > Physical Disks): cells 8px -> 4px horizontal at 390px and 12px -> 6px at 700px, matching the composed px-1 sm:px-1.5 lg:px-2; clipped cells at 390px 93 -> 18; detail row 12px -> 16px horizontal at 700px and 1440px", + "Storage pools table unchanged at 6px/8px cell padding across 390, 560 and 700px", + "Inline detail rows (Proxmox node, Kubernetes controller, TrueNAS system expansions): detail cell padding 12px/12px/2px/2px -> 0, detail panel inset now only its own px-2 sm:px-4; incident timeline panel in Alert History gets its p-3", + "Alerts overview stats table: dot column right padding 12px -> 0 and value column 12px -> 6px at 700px and up as its classes name; Alert History header cells 12px -> 8px horizontal; AI cost table (Settings > Pulse Intelligence) Est. USD column 12px -> 4px as its px-1 names", + "Workloads table header cells now 6px/8px to match their raw td data cells; Proxmox replication compact-mode !px-1 override still applies at 390px", + "Reactive case: switching a cell class between px-1 and a non-padding class re-resolves the base padding (unit test)" ], "interactions": [ - "Click Create schedule", - "Change Report type select to Patrol weekly summary", - "Type schedule name Weekly Patrol and click Save schedule", - "Click Run now on the saved schedule and reload", - "Click Edit on the saved schedule", - "Resize viewport to 375x812 and scroll through the form and table" + "Load each route at 1440, 1280, 900, 700, 560 and 390px and run the padding and clip sweep over all tables", + "Click Physical Disks on Proxmox > Storage at 390, 560, 700 and 1440px and expand the first disk row", + "Expand the first node row on Proxmox > Overview, the first deployment row on Kubernetes > Workloads, the first host row on Docker > Overview and the first system row on TrueNAS > Overview at 390, 560, 700 and 1440px", + "Expand the first alert row incident timeline on Alerts > History at 1440px", + "Compare before and after JSON sweeps per route, tab, viewport and table; re-measure the four states counted worse three times each", + "Revert and restore the change on the running dev server to A/B the Kubernetes clusters memory bar label at 560px" ] } diff --git a/frontend-modern/src/components/shared/Table.tsx b/frontend-modern/src/components/shared/Table.tsx index 472263d205..a39c4ebabc 100644 --- a/frontend-modern/src/components/shared/Table.tsx +++ b/frontend-modern/src/components/shared/Table.tsx @@ -81,6 +81,61 @@ export function TableRow(props: TableRowProps) { ); } +// Tailwind emits every padding utility at the same specificity, ordered by the +// spacing scale, so a caller's `px-1` loses to the base `px-2` no matter where +// it sits in the class string, and a caller's `px-3` only wins by luck. Like +// TableHeader's border and TableBody's divider, a side whose padding the caller +// names belongs to the caller: the base padding for that side is left out. +// Prefixed variants (`lg:px-0`) layer on top of the base the way they always +// did, so they do not count as owning the side. +type TableAxisPadding = { both: string; start: string; end: string }; + +const TABLE_HORIZONTAL_PADDING: TableAxisPadding = { + both: 'px-2 sm:px-3', + start: 'pl-2 sm:pl-3', + end: 'pr-2 sm:pr-3', +}; + +const TABLE_HEAD_VERTICAL_PADDING: TableAxisPadding = { + both: 'py-1.5', + start: 'pt-1.5', + end: 'pb-1.5', +}; + +const TABLE_CELL_VERTICAL_PADDING: TableAxisPadding = { + both: 'py-0.5', + start: 'pt-0.5', + end: 'pb-0.5', +}; + +const OWNS_PADDING_LEFT = /(?:^|\s)!?(?:p|px|pl|ps)-/; +const OWNS_PADDING_RIGHT = /(?:^|\s)!?(?:p|px|pr|pe)-/; +const OWNS_PADDING_TOP = /(?:^|\s)!?(?:p|py|pt)-/; +const OWNS_PADDING_BOTTOM = /(?:^|\s)!?(?:p|py|pb)-/; + +const resolveAxisPadding = ( + axis: TableAxisPadding, + callerOwnsStart: boolean, + callerOwnsEnd: boolean, +): string => { + if (callerOwnsStart) return callerOwnsEnd ? '' : axis.end; + return callerOwnsEnd ? axis.start : axis.both; +}; + +const resolveTablePaddingClass = (vertical: TableAxisPadding, customClass?: string): string => { + const custom = customClass ?? ''; + return [ + resolveAxisPadding( + TABLE_HORIZONTAL_PADDING, + OWNS_PADDING_LEFT.test(custom), + OWNS_PADDING_RIGHT.test(custom), + ), + resolveAxisPadding(vertical, OWNS_PADDING_TOP.test(custom), OWNS_PADDING_BOTTOM.test(custom)), + ] + .filter(Boolean) + .join(' '); +}; + export type TableHeadProps = JSX.HTMLAttributes & { colSpan?: number; colspan?: number; @@ -91,7 +146,7 @@ export function TableHead(props: TableHeadProps) { const [local, rest] = splitProps(props, ['class', 'children']); return ( {local.children} @@ -109,7 +164,10 @@ export type TableCellProps = JSX.HTMLAttributes & { export function TableCell(props: TableCellProps) { const [local, rest] = splitProps(props, ['class', 'children']); return ( - + {local.children} ); diff --git a/frontend-modern/src/components/shared/__tests__/PulseDataGrid.test.tsx b/frontend-modern/src/components/shared/__tests__/PulseDataGrid.test.tsx index 23ac2ff5e9..b42076dff2 100644 --- a/frontend-modern/src/components/shared/__tests__/PulseDataGrid.test.tsx +++ b/frontend-modern/src/components/shared/__tests__/PulseDataGrid.test.tsx @@ -185,4 +185,35 @@ describe('PulseDataGrid', () => { expect(refreshedCell).toBe(initialCell); }); + + it('lets the grid own header, cell and expansion padding instead of the shared base', () => { + render(() => ( + + data={[{ id: '1', name: 'Tower' }]} + columns={[{ key: 'name', label: 'Name' }]} + keyExtractor={(row) => row.id} + isRowExpanded={() => true} + expandedRender={() =>
Tower details
} + /> + )); + + const head = screen.getByText('Name').closest('th')!; + expect(head.className).toContain('px-3 sm:px-4 py-2.5'); + expect(head.className).not.toMatch(/(?:^|\s)(?:px|pl|pr)-2\b/); + expect(head.className).not.toContain('sm:px-3'); + expect(head.className).not.toContain('py-1.5'); + + const cell = screen.getByText('Tower').closest('td')!; + expect(cell.className).toContain('px-3 sm:px-4 py-2 sm:py-3.5'); + expect(cell.className).not.toMatch(/(?:^|\s)(?:px|pl|pr)-2\b/); + expect(cell.className).not.toContain('sm:px-3'); + expect(cell.className).not.toContain('py-0.5'); + + const expansion = screen.getByText('Tower details').closest('td')!; + expect(expansion).toHaveAttribute('colspan', '1'); + expect(expansion.className).toContain('px-0 py-0'); + expect(expansion.className).not.toMatch(/(?:^|\s)(?:px|pl|pr)-2\b/); + expect(expansion.className).not.toContain('sm:px-3'); + expect(expansion.className).not.toContain('py-0.5'); + }); }); diff --git a/frontend-modern/src/components/shared/__tests__/Table.test.tsx b/frontend-modern/src/components/shared/__tests__/Table.test.tsx index a0e7fc3260..42b54f4e87 100644 --- a/frontend-modern/src/components/shared/__tests__/Table.test.tsx +++ b/frontend-modern/src/components/shared/__tests__/Table.test.tsx @@ -230,3 +230,137 @@ describe('TableHeader', () => { expect(thead!.className).not.toContain('border-border '); }); }); + +describe('TableCell and TableHead padding ownership', () => { + const cellClass = (text: string) => screen.getByText(text).closest('td')!.className; + const headClass = (text: string) => screen.getByText(text).closest('th')!.className; + + it('keeps the base padding when the caller names none', () => { + render(() => ( + + + + default head + + + + + default cell + + +
+ )); + + expect(headClass('default head')).toContain('px-2 sm:px-3 py-1.5'); + expect(cellClass('default cell')).toContain('px-2 sm:px-3 py-0.5'); + }); + + it('drops the base horizontal padding when the caller passes px-*, including important and larger values', () => { + render(() => ( + + + + tight head + + + + + tight cell + important cell + wide cell + + +
+ )); + + const tightHead = headClass('tight head'); + expect(tightHead).not.toMatch(/(?:^|\s)(?:px|pl|pr)-2\b/); + expect(tightHead).not.toContain('sm:px-3'); + expect(tightHead).not.toContain('py-1.5'); + expect(tightHead).toContain('px-1 sm:px-1.5 lg:px-2 py-0.5'); + + const tightCell = cellClass('tight cell'); + expect(tightCell).not.toMatch(/(?:^|\s)(?:px|pl|pr)-2\b/); + expect(tightCell).not.toContain('sm:px-3'); + expect(tightCell).not.toContain('py-0.5'); + expect(tightCell).toContain('px-1 sm:px-1.5 lg:px-2 py-1'); + + const importantCell = cellClass('important cell'); + expect(importantCell).not.toContain('px-2'); + expect(importantCell).toContain('py-0.5'); + + const wideCell = cellClass('wide cell'); + expect(wideCell).not.toContain('px-2'); + expect(wideCell).not.toContain('py-0.5'); + }); + + it('keeps the base padding on the sides the caller leaves alone', () => { + render(() => ( + + + + right only + left only + vertical only + + +
+ )); + + const rightOnly = cellClass('right only'); + expect(rightOnly).toContain('pl-2 sm:pl-3'); + expect(rightOnly).not.toContain('px-2'); + expect(rightOnly).not.toContain('pr-2 sm:pr-3'); + expect(rightOnly).not.toContain('py-0.5'); + + const leftOnly = cellClass('left only'); + expect(leftOnly).toContain('pr-2 sm:pr-3'); + expect(leftOnly).not.toContain('px-2'); + expect(leftOnly).toContain('py-0.5'); + + const verticalOnly = cellClass('vertical only'); + expect(verticalOnly).toContain('px-2 sm:px-3'); + expect(verticalOnly).not.toContain('py-0.5'); + }); + + it('treats p-* as owning every side and ignores prefixed-only padding', () => { + render(() => ( + + + + spacer + prefixed + unrelated + + +
+ )); + + const spacer = cellClass('spacer'); + expect(spacer).not.toMatch(/(?:^|\s)(?:px|pl|pr|py|pt|pb)-/); + expect(spacer).toContain('p-0'); + + expect(cellClass('prefixed')).toContain('px-2 sm:px-3 py-0.5'); + expect(cellClass('unrelated')).toContain('px-2 sm:px-3 py-0.5'); + }); + + it('re-resolves the base padding when the caller class changes', () => { + const [compact, setCompact] = createSignal(false); + render(() => ( + + + + reactive + + +
+ )); + + expect(cellClass('reactive')).toContain('px-2 sm:px-3'); + setCompact(true); + expect(cellClass('reactive')).not.toContain('px-2'); + expect(cellClass('reactive')).toContain('px-1'); + setCompact(false); + expect(cellClass('reactive')).toContain('px-2 sm:px-3'); + }); +});