Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/pluggableWidgets/datagrid-web/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),

## [Unreleased]

### Fixed

- We fixed an issue where clicking a label inside a custom content column, such as the label of a checkbox, triggered the double click action with a single click.

## [3.11.3] - 2026-07-27

### Added
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-08-19
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# fix-custom-content-label-double-click

Fix single click on a label inside custom content triggering the double click action (WC-3524)
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
## Context

Row/cell interaction in Data Grid 2 is built from `EventCaseEntry` objects (entry + optional filter) that `eventSwitch()` turns into React props. `CellEventsController.getEntries()` collects action handlers, selection handlers and focus-target handlers, then passes everything through `ClickEventSwitch`, which merges the `onClick` and `onDoubleClick` groups into a single synthetic `onClick` prop. Consequence: the browser's own `dblclick` event never drives the double-click action in production, and every double-click decision is made by the switch.

The switch is shared: `gallery-web` uses it for item interaction, so any change lands in both widgets.

## Goals / Non-Goals

Goals:

- One physical click never runs the double click action, regardless of what is inside the cell.
- A real double click still runs it exactly once.
- Custom content keeps working: the label-forwarded click must still reach the checkbox, so the DOM event is not suppressed or `preventDefault`ed.
- Fix at the producer (`ClickEventSwitch`), not at the consumer (per-column workarounds).

Non-Goals:

- Reinstating React's native `onDoubleClick` path for cells. That is a larger refactor of `CellEventsController` and would change which handlers can be filtered.
- Fixing the adjacent defects found while investigating (listed under Follow-ups).
- E2E coverage in `datagrid-web/e2e/`: it needs a new page with a custom-content checkbox column in the shared `testProjects` repo (`datagrid-web/data-widgets-3.0` branch) — a separate change to a separate repo.

## Decisions

### Classify from `MouseEvent.detail`, not from elapsed time

`detail` is the browser-maintained click counter and already honors the OS double-click interval, which a hard-coded 320 ms cannot. `1` → click entries, `2` → double click entries, `0` → neither (keyboard activation and `element.click()` report 0), `> 2` → neither (the double click group already ran at 2).

Alternatives rejected:

- Keep the timer, widen/narrow the window: no window works. The forwarded label click arrives with the _same_ timestamp, so any window that treats "two clicks close together" as a double click misfires.
- Listen to the native `dblclick` event instead: would require restructuring `CellEventsController`'s entry folding, and the entries' filters are written against a single `onClick` context.
- Ignore clicks whose `target !== currentTarget`: would break clicking custom content deliberately, and would not fix the plain-cell 1-2-1-2 reset defect.

### Collapse duplicates inside one gesture with a small time window

`detail` alone is insufficient: both events of a label click carry `detail: 1`, so with classification only, a single label click would run the click entries **twice** — breaking `onClickTrigger = "single"` grids and double-selecting rows.

Rule: ignore a click whose `detail` equals the previously handled click's `detail` and whose `timeStamp` is within `sameGestureWindow = 5` ms.

Why 5 ms is safe where 320 ms was not: a genuinely separate fast click carries a _different_ `detail` (1 then 2), so the guard cannot swallow it. The window only has to absorb dispatch jitter between two events of the same gesture — measured 0 ms in Chromium (identical `timeStamp`) and ~1 ms in jsdom.

### State lives in the closure returned by `getClickEntry()`

That closure is created per `DataCell` (`useMemo` on `[item, eventsController]`). Label forwarding happens within one cell, so per-cell state is the right granularity; module-level state would leak across rows and columns.

## Risks / Trade-offs

- Third and further clicks of a rapid gesture (`detail >= 3`) now run nothing. Previously they alternated click/double-click due to the `startTime = 0` reset. This is the intended shape, but it is a behavior change for triple-click users.
- `detail === 0` clicks are now ignored. This intentionally stops keyboard activation of a control inside custom content from firing the row action; it also means a programmatic `element.click()` no longer triggers row actions, which some custom JS snippets might have relied on.
- The 5 ms window is empirical. If a browser ever dispatches a label-forwarded click more than 5 ms after the original, the single-click action would run twice again. Chromium dispatches both in the same task with an identical timestamp.

## Migration Plan

None. Internal behavior change, no API or XML surface. Apps using `allowEventPropagation = false` as a workaround keep working; they can turn row events back on.

## Open Questions

Follow-ups found while investigating, deliberately out of scope:

