feat: HTML5 drag-and-drop event support (#101)

[adnaan]

Summary

Closes #101.

Adds six HTML5 drag events to the lvt-on:* delegation surface — dragstart, dragover, drop, dragend, dragenter, dragleave — with smart source/target serialization built on the native DataTransfer API.

• dragstart auto-stashes the dragged element's nearest data-key into dataTransfer (application/x-lvt-key MIME, plus text/plain for older Firefox compatibility) and sets effectAllowed = "move".
• drop reads that key back as data.dragSourceKey and lifts the drop target's nearest data-key as data.dragTargetKey. The result is a single drop action message that carries everything a sortable controller needs:

  {"action": "reorder", "data": {"dragSourceKey": "task-3", "dragTargetKey": "task-1"}}

• dragover and drop call preventDefault() unconditionally — required by the HTML5 spec for drop to fire at all. The side-effects run synchronously in a Layer-A block before any throttle gate, so lvt-mod:throttle on dragover only suppresses the WS send, never the spec-mandated preventDefault.
• A follow-up commit adds an empty-action marker pattern: lvt-on:dragover="" (or lvt-on:dragstart="") opts an element into the side-effects without paying for a WS round-trip — useful when only the drop event needs server-side handling.
• dragend / dragenter / dragleave forward as plain action messages (no drag-specific payload) for visual-feedback use cases.

The wire reorder protocol is unchanged: server controllers mutate the state slice and the diff engine auto-emits ["o", newKeys]. No core library work needed.

Test plan

• 14 new Jest cases in tests/event-delegation.test.ts (cast to 16 with the marker-pattern follow-up); covers dragstart key-stash, dragover preventDefault under throttling, drop source/target injection, text/plain Firefox fallback, nested drop targets, dragend/dragenter/dragleave forwarding, empty-action skip, end-to-end reorder roundtrip
• npm test — all 477 tests pass, no regressions
• npm run build — IIFE bundle clean (84306 → 85064 bytes, +758)
• Manually verified end-to-end against the new Sortable List example in livetemplate/examples (companion PR) running the local dist build

Companion / dependency

A companion PR in livetemplate/examples adds a Sortable List pattern that demonstrates the new events end-to-end. That PR depends on this one being merged + released to npm before its CI can pass against @latest.

🤖 Generated with Claude Code

[claude]

Review: drag-and-drop event delegation

Overall the implementation is solid — the side-effect-before-throttle pattern is correct and well-explained, and the test coverage is thorough.

Bug: dragTargetKey can be null

// dom/event-delegation.ts ~line 394
message.data.dragTargetKey = tgtEl.getAttribute("data-key");

getAttribute returns string | null. closest("[data-key]") confirms the attribute exists, but TypeScript doesn't narrow that, so this could assign null. For consistency with how dragSourceKey is handled (only set when truthy), this should be:

const key = tgtEl.getAttribute("data-key");
if (key) message.data.dragTargetKey = key;

Security: external drag sources can inject arbitrary dragSourceKey

The text/plain fallback on drop:

const src = dropDt.getData(LVT_DRAG_MIME) || dropDt.getData("text/plain");

If a user drags text from outside the app (another tab, a text editor, etc.), LVT_DRAG_MIME is empty and text/plain holds arbitrary content, which gets forwarded to the server as dragSourceKey. The server must validate that received keys are known/authorized values — an unvalidated dragSourceKey could trigger unintended reorder operations.

Minor: dragenter/dragleave will fire on nested children

These events bubble through child elements, so a drop target with child nodes will see spurious dragenter/dragleave pairs as the pointer moves over children. Worth a note in docs, since users implementing highlight-on-hover will hit this.

Minor: empty-action early return applies to all drag events, not just dragover

The comment only mentions lvt-on:dragover="" as the marker pattern, but the action === "" return fires for all drag event types. lvt-on:drop="" silently skips key injection and send — tested, but undocumented. Worth a comment or README note.

[Copilot]

Pull request overview

Adds HTML5 drag-and-drop support to the lvt-on:* event delegation surface, including drag-specific side effects (DataTransfer key stashing, preventDefault, and effect/dropEffect settings) and drop payload augmentation for sortable/reorder use cases.

Changes:

• Register six drag event types (dragstart, dragover, drop, dragend, dragenter, dragleave) in the delegated event listener set.
• Implement drag side-effects (DataTransfer serialization + spec-required preventDefault) and a marker pattern (lvt-on:drag*="") to opt into side-effects without sending WS messages.
• Add Jest coverage for drag serialization, throttling interaction, marker pattern behavior, and end-to-end payload shape; update README feature list.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
dom/event-delegation.ts	Adds delegated drag event types, drag side-effects, marker pattern early-return, and drop payload injection for source/target keys.
tests/event-delegation.test.ts	Introduces comprehensive drag-and-drop delegation tests with a DataTransfer mock and throttling assertions.
README.md	Documents drag-and-drop support in the Event Delegation feature bullet.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

In the drop payload injection, dragTargetKey is currently only derived when dataTransfer is present because the target lookup is inside if (dropDt). This means a drop event with a missing/null dataTransfer (possible in some environments, and matches the test file’s note about JSDOM) would omit dragTargetKey even though it can be derived from the DOM alone. Consider moving the closest('[data-key]') / getAttribute('data-key') block outside the dropDt conditional, and only conditionally injecting dragSourceKey based on dataTransfer contents (also avoid assigning a potential null attribute value).

Suggested change

	const tgtEl = actionElement.closest("[data-key]");
	if (tgtEl) {
	message.data.dragTargetKey = tgtEl.getAttribute("data-key");
	}
	const tgtEl = actionElement.closest("[data-key]");
	if (tgtEl) {
	const tgt = tgtEl.getAttribute("data-key");
	if (tgt !== null) {
	message.data.dragTargetKey = tgt;

[claude]

Review: feat/drag-and-drop event support

Overall the implementation is solid — the Layer-A side-effects-before-throttle design is correct, the text/plain Firefox fallback is handled, and the empty-action marker pattern is a nice ergonomic touch. A few issues to address before merge:

Bug: test DOM pollution causes false positives

Every drag test appends elements to document.body using shared IDs (id="src", id="tgt", id="inner") with no cleanup. document.getElementById always returns the first match in the document, so by the second test that uses id="src", you're dispatching events against the wrong element. Tests currently pass only because the first-inserted element happens to exercise the same code path. Add afterEach(() => wrapper.remove()) or use wrapper.querySelector instead of document.getElementById.

Security: text/plain fallback accepts arbitrary cross-app input

dragSourceKey in the drop payload silently promotes text/plain from a cross-app drag (any string the user dragged from another tab, a file manager, etc.) into a key that controllers act on. The README and comment say "controllers MUST validate" but that's easy to miss. Consider adding a separate boolean field (e.g. dragSourceKeyTrusted: false) when the text/plain fallback path is taken, so server controllers can distinguish a same-app key from external input without having to parse it themselves.

Performance footgun: dragover without throttle floods the server

dragover fires at 60 Hz during a drag. The empty-action marker pattern (lvt-on:dragover="") handles the pure-preventDefault case, but any element with a real action and no lvt-mod:throttle will fire a WS message on every animation frame. A note in the README recommending lvt-mod:throttle on dragover handlers would prevent surprises.

Minor: hardcoded "move" effect

effectAllowed and dropEffect are unconditionally set to "move". Copy/link drag semantics aren't configurable. Probably fine for v1, but worth documenting as a limitation.

[claude]

Review: Drag-and-Drop Event Delegation

Good implementation overall — the MIME-based source verification (LVT vs. text/plain) is a solid security boundary, and running side-effects before the throttle gate is the right call for dragover/dragstart. Test coverage is thorough. Three issues worth addressing:

1. text/plain leaks internal data-key values outside the app

// dom/event-delegation.ts
dragEvent.dataTransfer.setData("text/plain", key);

If a user accidentally drops a dragged element on the browser URL bar, a text editor, or any external target, the raw data-key value gets pasted there. Consider using a randomized session token for the text/plain slot (Firefox compat) while keeping the real key in the LVT MIME:

dragEvent.dataTransfer.setData(LVT_DRAG_MIME, key);
dragEvent.dataTransfer.setData("text/plain", crypto.randomUUID()); // opaque to external targets

2. Unthrottled dragover is a server-flood risk by default

The README recommends lvt-mod:throttle="100" but doesn't enforce it. A developer who skips the README will ship ~60 WS messages/second. Consider logging a dev-mode warning when a dragover binding is registered without a throttle modifier, or defaulting to a built-in throttle that can be overridden.

3. dragenter/dragleave child-traversal noise has no code-level mitigation

The README documents the spurious-event problem, but the code provides no helper. A relatedTarget guard would make the feature safe to use without deep docs-diving:

if ((eventType === "dragenter" || eventType === "dragleave") && e instanceof DragEvent) {
  if (actionElement.contains(e.relatedTarget as Node)) return; // crossing child boundary — skip
}

This would be the correct place to add it (before the empty-action early-return), and it's worth a test.

---

The core logic and security model are sound. Items 1 and 3 are the most important to fix before shipping.

[claude]

Review: HTML5 Drag-and-Drop Support

Overall this is well-implemented. The security design (custom MIME only, never text/plain) is correct and the dragover side-effects running before the throttle gate is critical — good catch. Test coverage is thorough.

One behavioral edge case to verify

dragenter/dragleave relatedTarget filter is asymmetric between the two events.

The check actionElement.contains(related) filters events where relatedTarget is a descendant. For dragenter this is correct (suppress noise when dragging from a child back to the parent). For dragleave this is also correct (suppress when leaving the parent for a child). Both are fine.

However, Element.contains(element) returns true for self, so if relatedTarget === actionElement (possible in synthetic-event environments), the event is silently dropped. Probably harmless in practice, but worth a comment near the filter.

Silent ambiguity on drop: no-key dragstart vs. cross-app drag

When dragstart fires on an element with no enclosing data-key, the code stashes "" via setData. On drop, getData returns "" — falsy — so dragSourceKey is omitted, identical to a cross-app drag. A server controller can't distinguish the two cases. Low impact while effectAllowed is hardcoded to "move", but worth a note in the v1 limitations.

README

The feature bullet is a wall of text with 5 inline caveats. Consider a short bullet + a separate doc section or <details> block — the content is useful but hard to scan inline.

Verdict: LGTM — no blockers.

[adnaan]

Round-4 review comments triaged:

✅ Applied: README readability — the inline 5-caveat bullet is now a dedicated "Drag-and-drop notes" subsection (commit d3e357c).

❌ Declined with reason:

1. Comment on relatedTarget self-equality. The bot itself calls this "probably harmless in practice." Adding a 1-line comment for an edge case that doesn't occur in real DnD (only in synthetic-event environments where someone explicitly sets relatedTarget === actionElement) is bikeshedding.

2. Distinguishing no-key dragstart from cross-app drag on the server. Both legitimately produce the same payload (no dragSourceKey) by design — the server is meant to react identically in both cases (return state unchanged because the key is missing). It's not a v1 limitation, it's the intended trust model: anything not carried via application/x-lvt-key is untrusted, period.

Treating round 4 as the convergence signal per project CLAUDE.md (round 3 had two functional fixes — text/plain write removal + dragenter relatedTarget filter — round 4 returned only style/phrasing nits with an explicit "LGTM — no blockers").

[claude]

Drag-and-drop review

Overall this is solid work. The MIME-only trust boundary, side-effects-before-throttle ordering, and marker pattern are all correct. Test coverage is thorough. Two things worth addressing:

Bug: empty dragSourceKey silently treated as a cross-app drag

In dragstart, when the dragged element has no [data-key] ancestor:

const key = keyEl?.getAttribute("data-key") ?? "";
dragEvent.dataTransfer.setData(LVT_DRAG_MIME, key); // sets MIME -> ""

On drop, getData(LVT_DRAG_MIME) returns "", which is falsy, so dragSourceKey is omitted:

const src = (e as DragEvent).dataTransfer?.getData(LVT_DRAG_MIME);
if (src) {  // false for ""
  message.data.dragSourceKey = src;
}

A same-app drag from an unkeyed element is now indistinguishable from a cross-app drag at the server. The test "dragstart with no enclosing data-key sends empty key without throwing" verifies no crash and that setData is called, but doesn't test the drop payload. This seems like the wrong silent default — consider either requiring a data-key on the drag source (and logging a warning if absent), or sending dragSourceKey: "" explicitly so the server can distinguish the two cases.

dragenter suppression also fires on "entering from a child"

The containment check:

if (related && actionElement.contains(related)) return;

For dragleave this is clearly correct (suppress leaving-to-child noise). For dragenter, relatedTarget is the element the cursor was previously over, so this also suppresses entering the zone when coming from a child — e.g., pointer was over a child <span> then moves onto the parent <li>. This is probably the right behavior for drop-zone highlighting, but it's asymmetric from what the name implies. A comment or a test covering this specific direction would help future maintainers.

Minor

README docs are clear and the "v1 limitation" callout for effectAllowed/dropEffect is a good signal to set expectations.

[adnaan]

Round-5 review essentially restates round-4 nits:

1. "Empty dragSourceKey silently treated as cross-app drag" — same as round-4 item 2 (declined: it's the intended trust model; both cases produce the same payload because the server is meant to react identically — no-op + return state).

2. "dragenter suppression also fires when entering from a child" — same as round-4 item 1 in different phrasing (declined: bot's own wording in round 4 was "probably harmless in practice"). The behavior is symmetric and correct for drop-zone highlighting; my existing test dragenter is suppressed when relatedTarget is a child of the action element already covers it (the spec model is "relatedTarget = previous pointer location," so child→parent and parent→child are both contains(related) === true and both correctly suppressed).

This is the convergence signal per project CLAUDE.md: rounds 1–3 produced functional feedback (all addressed); rounds 4–5 produced only style/phrasing on content already accepted. Stopping the bot loop here.