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
This commit is contained in:
46
plugins/kanban/dashboard/dist/index.js
vendored
46
plugins/kanban/dashboard/dist/index.js
vendored
@@ -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);
|
||||
|
||||
77
tests/plugins/fixtures/kanban_touch_drag_probe.js
Normal file
77
tests/plugins/fixtures/kanban_touch_drag_probe.js
Normal file
@@ -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 <path-to-bundle>
|
||||
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");
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user