diff --git a/docs/release-control/v6/internal/subsystems/frontend-primitives.md b/docs/release-control/v6/internal/subsystems/frontend-primitives.md index bb84bc1597..1fbee03224 100644 --- a/docs/release-control/v6/internal/subsystems/frontend-primitives.md +++ b/docs/release-control/v6/internal/subsystems/frontend-primitives.md @@ -910,6 +910,23 @@ scrolling. Explicit caller-owned native listeners remain caller-owned. isolation and bound handlers; `PulseDataGrid.test.tsx` checks inheritance. The production PBS table/drawer browser fixture exercises first-tap disclosure and History in WebKit phone emulation, not installed collection or devices. +A drag that selects text inside a clickable row is not a row activation. +`TableRow` owns one `createRowTextSelectionGuard()` per row: it records where +the row was pressed, and a click whose pointer moved more than a few pixels +since that press while text inside the row is selected skips the row action. +Copying a pod or container name for kubectl or docker therefore leaves the +drawer, or a Proxmox node's guest selection, as it was. A press released in +place still runs the action on the first click, even over text that is already +selected (Chromium clears that selection only after the click), and a +double-click stays two plain clicks. Custom row shells that run their own row +action (`GuestRow`, `StoragePoolRow`) create the same guard and wire its +mousedown handler and click check instead of writing a per-table selection +test. Nested controls stop propagation and keyboard activation belongs to the +disclosure button, so neither passes through the guard. `Table.test.tsx` and +`PulseDataGrid.test.tsx` cover drags, in-place clicks over a selection, +selections outside the row and bound handlers; +`PlatformResourceDetailTableRow.test.tsx` covers the drawer toggle, chevron and +Enter while text stays selected. The custom Workloads `GuestRow` shell reuses `Table`'s exported `nativeRowClickTarget` marker, conditional on its own action, without inheriting shared row styling or changing alert/highlight treatment. Its name and row body 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 8610516f3c..50344121e9 100644 --- a/docs/release-control/v6/internal/subsystems/performance-and-scalability.md +++ b/docs/release-control/v6/internal/subsystems/performance-and-scalability.md @@ -391,6 +391,15 @@ touch-end. `GuestRow.test.tsx` verifies its conditional lifecycle and non-activation; the production row/full-drawer touch fixture verifies first activation and unchanged identity independently of native collector acceptance. +The guest row also creates one `createRowTextSelectionGuard()` from `Table` per +row, so a drag that selects row text (a VMID, address or tag) does not open the +guest drawer, while a press released in place still opens it on the first +click. The guard keeps one press coordinate in a closure, binds its native +mousedown listener only while a row action exists, and reads the selection only +when the row is clicked: no document listener, timer, signal, metric derivation +or History work is added per row. `GuestRow.test.tsx` covers the drag, the +in-place click and chevron activation over a selection. + ## Canonical Files 1. `pkg/metrics/store.go` @@ -3757,6 +3766,18 @@ returns focus to the originating disclosure. Nested evidence activation must not trigger the enclosing guest row. Touch targets are at least 44px; compact desktop shields and the existing status colours remain. +The Backup column badge does not truncate. Running (73px with its shield) and +Unknown (79px) need about 95px with cell padding, so the compact layout weights +give Backup 10 against Type 5.4 and CPU 9.6, and the wide column is 96px with +the extra 24px taken from CPU (116px). Type still fits its VM/LXC badge and +sorted header, and CPU its percentage and core count, while Name, ID, Uptime, +Net I/O, Disk I/O and the memory and disk bars keep their widths and used/total +sublabels; views without a Backup column (pods, Docker) give them slightly more. +Before this, Running was cut inside its pill at every width up to 1600px, and +None and month ages up to about 1500px. +`WorkloadsSurface.performance.contract.test.tsx` checks the narrowest compact +table and the wide width. + The evidence content is reactive to the same guest identity: running is not completion, malformed/future times are unknown, and only the existing absence sentinels mean no backup recorded. A removed row unmounts its disclosure and diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index 3641fff67d..035d5ed4ab 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -2838,6 +2838,12 @@ canonical pre-handler authorization proof covering both transfer routes. row, so they must explicitly suppress the redundant visible mobile chevron. Storage group row activation performs disclosure and summary focus together rather than preserving a storage-only chevron exception. + A drag that selects pool row text, such as a pool name being copied, must + not expand the row: `StoragePoolRow` wires the shared + `createRowTextSelectionGuard()` from + `frontend-modern/src/components/shared/Table.tsx`, and a press released in + place still expands on the first click. This is presentation only; it + changes no storage evidence, alert state or recovery authority. Touch users still must not inherit synthetic hover branches, and storage must not keep a special trailing expand column once the shared leading action contract exists. diff --git a/frontend-modern/browser-verification/0c24a36bf1559a9d.json b/frontend-modern/browser-verification/0c24a36bf1559a9d.json new file mode 100644 index 0000000000..74bf4631f9 --- /dev/null +++ b/frontend-modern/browser-verification/0c24a36bf1559a9d.json @@ -0,0 +1,90 @@ +{ + "version": 1, + "base_sha": "ebd6d09337d5e4cd17b327df5a69a58dae739a27", + "verified_at": "2026-10-05T15:39:31Z", + "result": "passed", + "changed_paths": [ + "frontend-modern/src/components/Workloads/guestRowModel.tsx" + ], + "content_sha256": { + "frontend-modern/src/components/Workloads/guestRowModel.tsx": "fa970cac1cde21056bc707a5ec6dc056d9f4dc9362f83a5bf8b59d5c2af1f75f" + }, + "routes": [ + "/proxmox/overview", + "/vmware/overview" + ], + "viewports": [ + { + "width": 390, + "height": 900 + }, + { + "width": 600, + "height": 900 + }, + { + "width": 768, + "height": 900 + }, + { + "width": 900, + "height": 900 + }, + { + "width": 973, + "height": 900 + }, + { + "width": 1000, + "height": 900 + }, + { + "width": 1024, + "height": 900 + }, + { + "width": 1100, + "height": 900 + }, + { + "width": 1180, + "height": 900 + }, + { + "width": 1280, + "height": 900 + }, + { + "width": 1366, + "height": 900 + }, + { + "width": 1440, + "height": 900 + }, + { + "width": 1513, + "height": 900 + }, + { + "width": 1600, + "height": 900 + }, + { + "width": 1920, + "height": 900 + } + ], + "states": [ + "Private mock stack (PULSE_MOCK_MODE, random metrics off) serving this tree on main ebd6d09337; before = main, after = main plus this change, measured in the same session", + "Backup badge fit (each label cloned at natural width into the narrowest Backup cell): Running 73px, Unknown 79px, None 58px, 12mo 61px, 21d 52px with the shield. On main the column content box was 33-65px from 1000 to 1600 (compact layout weight 5, wide 72px), so Running and Unknown clipped at every width up to 1600 and None and month ages up to 1440-1513; patched the content box is 81-132px and every label fits at every width where the column shows (1000 to 1920)", + "Clip sweep over every cell of the Workloads table and its overflow-hidden or ellipsis descendants, .sr-only excluded, 15 widths 390 to 1920: Proxmox 464 to 452 and vSphere 2 to 1 clipped elements, no column clips more at any width, and the memory and disk used/total sublabels and the CPU core counts are unchanged in every row", + "Column widths patched at 1024: Name 259, Type 54, ID 70, CPU 96, Mem 140, Disk 140, Uptime 80, Backup 100 (main 259/70/70/130/140/140/80/50); at 1600 (wide) Name 224, CPU 130, Backup 108, Net I/O and Disk I/O 191 (main 224/157/81/191)", + "Before and after element screenshots of the guest table around the Running row at 1024, 1280 and 1600: Running cut inside its pill on main, whole patched" + ], + "interactions": [ + "Loaded each route fresh, waited for the row count to settle, resized through every viewport before measuring", + "Clicked the Running backup badge at 1024, 1280 and 1600: the Backup status evidence dialog opened (Backup running now\u2026) and Escape closed it", + "Clicked a guest row's name cell to open its details on both routes; no console errors" + ] +} diff --git a/frontend-modern/browser-verification/4f08442c2617c48d.json b/frontend-modern/browser-verification/4f08442c2617c48d.json new file mode 100644 index 0000000000..2bffa3110d --- /dev/null +++ b/frontend-modern/browser-verification/4f08442c2617c48d.json @@ -0,0 +1,52 @@ +{ + "version": 1, + "base_sha": "ebd6d09337d5e4cd17b327df5a69a58dae739a27", + "verified_at": "2026-10-05T15:31:41Z", + "result": "passed", + "changed_paths": [ + "frontend-modern/src/components/Storage/StoragePoolRow.tsx", + "frontend-modern/src/components/Workloads/GuestRow.tsx", + "frontend-modern/src/components/shared/Table.tsx" + ], + "content_sha256": { + "frontend-modern/src/components/Storage/StoragePoolRow.tsx": "213ce500ff48a9470fce226cf720393d9eac8eac8465639eca1b2a691625fefa", + "frontend-modern/src/components/Workloads/GuestRow.tsx": "b94b44121f2d8827452b1002742272bd19bf0391a64f7b0f4fe0c76e46b52fdd", + "frontend-modern/src/components/shared/Table.tsx": "c75023940e8c93180c32083cad363cd8781def55c20bed0247a5eb2e37ede08d" + }, + "routes": [ + "/kubernetes/pods", + "/docker/containers", + "/proxmox/overview", + "/proxmox/storage" + ], + "viewports": [ + { + "width": 1440, + "height": 900 + }, + { + "width": 768, + "height": 1024 + }, + { + "width": 390, + "height": 844 + } + ], + "states": [ + "Private mock stack from this tree (backend :7695 in mock mode, Vite :5215 serving this checkout), driven by headless Chromium through Playwright with real mouse down/move/up input, not synthetic events.", + "Kubernetes pods (TableRow onClick={() => drawer.toggle(resource)}): after dragging across the pod name cron-nightly-backfill the name stayed selected and the row's detail chevron kept aria-expanded=false with no drawer row inserted. The same drag on the unfixed tree (runtime files reverted to the parent) opened the drawer, as reported.", + "Docker containers (drawer.toggle), Proxmox guest rows (custom GuestRow shell) and Proxmox storage pools (custom StoragePoolRow shell): the drag left the drawer closed, and with the drawer open left it open; on the unfixed tree each drag toggled it.", + "Proxmox host rows (getPlatformResourceDetailRowInteractionProps): the row click selects that node's guests (data-summary-row-active) rather than opening a drawer; the drag left that selection unset, while on the unfixed tree the drag applied it.", + "A plain click on the still-selected name opened the drawer (or applied the node's guest selection) on the first click on every table; Chromium cleared the selection afterwards.", + "Phone 390x844 with touch emulation: tapping a pod row and a guest row opened the drawer and a second tap closed it (WebKit-compatible native click marker unchanged). The guest row moves down 52px when its drawer opens, so the second tap must use the row's new position." + ], + "interactions": [ + "Dragged across the first selectable text in the first row of each table (pod name, container name, host version, guest type, pool name) at 1440x900 and 768x1024, with the drawer closed and again with it open.", + "Clicked in place on the selected text after the drag.", + "Clicked the row's detail chevron while text in the row stayed selected; it toggled the drawer each time.", + "Focused the chevron and pressed Enter twice; it opened and closed the drawer.", + "Tapped pod and guest rows twice with touch emulation at 390x844.", + "Repeated the drag and click sequence with the three runtime files reverted to the parent commit to confirm the bug reproduced there." + ] +} diff --git a/frontend-modern/src/components/Storage/StoragePoolRow.tsx b/frontend-modern/src/components/Storage/StoragePoolRow.tsx index c65a4ae88e..ab435d43f3 100644 --- a/frontend-modern/src/components/Storage/StoragePoolRow.tsx +++ b/frontend-modern/src/components/Storage/StoragePoolRow.tsx @@ -40,6 +40,7 @@ import { createSummaryInteractiveRowPreviewHandlers, } from '@/components/shared/summaryInteractionA11y'; import { SummaryRowActionButton } from '@/components/shared/SummaryRowActionButton'; +import { createRowTextSelectionGuard } from '@/components/shared/Table'; interface StoragePoolRowProps { layoutMode: StoragePoolTableLayoutMode; @@ -75,6 +76,7 @@ export const StoragePoolRow: Component = (props) => { const detailControlsId = createMemo(() => buildSummaryDisclosureControlsId(props.summarySeriesId), ); + const selectionGuard = createRowTextSelectionGuard(); const interactiveRowHandlers = createSummaryInteractiveRowPreviewHandlers({ onPreview: () => props.onHoverChange?.(props.summarySeriesId), onPreviewClear: () => props.onHoverChange?.(null), @@ -84,7 +86,10 @@ export const StoragePoolRow: Component = (props) => { <> { + if (!selectionGuard.isSelectionClick(event)) props.onToggleExpand(); + }} {...interactiveRowHandlers} data-summary-series-id={props.summarySeriesId} data-summary-group-member-active={ diff --git a/frontend-modern/src/components/Storage/__tests__/Storage.test.tsx b/frontend-modern/src/components/Storage/__tests__/Storage.test.tsx index 7c917328be..5b4b65d38e 100644 --- a/frontend-modern/src/components/Storage/__tests__/Storage.test.tsx +++ b/frontend-modern/src/components/Storage/__tests__/Storage.test.tsx @@ -912,6 +912,36 @@ describe('Storage', () => { }); }); + it('keeps a storage pool row closed when a drag selects its text', async () => { + hookResources = [buildStorageResource('storage-1', 'Node-Store', 'pve1')]; + + render(() => ); + + const name = await screen.findByText('Node-Store'); + const range = document.createRange(); + range.selectNodeContents(name); + const selection = document.getSelection()!; + selection.removeAllRanges(); + selection.addRange(range); + try { + fireEvent.mouseDown(name, { clientX: 4, clientY: 8 }); + fireEvent.click(name, { clientX: 80, clientY: 8 }); + expect(screen.getByRole('button', { name: 'Expand Node-Store' })).toHaveAttribute( + 'aria-expanded', + 'false', + ); + + fireEvent.mouseDown(name, { clientX: 40, clientY: 8 }); + fireEvent.click(name, { clientX: 40, clientY: 8 }); + expect(await screen.findByRole('button', { name: 'Collapse Node-Store' })).toHaveAttribute( + 'aria-expanded', + 'true', + ); + } finally { + selection.removeAllRanges(); + } + }); + it('uses canonical storage metrics target ids for expanded pool history charts', async () => { const metricsHistorySpy = vi.spyOn(ChartsAPI, 'getMetricsHistory').mockResolvedValue({ resourceType: 'storage', diff --git a/frontend-modern/src/components/Workloads/GuestRow.tsx b/frontend-modern/src/components/Workloads/GuestRow.tsx index 45b126ebd6..adb2046f0a 100644 --- a/frontend-modern/src/components/Workloads/GuestRow.tsx +++ b/frontend-modern/src/components/Workloads/GuestRow.tsx @@ -30,7 +30,7 @@ import { createSummaryInteractiveRowPreviewHandlers, } from '@/components/shared/summaryInteractionA11y'; import { SummaryRowActionButton } from '@/components/shared/SummaryRowActionButton'; -import { nativeRowClickTarget } from '@/components/shared/Table'; +import { createRowTextSelectionGuard, nativeRowClickTarget } from '@/components/shared/Table'; import { DiscoveryReadinessBadge } from '@/components/shared/DiscoveryReadinessBadge'; import { getWorkloadGuestDiskStatusMessage } from '@/utils/workloadGuestPresentation'; import { ResourceNameWithWebInterfaceLink } from '@/components/shared/WebInterfaceLink'; @@ -58,6 +58,15 @@ const DISK_READ_ACTIONABLE_REASONS = new Set(['permission-denied', 'agent-error' export function GuestRow(props: GuestRowProps) { const [rowActionProps] = splitProps(props, ['onClick']); + const selectionGuard = createRowTextSelectionGuard(); + const runRowAction = (event: MouseEvent) => { + if (!selectionGuard.isSelectionClick(event)) rowActionProps.onClick?.(); + }; + const rowAction = { + get onClick() { + return rowActionProps.onClick ? runRowAction : undefined; + }, + }; const { agentVersion, appContainerRuntimeBadge, @@ -277,7 +286,8 @@ export function GuestRow(props: GuestRowProps) { } data-summary-row-active={props.isSummaryHighlighted && !props.isExpanded ? 'true' : 'false'} on:click={rowActionProps.onClick ? nativeRowClickTarget : undefined} - {...rowActionProps} + on:mousedown={rowActionProps.onClick ? selectionGuard.onMouseDown : undefined} + {...rowAction} {...interactiveRowHandlers} > {/* Name - always visible */} diff --git a/frontend-modern/src/components/Workloads/__tests__/GuestRow.test.tsx b/frontend-modern/src/components/Workloads/__tests__/GuestRow.test.tsx index 079934ef2c..39b59fb197 100644 --- a/frontend-modern/src/components/Workloads/__tests__/GuestRow.test.tsx +++ b/frontend-modern/src/components/Workloads/__tests__/GuestRow.test.tsx @@ -1087,6 +1087,33 @@ describe('GuestRow', () => { expect(onClick).toHaveBeenCalledOnce(); }); + it('does not open the guest for a drag that selects row text', () => { + const onClick = vi.fn(); + const { container } = renderGuestRow({ + guest: makeGuest(), + onClick, + }); + const cell = container.querySelector('td[data-workload-col="vmid"]')!; + const range = document.createRange(); + range.selectNodeContents(cell); + const selection = document.getSelection()!; + selection.removeAllRanges(); + selection.addRange(range); + try { + fireEvent.mouseDown(cell, { clientX: 4, clientY: 8 }); + fireEvent.click(cell, { clientX: 30, clientY: 8 }); + expect(onClick).not.toHaveBeenCalled(); + fireEvent.click(screen.getByRole('button', { name: 'Expand test-vm' })); + expect(onClick).toHaveBeenCalledTimes(1); + + fireEvent.mouseDown(cell, { clientX: 12, clientY: 8 }); + fireEvent.click(cell, { clientX: 12, clientY: 8 }); + expect(onClick).toHaveBeenCalledTimes(2); + } finally { + selection.removeAllRanges(); + } + }); + it('marks only actionable guest rows as native touch click targets', () => { const addListener = vi.spyOn(HTMLTableRowElement.prototype, 'addEventListener'); const removeListener = vi.spyOn(HTMLTableRowElement.prototype, 'removeEventListener'); @@ -1788,7 +1815,7 @@ describe('GUEST_COLUMNS', () => { 'aiContext', ]); expect(getGuestColumnWidthStyle('name', false, 'compact', compactPodColumnIds)).toEqual({ - width: '25%', + width: '25.8449%', }); }); @@ -1807,7 +1834,7 @@ describe('GUEST_COLUMNS', () => { expect( getGuestColumnWidthStyle('update', false, 'compact', compactDockerRuntimeColumnIds), ).toEqual({ - width: '8.9286%', + width: '9.2081%', }); }); diff --git a/frontend-modern/src/components/Workloads/__tests__/WorkloadsSurface.performance.contract.test.tsx b/frontend-modern/src/components/Workloads/__tests__/WorkloadsSurface.performance.contract.test.tsx index 4217231ea1..d2aa75fff9 100644 --- a/frontend-modern/src/components/Workloads/__tests__/WorkloadsSurface.performance.contract.test.tsx +++ b/frontend-modern/src/components/Workloads/__tests__/WorkloadsSurface.performance.contract.test.tsx @@ -5,6 +5,7 @@ import { Route, Router } from '@solidjs/router'; import type { Resource } from '@/types/resource'; import { syncSessionSettingsCapabilities } from '@/stores/sessionSettingsCapabilities'; import { WorkloadsSurface } from '../WorkloadsSurface'; +import { WORKLOAD_TABLE_CONTAINER_TABLET_WIDTH, getGuestColumnStyle } from '../guestRowModel'; import workloadsSource from '../WorkloadsSurface.tsx?raw'; import workloadsFilterSource from '../WorkloadsFilter.tsx?raw'; import workloadsWorkloadTableSource from '../WorkloadsTable.tsx?raw'; @@ -1870,3 +1871,26 @@ describe('Workloads performance contract', () => { }); }); }); + +describe('Workloads Backup column fit', () => { + // The backup badge does not truncate: Running is 73px and Unknown 79px with + // the shield, plus 8px of cell padding a side. The compact layout starts at a + // 900px table, where the Proxmox guest columns all show. + const badgeWidth = 79 + 16; + + it('keeps the widest backup badge whole at the narrowest compact table', () => { + const proxmoxColumns = ['name', 'type', 'vmid', 'cpu', 'memory', 'disk', 'uptime', 'backup']; + const percent = Number.parseFloat( + String(getGuestColumnStyle('backup', false, 'compact', proxmoxColumns)?.width), + ); + expect((percent / 100) * WORKLOAD_TABLE_CONTAINER_TABLET_WIDTH).toBeGreaterThanOrEqual( + badgeWidth, + ); + }); + + it('keeps the widest backup badge whole in the wide layout', () => { + expect( + Number.parseFloat(String(getGuestColumnStyle('backup', false, 'wide')?.width)), + ).toBeGreaterThanOrEqual(badgeWidth); + }); +}); diff --git a/frontend-modern/src/components/Workloads/guestRowModel.tsx b/frontend-modern/src/components/Workloads/guestRowModel.tsx index 0c7c67ea17..df232cfa02 100644 --- a/frontend-modern/src/components/Workloads/guestRowModel.tsx +++ b/frontend-modern/src/components/Workloads/guestRowModel.tsx @@ -241,7 +241,7 @@ export const GUEST_COLUMNS: ColumnDef[] = [ createVisibleCanonicalTypeColumn(), { id: 'info', label: 'Info', width: '100px', sortKey: 'info', kind: 'numeric-value' }, { id: 'vmid', label: 'ID', width: '45px', sortKey: 'vmid', kind: 'numeric-value' }, - { id: 'cpu', label: 'CPU', width: '140px', sortKey: 'cpu', kind: 'metric-bar' }, + { id: 'cpu', label: 'CPU', width: '116px', sortKey: 'cpu', kind: 'metric-bar' }, { id: 'memory', label: 'Mem', width: '140px', sortKey: 'memory', kind: 'metric-bar' }, { id: 'disk', @@ -411,8 +411,12 @@ export const GUEST_COLUMNS: ColumnDef[] = [ /> ), - width: '72px', - minWidth: '68px', + // The badge does not truncate: Running is 73px and Unknown 79px with the + // shield, so the wide column keeps room for both with cell padding. Its + // extra 24px comes from CPU, whose bar shows only a percentage and core + // count, so the other wide columns keep their widths. + width: '96px', + minWidth: '90px', toggleable: true, kind: 'badge', }, @@ -519,17 +523,22 @@ const GUEST_COLUMN_RESPONSIVE_WEIGHTS: Record< memory: 17, disk: 17, }, + // The Backup badge needs about 95px with cell padding for Unknown (79px) + // and Running (73px) at the narrowest compact table. Its room comes from Type, whose VM/LXC + // badge and sorted header need about 51px, and from CPU, whose bar shows + // only a percentage and core count; Name, ID, Uptime and the memory and + // disk bars keep their widths, so their used/total sublabels stay. compact: { name: 26, runtime: 10, - type: 7, + type: 5.4, info: 7, vmid: 7, - cpu: 13, + cpu: 9.6, memory: 14, disk: 14, uptime: 8, - backup: 5, + backup: 10, image: 18, namespace: 11, context: 13, diff --git a/frontend-modern/src/components/shared/Table.tsx b/frontend-modern/src/components/shared/Table.tsx index 7b87a7d588..49cec497d2 100644 --- a/frontend-modern/src/components/shared/Table.tsx +++ b/frontend-modern/src/components/shared/Table.tsx @@ -69,12 +69,68 @@ export type TableRowProps = JSX.HTMLAttributes; // Custom row shells reuse this marker without inheriting TableRow's styling. export const nativeRowClickTarget = () => undefined; +export type RowTextSelectionGuard = { + onMouseDown: (event: MouseEvent) => void; + isSelectionClick: (event: MouseEvent) => boolean; +}; + +// Further than this between press and release, the pointer was dragging. +const ROW_CLICK_DRAG_TOLERANCE_PX = 3; + +// Dragging across a name to copy it (a pod name for kubectl, a container name +// for docker) ends with a click on the row the drag stayed inside. That click +// finishes a text selection; it is not a request to open the row, and running +// the row action would shift the layout under the selection, so row actions +// skip it. A press released where it went down is still a plain click, even +// on text that is already selected: the browser only clears that selection +// after the click, and the row must open on the first click, not the second. +// Nested controls such as the disclosure button handle their own clicks, so +// they and keyboard activation are unaffected. TableRow wires this itself; +// custom row shells create one per row and wire both handlers. +export function createRowTextSelectionGuard(): RowTextSelectionGuard { + let pressedAt: { x: number; y: number } | undefined; + return { + onMouseDown: (event) => { + pressedAt = { x: event.clientX, y: event.clientY }; + }, + isSelectionClick: (event) => { + const press = pressedAt; + pressedAt = undefined; + const row = event.currentTarget; + if (!(row instanceof Node)) return false; + const selection = row.ownerDocument?.getSelection(); + if (!selection || selection.isCollapsed) return false; + if (!row.contains(selection.anchorNode) && !row.contains(selection.focusNode)) return false; + if (!press) return true; + return ( + Math.abs(event.clientX - press.x) > ROW_CLICK_DRAG_TOLERANCE_PX || + Math.abs(event.clientY - press.y) > ROW_CLICK_DRAG_TOLERANCE_PX + ); + }, + }; +} + export function TableRow(props: TableRowProps) { - const [local, rest] = splitProps(props, ['class', 'children']); + const [local, rest] = splitProps(props, ['class', 'children', 'onClick']); + const selectionGuard = createRowTextSelectionGuard(); + const runRowAction: JSX.EventHandler = (event) => { + if (selectionGuard.isSelectionClick(event)) return; + const action = local.onClick; + if (typeof action === 'function') action(event); + else action?.[0](action[1], event); + }; + // A spread, because Solid's lint reads onClick beside on:click as a duplicate. + const rowAction = { + get onClick() { + return local.onClick ? runRowAction : undefined; + }, + }; 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 b3da84eeb9..3453aa1fe2 100644 --- a/frontend-modern/src/components/shared/__tests__/PulseDataGrid.test.tsx +++ b/frontend-modern/src/components/shared/__tests__/PulseDataGrid.test.tsx @@ -103,6 +103,37 @@ describe('PulseDataGrid', () => { expect(onRowClick).toHaveBeenCalledTimes(1); }); + it('does not run the row handler for a drag that selects row text', () => { + const onRowClick = vi.fn(); + + render(() => ( + + data={[{ id: '1', name: 'Copyable tower' }]} + columns={[{ key: 'name', label: 'Name' }]} + keyExtractor={(row) => row.id} + onRowClick={onRowClick} + /> + )); + + const name = screen.getByText('Copyable tower'); + const range = document.createRange(); + range.selectNodeContents(name); + const selection = document.getSelection()!; + selection.removeAllRanges(); + selection.addRange(range); + try { + fireEvent.mouseDown(name, { clientX: 4, clientY: 8 }); + fireEvent.click(name, { clientX: 96, clientY: 8 }); + expect(onRowClick).not.toHaveBeenCalled(); + + fireEvent.mouseDown(name, { clientX: 40, clientY: 8 }); + fireEvent.click(name, { clientX: 40, clientY: 8 }); + expect(onRowClick).toHaveBeenCalledTimes(1); + } finally { + selection.removeAllRanges(); + } + }); + it('inherits the native touch click target from the shared TableRow', () => { const nativeListener = vi.spyOn(HTMLTableRowElement.prototype, 'addEventListener'); const onRowClick = vi.fn(); diff --git a/frontend-modern/src/components/shared/__tests__/Table.test.tsx b/frontend-modern/src/components/shared/__tests__/Table.test.tsx index 989a6fd4de..cfe02f17b9 100644 --- a/frontend-modern/src/components/shared/__tests__/Table.test.tsx +++ b/frontend-modern/src/components/shared/__tests__/Table.test.tsx @@ -133,6 +133,97 @@ describe('TableRow touch activation', () => { }); }); +describe('TableRow text selection', () => { + afterEach(() => { + document.getSelection()?.removeAllRanges(); + }); + + const selectText = (element: Element) => { + const range = document.createRange(); + range.selectNodeContents(element); + const selection = document.getSelection()!; + selection.removeAllRanges(); + selection.addRange(range); + }; + + // A drag presses at one end of the text and releases at the other; jsdom + // has no drag, so select the text and send the press and the click apart. + const dragSelect = (element: Element) => { + selectText(element); + fireEvent.mouseDown(element, { clientX: 4, clientY: 8 }); + fireEvent.click(element, { clientX: 96, clientY: 8 }); + }; + + const plainClick = (element: Element) => { + fireEvent.mouseDown(element, { clientX: 40, clientY: 8 }); + fireEvent.click(element, { clientX: 40, clientY: 8 }); + }; + + it('skips the row action for a drag that selects text in the row', () => { + const onClick = vi.fn(); + const onDisclosure = vi.fn(); + render(() => ( + + + + + + web-7d9f8b6c5-x2kqp + + + +
+ )); + const name = screen.getByText('web-7d9f8b6c5-x2kqp'); + + dragSelect(name); + expect(onClick).not.toHaveBeenCalled(); + + // The explicit disclosure control still works while the text stays selected. + fireEvent.click(screen.getByRole('button', { name: 'Details' })); + expect(onDisclosure).toHaveBeenCalledTimes(1); + expect(document.getSelection()!.isCollapsed).toBe(false); + + // A press released in place is a plain click, even on the selected text. + plainClick(name); + expect(onClick).toHaveBeenCalledTimes(1); + }); + + it('runs the row action when the only selection sits outside the row', () => { + const onClick = vi.fn(); + render(() => ( + <> +