1. `allowEventPropagation: false` only stops `onClick` / `onKeyUp` / `onKeyDown` on `.td-custom-content` (`src/helpers/state/column/ColumnStore.tsx`). `onMouseDown` (which calls `removeAllRanges()`, clearing text selection) and `onFocus` (which moves the keyboard focus target) still fire from inside custom content.
2. `canExecOnSpaceOrEnter` (`src/features/row-interaction/action-handlers.ts`) checks only `event.code`, with no `target === currentTarget` guard, so Space/Enter typed in a nested text box inside custom content fires the row's on-click action. Ctrl/Cmd+A already has that guard (`widget-plugin-grid/src/selection/keyboard.ts`).
3. `cell-pointer.spec.tsx` asserts native `dblclick` semantics that production does not have, because it bypasses `ClickEventSwitch`. Realigning it would make the row-interaction suite describe real behavior.
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
## Why

WC-3524: in a Data Grid 2 column with `showContentAs = customContent` holding a checkbox widget, **one** physical click on the checkbox's `<label>` runs the grid's **double click** action. The customer workaround is setting the column's "Allow row events" (`allowEventPropagation`) to `false`, which also stops legitimate row events for that column.

Root cause is not the checkbox and not the custom content wrapper. Cell handlers sit on `div.td`, and single-vs-double click is not decided by the browser: `CellEventsController.getEntries()` folds every `onDoubleClick` entry into one synthetic `onClick` entry via the shared `ClickEventSwitch`, which hand-rolls detection with a 320 ms `Date.now()` window. React's real `onDoubleClick` prop on the cell is dead code in production.

The `<label>` activation behavior makes the browser dispatch a **second, trusted** click on the labelled control, and that click bubbles back through the same `div.td`. Instrumented in real Chromium against the customer repro app, one label click produces:

| target | detail | isTrusted | timeStamp | cell |
| ------ | ------ | --------- | --------- | ---- |
| LABEL | 1 | true | 2151 | 4,0 |
| INPUT | 1 | true | 2151 | 4,0 |

Two clicks 0 ms apart: the 320 ms window classifies the second one as a double click, so the double click action runs from a single physical click. Clicking the checkbox square directly produces only one click, which is why the bug looks target-dependent.

The browser already tracks click count in `MouseEvent.detail`, honoring the OS double-click interval. The 320 ms re-implementation is what breaks, and it carries two further latent defects: `startTime = 0` after a double click makes the classification pattern 1-2-1-2 instead of 1-2-3-4, and the hard-coded 320 ms ignores the user's OS double-click interval.

## What Changes

- `ClickEventSwitch.getClickEntry()` (shared `@mendix/widget-plugin-grid`) classifies from `event.detail` instead of a timer: `1` runs the click entries, `2` runs the double click entries, `0` (keyboard activation, `element.click()`) and `> 2` run neither.
- The same switch collapses duplicate clicks belonging to **one** gesture: a click whose `detail` equals the previously handled click's `detail` and whose `timeStamp` is within 5 ms of it is ignored. This is what discards the label-forwarded duplicate without suppressing it in the DOM, so the checkbox keeps toggling.
- Regression coverage for the switch itself (it had none) and for a data grid cell containing a checkbox with its own label.
- Gallery inherits the same fix through the shared package (`ItemEvents.viewModel.ts` uses the same switch), so its changelog gets the equivalent entry.

## Capabilities

### New Capabilities

- `datagrid-click-gesture-classification`: how Data Grid 2 decides whether a pointer gesture on a row/cell is a single click or a double click for the purpose of running the configured on-click action and row selection, including gestures that produce more than one click event.

### Modified Capabilities

_None — no existing `openspec/specs/` capability documents cell click classification, so this is captured as a new capability rather than a delta._

## Impact

