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..c5823d1cbc 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` 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/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..c269daff86 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'); 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'); + }); +});