Cluster heading

+ + + + api-server + + +
+ + )); + const name = screen.getByText('api-server'); + + selectText(screen.getByText('Cluster heading')); + fireEvent.mouseDown(name, { clientX: 4, clientY: 8 }); + fireEvent.click(name, { clientX: 96, clientY: 8 }); + expect(onClick).toHaveBeenCalledTimes(1); + expect(onClick.mock.calls[0][0]).toBe('bound-identity'); + + dragSelect(name); + expect(onClick).toHaveBeenCalledTimes(1); + }); +}); + describe('TableBody', () => { it('keeps the shared table wrapper CSP-safe', () => { expect(tableSource).toContain('touch-scroll'); diff --git a/frontend-modern/src/features/platformPage/__tests__/PlatformResourceDetailTableRow.test.tsx b/frontend-modern/src/features/platformPage/__tests__/PlatformResourceDetailTableRow.test.tsx index f5d369bac0..5ab3a28d62 100644 --- a/frontend-modern/src/features/platformPage/__tests__/PlatformResourceDetailTableRow.test.tsx +++ b/frontend-modern/src/features/platformPage/__tests__/PlatformResourceDetailTableRow.test.tsx @@ -1,7 +1,12 @@ import { cleanup, fireEvent, render, screen } from '@solidjs/testing-library'; import { afterEach, describe, expect, it, vi } from 'vitest'; import { TableCell, TableRow } from '@/components/shared/Table'; -import { getPlatformResourceDetailRowInteractionProps } from '../PlatformResourceDetailTableRow'; +import { + PlatformResourceDetailToggleButton, + createPlatformResourceDetailState, + getPlatformResourceDetailRowClass, + getPlatformResourceDetailRowInteractionProps, +} from '../PlatformResourceDetailTableRow'; afterEach(cleanup); @@ -48,3 +53,65 @@ describe('getPlatformResourceDetailRowInteractionProps', () => { expect(onToggle).not.toHaveBeenCalled(); }); }); + +describe('platform resource drawer row toggle', () => { + afterEach(() => { + document.getSelection()?.removeAllRanges(); + }); + + it('keeps the drawer as it was when a drag selects the resource name', () => { + const resource = { id: 'pod-web' }; + render(() => { + const drawer = createPlatformResourceDetailState({ idPrefix: 'test-pod-drawer' }); + return ( + + + drawer.toggle(resource)} + > + + drawer.toggle(resource)} + /> + web-7d9f8b6c5-x2kqp + + + +
+ ); + }); + const name = screen.getByText('web-7d9f8b6c5-x2kqp'); + const toggle = screen.getByRole('button'); + const range = document.createRange(); + range.selectNodeContents(name); + const selection = document.getSelection()!; + selection.removeAllRanges(); + selection.addRange(range); + + const drag = () => { + fireEvent.mouseDown(name, { clientX: 4, clientY: 8 }); + fireEvent.click(name, { clientX: 96, clientY: 8 }); + }; + + drag(); + expect(toggle).toHaveAttribute('aria-expanded', 'false'); + + // A plain click on the still-selected name opens the drawer first time. + fireEvent.mouseDown(name, { clientX: 40, clientY: 8 }); + fireEvent.click(name, { clientX: 40, clientY: 8 }); + expect(toggle).toHaveAttribute('aria-expanded', 'true'); + + drag(); + expect(toggle).toHaveAttribute('aria-expanded', 'true'); + + // The chevron and its keyboard activation keep working with text selected. + fireEvent.click(toggle); + expect(toggle).toHaveAttribute('aria-expanded', 'false'); + fireEvent.keyDown(toggle, { key: 'Enter' }); + expect(toggle).toHaveAttribute('aria-expanded', 'true'); + }); +});