- `packages/shared/widget-plugin-grid/src/event-switch/ClickEventSwitch.ts` — the fix site. Consumed by `datagrid-web` (`CellEventsController`, `RowEventsController`) and `gallery-web` (`ItemEvents.viewModel.ts`).
- `packages/shared/widget-plugin-grid/src/event-switch/__tests__/ClickEventSwitch.spec.ts` — new, first coverage of the class.
- `packages/pluggableWidgets/datagrid-web/src/features/row-interaction/__tests__/cell-custom-content.spec.tsx` — new integration regression test through the real `CellEventsController` (the existing `cell-pointer.spec.tsx` builds props with raw `eventSwitch(...)` and bypasses `ClickEventSwitch`, so it cannot cover this).
- `packages/pluggableWidgets/datagrid-web/CHANGELOG.md`, `packages/pluggableWidgets/gallery-web/CHANGELOG.md` — user-facing fix entries.
- No XML/property changes, no widget API changes, no version bumps. `allowEventPropagation` keeps its current meaning; it is no longer needed as a workaround for this symptom.
- Behavior change worth noting: clicks with `detail === 0` (keyboard activation of a control inside custom content, programmatic `element.click()`) no longer run the row's click action. Keyboard activation of the row itself is handled by the separate key entries and is unaffected.
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
## ADDED Requirements

### Requirement: One pointer gesture runs at most one click classification

A single pointer gesture on a row or cell SHALL run either the configured single-click behavior or the double-click behavior, never both, and SHALL run it at most once — even when the gesture produces more than one DOM `click` event on the same element. Forwarded clicks, such as the click a browser dispatches on a labelled control when its `<label>` is clicked, SHALL NOT be counted as an additional gesture.

The widget SHALL NOT suppress the forwarded DOM event itself: content inside the cell keeps its native behavior, so a checkbox inside a custom content column still toggles.

#### Scenario: Single click on the label of a checkbox in a custom content column

- **GIVEN** a column with content shown as custom content, containing a checkbox with its own label, and the grid's on-click action configured to trigger on double click
- **WHEN** the user clicks the label once
- **THEN** the double click action does not run, and the checkbox toggles

#### Scenario: Single click on the label with the trigger set to single click

- **GIVEN** the same column, with the grid's on-click action configured to trigger on single click
- **WHEN** the user clicks the label once
- **THEN** the on-click action runs exactly once

#### Scenario: Double click on the label

- **GIVEN** the same column, with the on-click action configured to trigger on double click
- **WHEN** the user double clicks the label
- **THEN** the double click action runs exactly once

#### Scenario: Row selection by click is not doubled

- **GIVEN** a grid where rows are selected by clicking them, and a custom content column containing a checkbox with a label
- **WHEN** the user clicks the label once
- **THEN** the row's selection changes once, as if a single click had happened anywhere else in the row

#### Scenario: Selecting with the row selection checkbox column

- **GIVEN** a grid whose selection method is the checkbox column
- **WHEN** the user clicks a row's selection checkbox once
- **THEN** the row becomes selected and the grid's on-click action does not run

### Requirement: Click classification follows the browser click count

The widget SHALL derive single-versus-double click from the browser's click count for the event (`MouseEvent.detail`) rather than from a widget-owned time window, so the user's operating system double-click interval is respected.

A count of 1 SHALL run the single-click behavior, a count of 2 SHALL run the double-click behavior, and counts beyond 2 SHALL run neither — the double-click behavior having already run at 2. Successive gestures SHALL be classified independently: a click after a double click is a single click again.

#### Scenario: Two deliberate, separated clicks

- **WHEN** the user clicks a cell twice with a pause longer than the double-click interval
- **THEN** the single-click behavior runs twice and the double-click behavior does not run

#### Scenario: Clicking again after a double click

- **WHEN** the user double clicks a cell and then, after a pause, clicks and double clicks again
- **THEN** each gesture is classified on its own: two single clicks and two double clicks in total, with no alternating misclassification

#### Scenario: Triple click

- **WHEN** the user clicks three times in rapid succession
- **THEN** the single-click behavior runs once (first click), the double-click behavior runs once (second click), and the third click runs neither

### Requirement: Clicks that are not pointer gestures do not run row actions

A `click` event that reports a click count of 0 — as produced by keyboard activation of a control or by a programmatic `element.click()` — SHALL NOT run the row's click or double-click behavior. Keyboard interaction with the row itself remains served by the widget's keyboard handling.

#### Scenario: Activating a button inside custom content with the keyboard

- **GIVEN** a custom content column containing a button
- **WHEN** the user focuses that button and presses Enter or Space
- **THEN** the button's own action runs and the grid's row click action does not
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
## 1. Diagnose the mechanism in a real browser

