Keep a text-selection drag from toggling table rows (#2524)

Dragging across a pod or container name to copy it ends with a click on
the row the drag stayed inside. Every platform table ran its row action
on that click, so the detail drawer opened or closed and shifted the
layout under the selection.

**Change**

- `TableRow` (`components/shared/Table.tsx`) owns a per-row
`createRowTextSelectionGuard()`. It records where the row was pressed. A
click whose pointer moved more than 3px since that press, while text
inside the row is selected, skips the row action.
- 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, so a plain selection check alone would need a
second click.
- The 37 `drawer.toggle` tables, the Proxmox tables built on
`getPlatformResourceDetailRowInteractionProps`, and `PulseDataGrid`
inherit the guard with no call-site changes. The custom `GuestRow`
(Workloads) and `StoragePoolRow` shells wire the same guard.
- The disclosure chevron and its keyboard activation handle their own
events and are unchanged. A double-click is two stationary clicks, so it
behaves as before.

**Verification**

- New cases in `Table.test.tsx`, `PulseDataGrid.test.tsx`,
`GuestRow.test.tsx`, `Storage.test.tsx` and
`PlatformResourceDetailTableRow.test.tsx` cover drags, in-place clicks
over a selection, selections outside the row, bound handlers, and the
chevron plus Enter while text stays selected. All six fail with the
guard disabled.
- `src/features src/hooks src/__tests__
src/components/{shared,Workloads,Storage,Infrastructure}`: 580 files and
10,012 tests pass.
- Ran Playwright with real mouse drags on a private mock stack at
1440x900 and 768x1024 across Kubernetes pods, Docker containers, Proxmox
hosts, guests and storage pools, plus 390x844 touch taps.
- The drag reproduces the toggle on the parent commit and leaves state
unchanged on this branch.
  - A plain click on the selected name opens the row first time.
- Receipt: `frontend-modern/browser-verification/4f08442c2617c48d.json`.
This commit is contained in:
rcourtman
2026-10-05 17:19:04 +01:00
committed by GitHub
12 changed files with 407 additions and 6 deletions
@@ -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
@@ -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`
@@ -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.
@@ -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."
]
}
@@ -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<StoragePoolRowProps> = (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<StoragePoolRowProps> = (props) => {
<>
<tr
class={`${STORAGE_POOL_ROW_CLASS} ${STORAGE_POOL_ROW_HEIGHT_CLASS} ${props.rowClass} ${props.expanded ? STORAGE_POOL_ROW_EXPANDED_CLASS : ''}`.trim()}
onClick={props.onToggleExpand}
on:mousedown={selectionGuard.onMouseDown}
onClick={(event) => {
if (!selectionGuard.isSelectionClick(event)) props.onToggleExpand();
}}
{...interactiveRowHandlers}
data-summary-series-id={props.summarySeriesId}
data-summary-group-member-active={
@@ -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(() => <Storage />);
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',
@@ -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 */}
@@ -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');
@@ -69,12 +69,68 @@ export type TableRowProps = JSX.HTMLAttributes<HTMLTableRowElement>;
// 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<HTMLTableRowElement, MouseEvent> = (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 (
<tr
class={`group transition-colors duration-150 hover:bg-surface-hover ${local.class || ''}`}
on:click={rest.onClick ? nativeRowClickTarget : undefined}
on:click={local.onClick ? nativeRowClickTarget : undefined}
on:mousedown={local.onClick ? selectionGuard.onMouseDown : undefined}
{...rowAction}
{...rest}
>
{local.children}
@@ -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(() => (
<PulseDataGrid<TestRow>
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();
@@ -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(() => (
<Table>
<TableBody>
<TableRow onClick={onClick}>
<TableCell>
<button
type="button"
onClick={(event) => {
event.stopPropagation();
onDisclosure();
}}
>
Details
</button>
<span>web-7d9f8b6c5-x2kqp</span>
</TableCell>
</TableRow>
</TableBody>
</Table>
));
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(() => (
<>
<p>Cluster heading</p>
<Table>
<TableBody>
<TableRow onClick={[onClick, 'bound-identity']}>
<TableCell>api-server</TableCell>
</TableRow>
</TableBody>
</Table>
</>
));
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');
@@ -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 (
<table>
<tbody>
<TableRow
class={getPlatformResourceDetailRowClass(drawer.isExpanded(resource))}
onClick={() => drawer.toggle(resource)}
>
<TableCell>
<PlatformResourceDetailToggleButton
expanded={drawer.isExpanded(resource)}
resourceLabel="web-7d9f8b6c5-x2kqp"
controlsId={drawer.detailRowId(resource)}
onToggle={() => drawer.toggle(resource)}
/>
<span>web-7d9f8b6c5-x2kqp</span>
</TableCell>
</TableRow>
</tbody>
</table>
);
});
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');
});
});