diff --git a/.changeset/brown-rooms-push.md b/.changeset/brown-rooms-push.md new file mode 100644 index 000000000..c6a7f483b --- /dev/null +++ b/.changeset/brown-rooms-push.md @@ -0,0 +1,5 @@ +--- +'@dnd-kit/abstract': patch +--- + +Fix `dragstart` events so `operation.target` includes the initial droppable target while preserving `dragstart` before `dragover` ordering and queued `setDropTarget()` completion semantics. diff --git a/packages/abstract/src/core/manager/actions.ts b/packages/abstract/src/core/manager/actions.ts index e729c83ce..dcedcd053 100644 --- a/packages/abstract/src/core/manager/actions.ts +++ b/packages/abstract/src/core/manager/actions.ts @@ -8,9 +8,19 @@ import type { } from '../entities/index.ts'; import type {DragDropManager} from './manager.ts'; -import {defaultPreventable} from './events.ts'; +import {defaultPreventable, type DragOverEvent} from './events.ts'; import {StatusValue} from './status.ts'; +type DeferredDragOverEvent< + T extends Draggable, + U extends Droppable, + V extends DragDropManager, +> = { + event: DragOverEvent; + resolve(value: boolean): void; + reject(reason?: unknown): void; +}; + /** * Provides actions for controlling drag and drop operations. * @@ -30,6 +40,75 @@ export class DragActions< */ constructor(private readonly manager: V) {} + #deferredDragOverEvents: DeferredDragOverEvent[] | null = null; + + #dispatchDragOver(event: DragOverEvent): Promise { + const deferredEvents = this.#deferredDragOverEvents; + + if (deferredEvents) { + return new Promise((resolve, reject) => { + deferredEvents.push({event, resolve, reject}); + }); + } + + return this.#dispatchDragOverNow(event); + } + + #dispatchDragOverNow(event: DragOverEvent): Promise { + this.manager.monitor.dispatch('dragover', event); + return this.manager.renderer.rendering.then(() => event.defaultPrevented); + } + + #flushDeferredDragOverEvents() { + const events = this.#deferredDragOverEvents; + this.#deferredDragOverEvents = null; + + if (!events) { + return; + } + + let error: unknown; + + for (const {event, resolve, reject} of events) { + try { + this.#dispatchDragOverNow(event).then(resolve, reject); + } catch (reason) { + error ??= reason; + reject(reason); + } + } + + if (error) { + throw error; + } + } + + #resolveDeferredDragOverEvents(value: boolean) { + const events = this.#deferredDragOverEvents; + this.#deferredDragOverEvents = null; + + if (!events) { + return; + } + + for (const {resolve} of events) { + resolve(value); + } + } + + #rejectDeferredDragOverEvents(reason: unknown) { + const events = this.#deferredDragOverEvents; + this.#deferredDragOverEvents = null; + + if (!events) { + return; + } + + for (const {reject} of events) { + reject(reason); + } + } + /** * Sets the source of the drag operation. * @@ -67,7 +146,7 @@ export class DragActions< }); if (dragOperation.status.dragging) { - this.manager.monitor.dispatch('dragover', event); + return this.#dispatchDragOver(event); } return this.manager.renderer.rendering.then(() => event.defaultPrevented); @@ -144,14 +223,41 @@ export class DragActions< const {status} = dragOperation; if (status.current !== StatusValue.Initializing) return; - batch(() => { - dragOperation.status.set(StatusValue.Dragging); + this.#deferredDragOverEvents = []; - this.manager.monitor.dispatch('dragstart', { - nativeEvent, - operation: dragOperation.snapshot(), - cancelable: false, - }); + dragOperation.status.set(StatusValue.Dragging); + + // Let start-time collision effects populate the initial target before + // dragstart, while keeping dragover ordered after dragstart. + queueMicrotask(() => { + try { + if ( + controller.signal.aborted || + dragOperation.status.current !== StatusValue.Dragging + ) { + this.#resolveDeferredDragOverEvents(false); + return; + } + + this.manager.monitor.dispatch('dragstart', { + nativeEvent, + operation: dragOperation.snapshot(), + cancelable: false, + }); + + if ( + controller.signal.aborted || + dragOperation.status.current !== StatusValue.Dragging + ) { + this.#resolveDeferredDragOverEvents(false); + return; + } + + this.#flushDeferredDragOverEvents(); + } catch (error) { + this.#rejectDeferredDragOverEvents(error); + throw error; + } }); }); diff --git a/packages/abstract/tests/drag-event-order.test.ts b/packages/abstract/tests/drag-event-order.test.ts index 8e326b649..ba2591516 100644 --- a/packages/abstract/tests/drag-event-order.test.ts +++ b/packages/abstract/tests/drag-event-order.test.ts @@ -14,71 +14,216 @@ function flush() { return new Promise((resolve) => setTimeout(resolve, 10)); } +function setupCollidingDraggable( + manager: DragDropManager +) { + // Create draggable and droppable with the same id (as in the reported bug) + const draggable = new Draggable({id: 'item', register: false}, manager); + draggable.register(); + + const droppable = new Droppable( + { + id: 'item', + register: false, + collisionDetector: ({droppable}) => ({ + id: droppable.id, + value: 1, + type: CollisionType.ShapeIntersection, + priority: CollisionPriority.Normal, + }), + }, + manager + ); + droppable.shape = new Rectangle(0, 0, 100, 100); + droppable.register(); + + // Mimic the Feedback plugin's behavior: it only sets the drag shape + // when status is initialized AND not initializing (i.e., when Dragging). + // See: packages/dom/src/core/plugins/feedback/Feedback.ts lines 136-143 + const dispose = effects(() => { + const {status} = manager.dragOperation; + + if (status.initialized && !status.initializing) { + manager.dragOperation.shape = new Rectangle(0, 0, 100, 100); + } + }); + + return {dispose, draggable, droppable}; +} + describe('Drag event ordering', () => { it('should fire dragstart before dragover when element is both draggable and droppable', async () => { const manager = new DragDropManager(); const events: string[] = []; + let dragStartTarget: string | number | null | undefined; + let dragOverTarget: string | number | null | undefined; // Listen for all drag events manager.monitor.addEventListener('beforedragstart', () => { events.push('beforedragstart'); }); - manager.monitor.addEventListener('dragstart', () => { + manager.monitor.addEventListener('dragstart', (event) => { + events.push('dragstart'); + dragStartTarget = event.operation.target?.id; + }); + manager.monitor.addEventListener('dragover', (event) => { + events.push('dragover'); + dragOverTarget = event.operation.target?.id; + }); + + const {dispose, draggable, droppable} = setupCollidingDraggable(manager); + + // Start drag operation + manager.actions.start({ + source: draggable, + coordinates: {x: 50, y: 50}, + }); + + // Wait for renderer.rendering promise to resolve + await flush(); + + // dragstart must come before dragover + const dragStartIndex = events.indexOf('dragstart'); + const dragOverIndex = events.indexOf('dragover'); + + expect(dragStartIndex).not.toBe(-1); + expect(dragOverIndex).not.toBe(-1); + expect(dragStartIndex).toBeLessThan(dragOverIndex); + expect(dragStartTarget).toBe('item'); + expect(dragOverTarget).toBe('item'); + + // Clean up + manager.actions.stop(); + await flush(); + dispose(); + draggable.destroy(); + droppable.destroy(); + manager.destroy(); + }); + + it('should not fire queued dragover when dragstart stops the operation', async () => { + const manager = new DragDropManager(); + const events: string[] = []; + let dragStartTarget: string | number | null | undefined; + + manager.monitor.addEventListener('dragstart', (event) => { + events.push('dragstart'); + dragStartTarget = event.operation.target?.id; + manager.actions.stop(); + }); + manager.monitor.addEventListener('dragover', () => { + events.push('dragover'); + }); + + const {dispose, draggable, droppable} = setupCollidingDraggable(manager); + + manager.actions.start({ + source: draggable, + coordinates: {x: 50, y: 50}, + }); + + await flush(); + + expect(dragStartTarget).toBe('item'); + expect(events).toEqual(['dragstart']); + + await flush(); + dispose(); + draggable.destroy(); + droppable.destroy(); + manager.destroy(); + }); + + it('should start without a target when there is no initial droppable', async () => { + const manager = new DragDropManager(); + const events: string[] = []; + let dragStartTarget: Droppable | null | undefined; + + manager.monitor.addEventListener('dragstart', (event) => { events.push('dragstart'); + dragStartTarget = event.operation.target; }); manager.monitor.addEventListener('dragover', () => { events.push('dragover'); }); - // Create draggable and droppable with the same id (as in the reported bug) + const draggable = new Draggable({id: 'item', register: false}, manager); + draggable.register(); + + manager.actions.start({ + source: draggable, + coordinates: {x: 50, y: 50}, + }); + + await flush(); + + expect(events).toEqual(['dragstart']); + expect(dragStartTarget).toBeNull(); + + manager.actions.stop(); + await flush(); + draggable.destroy(); + manager.destroy(); + }); + + it('should resolve queued setDropTarget after dragover dispatches', async () => { + const events: string[] = []; + const renderingSnapshots: string[] = []; + const manager = new DragDropManager({ + renderer: { + get rendering() { + renderingSnapshots.push(events.join('|')); + return Promise.resolve(); + }, + }, + }); + let targetPromise: Promise | undefined; + let targetPromiseResolved = false; + const draggable = new Draggable({id: 'item', register: false}, manager); draggable.register(); const droppable = new Droppable( { - id: 'item', + id: 'target', register: false, - collisionDetector: ({droppable}) => ({ - id: droppable.id, - value: 1, - type: CollisionType.ShapeIntersection, - priority: CollisionPriority.Normal, - }), + collisionDetector: () => null, }, manager ); - droppable.shape = new Rectangle(0, 0, 100, 100); droppable.register(); - // Mimic the Feedback plugin's behavior: it only sets the drag shape - // when status is initialized AND not initializing (i.e., when Dragging). - // See: packages/dom/src/core/plugins/feedback/Feedback.ts lines 136-143 const dispose = effects(() => { - const {status} = manager.dragOperation; - - if (status.initialized && !status.initializing) { - manager.dragOperation.shape = new Rectangle(0, 0, 100, 100); + if (manager.dragOperation.status.dragging && !targetPromise) { + targetPromise = manager.actions.setDropTarget(droppable.id); + targetPromise.then(() => { + targetPromiseResolved = true; + }); } }); - // Start drag operation + manager.monitor.addEventListener('dragstart', () => { + events.push('dragstart'); + }); + manager.monitor.addEventListener('dragover', (event) => { + events.push('dragover'); + expect(targetPromiseResolved).toBe(false); + event.preventDefault(); + }); + manager.actions.start({ source: draggable, coordinates: {x: 50, y: 50}, }); - // Wait for renderer.rendering promise to resolve await flush(); - // dragstart must come before dragover - const dragStartIndex = events.indexOf('dragstart'); - const dragOverIndex = events.indexOf('dragover'); - - expect(dragStartIndex).not.toBe(-1); - expect(dragOverIndex).not.toBe(-1); - expect(dragStartIndex).toBeLessThan(dragOverIndex); + expect(targetPromise).toBeDefined(); + expect(events).toEqual(['dragstart', 'dragover']); + expect(renderingSnapshots[1]).toBe('dragstart|dragover'); + expect(await targetPromise!).toBe(true); + expect(targetPromiseResolved).toBe(true); - // Clean up manager.actions.stop(); await flush(); dispose();