- [x] 1.1 Instrument a capture-phase `click` listener on a synthetic label + checkbox page in real Chromium; record `detail`, `isTrusted`, `timeStamp`, `target`.
- [x] 1.2 Confirm the `<label>` forwards a second **trusted** click with the same `detail` and the same `timeStamp` (0 ms apart), and that clicking the checkbox square produces only one click.
- [x] 1.3 Confirm the same signature in the customer repro app through the real widget, with both events passing the same `div.td` (cell `data-position` 4,0).
- [x] 1.4 Read `CellEventsController.getEntries()` and confirm React's `onDoubleClick` prop is dead code in production — all double-click decisions come from `ClickEventSwitch`.

## 2. Rewrite the click classification (widget-plugin-grid)

- [x] 2.1 Replace the 320 ms `Date.now()` window in `ClickEventSwitch.getClickEntry()` with classification from `event.detail`: 1 → click entries, 2 → double click entries.
- [x] 2.2 Ignore `event.detail === 0` (keyboard activation, `element.click()`) and `event.detail > 2`.
- [x] 2.3 Add same-gesture dedupe: skip a click whose `detail` equals the previously handled click's `detail` and whose `timeStamp` is within `sameGestureWindow = 5` ms.
- [x] 2.4 Keep the state in the closure returned by `getClickEntry()` (per `DataCell`), not module level.
- [x] 2.5 Do not suppress or `preventDefault` the forwarded DOM event — the checkbox must keep toggling.

## 3. Unit coverage for ClickEventSwitch (was zero)

- [x] 3.1 New `packages/shared/widget-plugin-grid/src/event-switch/__tests__/ClickEventSwitch.spec.ts` driving the entry handler with plain `{detail, timeStamp}` objects (this package's Jest env is node — no RTL).
- [x] 3.2 Classification: `detail` 1 → single only, 2 → double only, 0 → neither, 3+ → neither.
- [x] 3.3 Same gesture: `1@100` + `1@100` → single runs once; `1@100` + `1@100.1` → once; `1,1,2,2` (label double click) → 1 single + 1 double; `1@100` + `2@100.1` → not collapsed.
- [x] 3.4 Separate gestures: `1@100` + `1@400` → two singles; `1,2,1,2` → 2 singles + 2 doubles (proves the `startTime = 0` reset defect is gone).
- [x] 3.5 Entry `filter` still honored; ctx and event still passed through.
- [x] 3.6 `pnpm run test` in `widget-plugin-grid`: 18 suites / 149 tests green.

## 4. Integration regression in datagrid-web

- [x] 4.1 New `src/features/row-interaction/__tests__/cell-custom-content.spec.tsx` rendering the real DOM shape (`.td` with controller props, `.td-custom-content` with `<label for>` + checkbox) driven by the real `CellEventsController`.
- [x] 4.2 Trigger `double`: label single click executes the action 0 times; checkbox single click 0 times; label double click exactly once.
- [x] 4.3 Trigger `single`: label click and checkbox click each execute exactly once.
- [x] 4.4 `rowClick` selection: label click selects exactly once; checkbox still toggles.
- [x] 4.5 `pnpm run test` in `datagrid-web` (18 suites / 231 tests) and `gallery-web` (8 suites / 56 tests) green.

## 5. A/B verification in the customer repro app

- [x] 5.1 Build shared package then widget into the repro project; verify which version the runtime serves via `deployment/web/widgets/.../Datagrid.js` (`awaitTime` vs `sameGestureWindow`).
- [x] 5.2 Pre-fix build, trigger `double`: single label click → 2 clicks (`detail` 1, 1, same timestamp) and the double-click microflow **fires** — bug reproduced on this exact pipeline.
- [x] 5.3 Post-fix build, trigger `double`: identical DOM event stream, microflow does **not** fire, checkbox toggles; real double click still fires it once; plain cell single click fires nothing and double click fires once.
- [x] 5.4 Post-fix build, trigger `single` + checkbox selection method: single label click fires the action exactly once; plain cell single click fires once; clicking the selection checkbox selects the row once and fires no action.

## 6. Release hygiene

- [x] 6.1 `datagrid-web/CHANGELOG.md` entry under `[Unreleased]` / `Fixed`.
- [x] 6.2 `gallery-web/CHANGELOG.md` equivalent entry (gallery consumes the same shared switch).
- [x] 6.3 No version bumps, no XML/property changes, no changelog for the shared package.
- [x] 6.4 Delete the throwaway Playwright diagnostic files (`e2e/wc3524-label-diag.spec.js`, `e2e/wc3524-probe.spec.js`, `e2e/wc3524-probe.png`, `wc3524.playwright.config.cjs`, `test-results/`) before opening the PR.
Loading
Loading