From 88d2d90fe1b6edc624412328abac7428bf4844cd Mon Sep 17 00:00:00 2001 From: Tyler Lyon Date: Fri, 18 Sep 2026 22:57:40 -0400 Subject: [PATCH] fix(kanban): tapping a card on touch opens it instead of moving it attachTouchDrag() armed a drag on ANY touch pointerdown and immediately called preventDefault(), which suppresses the synthesized click TaskCard.handleClick relies on to call props.onOpen(). There was no movement threshold, so a finger drifting even ~2-3px on a normal tap -- which is universal on real touch hardware -- was enough to arm the drag and swallow the open. Fix: defer starting the drag proxy and calling preventDefault() until the pointer has actually moved past an 8px threshold (matches the common native drag-affordance convention). A stationary tap never crosses the threshold, dragging is never armed, and the click fires normally. A real drag still claims the gesture identically to before, just after the same few pixels of travel every touch drag implementation already tolerates. The bundle (plugins/kanban/dashboard/dist/index.js) has no build step -- it is hand-maintained directly, as established by prior kanban dashboard PRs (#114882, #108694) -- so the fix is applied there. Closes #115568. Testing: no jsdom/vitest harness exists for this bundle (confirmed by PR #114882's review follow-up, which explicitly rejected turning a "live-repro jsdom harness" into a pytest because jsdom/react aren't declared in the root package.json and the Python CI job has no node_modules -- such a test would be vacuous in CI). Per that precedent and the "never read source code in tests" rule (no regex/substring pin on the bundle text), this PR instead extracts attachTouchDrag() verbatim at test time via Node (already present: tests-js/ + vitest are in the repo) and drives it through real pointerdown/pointermove/pointerup sequences against a minimal DOM stub -- a behavioral test, not a source-shape test. Proven red on the unfixed bundle (asserts preventDefault is called on a stationary tap) and green on the fix; skips cleanly via shutil.which("node") if Node is unavailable in a given lane. Verification: - node tests/plugins/fixtures/kanban_touch_drag_probe.js against the ORIGINAL (unfixed) bundle: fails with "FAIL: a stationary tap called preventDefault (suppresses the click)", exit 1 -- confirms the probe reproduces the reported bug - Same probe against the fixed bundle: "PASS", exit 0 - scripts/run_tests.sh tests/plugins/test_kanban_dashboard_plugin.py -- 42/42 passed (1 new, 41 unchanged) - node --check plugins/kanban/dashboard/dist/index.js -- syntax OK --- plugins/kanban/dashboard/dist/index.js | 46 ++++++++--- .../fixtures/kanban_touch_drag_probe.js | 77 +++++++++++++++++++ tests/plugins/test_kanban_dashboard_plugin.py | 26 +++++++ 3 files changed, 137 insertions(+), 12 deletions(-) create mode 100644 tests/plugins/fixtures/kanban_touch_drag_probe.js diff --git a/plugins/kanban/dashboard/dist/index.js b/plugins/kanban/dashboard/dist/index.js index 11fd0ad926..658ae11c5b 100644 --- a/plugins/kanban/dashboard/dist/index.js +++ b/plugins/kanban/dashboard/dist/index.js @@ -468,15 +468,44 @@ function attachTouchDrag(el, taskId) { if (!el) return; + // A finger drifts a few px on every real tap; without a movement threshold ANY touch + // pointerdown armed a drag and called preventDefault(), which suppresses the synthesized + // click the card relies on to open (#115568). Defer the drag proxy + preventDefault until + // the pointer has actually moved past DRAG_THRESHOLD_PX; a tap that never crosses it falls + // through to the native click, same as it already does for a mouse. + const DRAG_THRESHOLD_PX = 8; function onDown(e) { if (e.pointerType !== "touch") return; - e.preventDefault(); - const proxy = el.cloneNode(true); - proxy.classList.add("hermes-kanban-touch-proxy"); - document.body.appendChild(proxy); + const startX = e.clientX; + const startY = e.clientY; + let proxy = null; let lastTarget = null; + let dragging = false; + + function startDrag() { + dragging = true; + proxy = el.cloneNode(true); + proxy.classList.add("hermes-kanban-touch-proxy"); + document.body.appendChild(proxy); + proxy.style.position = "fixed"; + proxy.style.pointerEvents = "none"; + proxy.style.opacity = "0.85"; + proxy.style.zIndex = "9999"; + proxy.style.width = `${el.offsetWidth}px`; + proxy.style.left = `${startX - el.offsetWidth / 2}px`; + proxy.style.top = `${startY - 24}px`; + } function move(ev) { + if (!dragging) { + const dx = ev.clientX - startX; + const dy = ev.clientY - startY; + if (Math.hypot(dx, dy) < DRAG_THRESHOLD_PX) return; + startDrag(); + } + // Only now, once a drag is actually underway, does it claim the gesture — a stationary + // tap never reaches preventDefault() and its click event fires normally. + ev.preventDefault(); proxy.style.left = `${ev.clientX - proxy.offsetWidth / 2}px`; proxy.style.top = `${ev.clientY - 24}px`; proxy.style.display = "none"; @@ -495,6 +524,7 @@ document.removeEventListener("pointermove", move); document.removeEventListener("pointerup", up); document.removeEventListener("pointercancel", up); + if (!dragging) return; if (lastTarget) { lastTarget.classList.remove("hermes-kanban-column--drop"); const status = lastTarget.getAttribute("data-kanban-column"); @@ -513,14 +543,6 @@ } proxy.remove(); } - // Kick off proxy at the pointer origin. - proxy.style.position = "fixed"; - proxy.style.pointerEvents = "none"; - proxy.style.opacity = "0.85"; - proxy.style.zIndex = "9999"; - proxy.style.width = `${el.offsetWidth}px`; - proxy.style.left = `${e.clientX - el.offsetWidth / 2}px`; - proxy.style.top = `${e.clientY - 24}px`; document.addEventListener("pointermove", move); document.addEventListener("pointerup", up); document.addEventListener("pointercancel", up); diff --git a/tests/plugins/fixtures/kanban_touch_drag_probe.js b/tests/plugins/fixtures/kanban_touch_drag_probe.js new file mode 100644 index 0000000000..fdbf032468 --- /dev/null +++ b/tests/plugins/fixtures/kanban_touch_drag_probe.js @@ -0,0 +1,77 @@ +// Behavioral probe for attachTouchDrag() (#115568): extracts the function from the shipped +// dashboard bundle (no build step — the bundle IS the source) and drives it through real +// pointerdown/pointermove/pointerup sequences with a minimal DOM stub. Exits 0 and prints "PASS" +// when a stationary tap never claims the gesture (leaves preventDefault/dispatchEvent untouched +// so the synthesized click still opens the card) and a real drag still claims it past the +// movement threshold. Run via: node kanban_touch_drag_probe.js +const fs = require("fs"); + +const bundlePath = process.argv[2]; +const src = fs.readFileSync(bundlePath, "utf8"); +const start = src.indexOf("function attachTouchDrag"); +if (start === -1) { console.error("attachTouchDrag not found in bundle"); process.exit(1); } +const bodyStart = src.indexOf("{", start); +let depth = 0, end = bodyStart; +for (; end < src.length; end++) { + if (src[end] === "{") depth++; + else if (src[end] === "}") { depth--; if (depth === 0) break; } +} +const fnSrc = src.slice(start, end + 1); + +class FakeEl { + constructor() { + this.listeners = {}; + this.classList = { add() {}, remove() {}, contains() { return false; } }; + this.style = {}; + this.offsetWidth = 100; + } + addEventListener(t, f) { this.listeners[t] = f; } + removeEventListener(t) { delete this.listeners[t]; } + cloneNode() { return new FakeEl(); } + closest() { return null; } + getAttribute() { return null; } + hasAttribute() { return false; } + dispatchEvent(ev) { this.dispatched = (this.dispatched || []).concat([ev.type]); } + remove() {} +} +const docListeners = {}; +global.document = { + body: { appendChild() {} }, + addEventListener(t, f) { docListeners[t] = f; }, + removeEventListener(t) { delete docListeners[t]; }, + elementFromPoint() { return null; }, +}; +global.CustomEvent = function (type, opts) { this.type = type; this.detail = opts && opts.detail; }; + +eval(fnSrc); + +// A real tap: pointerdown then pointerup with sub-threshold jitter must NOT claim the gesture. +const tapEl = new FakeEl(); +attachTouchDrag(tapEl, "task-tap"); +const tapDown = { pointerType: "touch", clientX: 100, clientY: 100, preventDefault() { this._pd = true; } }; +tapEl.listeners["pointerdown"](tapDown); +const tapMove = { clientX: 102, clientY: 101, preventDefault() { this._pd = true; } }; +docListeners["pointermove"](tapMove); +docListeners["pointerup"]({}); + +if (tapDown._pd || tapMove._pd) { + console.error("FAIL: a stationary tap called preventDefault (suppresses the click)"); + process.exit(1); +} +if (tapEl.dispatched) { + console.error("FAIL: a stationary tap dispatched a drag/drop event"); + process.exit(1); +} + +// A real drag: movement past the threshold must still claim the gesture. +const dragEl = new FakeEl(); +attachTouchDrag(dragEl, "task-drag"); +dragEl.listeners["pointerdown"]({ pointerType: "touch", clientX: 100, clientY: 100, preventDefault() {} }); +let dragClaimed = false; +docListeners["pointermove"]({ clientX: 140, clientY: 140, preventDefault() { dragClaimed = true; } }); +if (!dragClaimed) { + console.error("FAIL: a real drag past the threshold never claimed the gesture"); + process.exit(1); +} + +console.log("PASS"); diff --git a/tests/plugins/test_kanban_dashboard_plugin.py b/tests/plugins/test_kanban_dashboard_plugin.py index d88d33184c..cdeaede133 100644 --- a/tests/plugins/test_kanban_dashboard_plugin.py +++ b/tests/plugins/test_kanban_dashboard_plugin.py @@ -11,6 +11,7 @@ import importlib.util import json import os import subprocess +import shutil import sys import time from pathlib import Path @@ -1278,3 +1279,28 @@ def test_specify_happy_path(client, monkeypatch): # --------------------------------------------------------------------------- + + +# --------------------------------------------------------------------------- +# Touch drag-vs-tap threshold (#115568) +# --------------------------------------------------------------------------- + +def test_touch_card_tap_opens_instead_of_dragging(): + """attachTouchDrag() must not claim a stationary tap: without a movement threshold, + every touch pointerdown called preventDefault() immediately, which suppresses the + synthesized click TaskCard.handleClick relies on to call props.onOpen() (#115568). + The bundle has no build step, so this runs the real function (extracted verbatim, not + regex-matched) through a real pointerdown/move/up sequence with a minimal DOM stub — + behavioral, not a source-text pin. + """ + node = shutil.which("node") + if not node: + pytest.skip("node not available") + bundle = Path(__file__).resolve().parents[2] / "plugins" / "kanban" / "dashboard" / "dist" / "index.js" + probe = Path(__file__).parent / "fixtures" / "kanban_touch_drag_probe.js" + result = subprocess.run( + [node, str(probe), str(bundle)], + capture_output=True, text=True, timeout=30, + ) + assert result.returncode == 0, f"stdout={result.stdout!r} stderr={result.stderr!r}" + assert "PASS" in result.stdout