Skip to content

Commit e59c302

Browse files
author
Antoine Boisier-Michaud
committed
fix(drag-drop): fix drag start delay behavior to allow scrolling (#16224)
The current implementation of the drag start delay does not allow scrolling on iOS devices. Also, once the element has been scrolled once, the scrolling is never reenabled. In order to fix this issue, we prevent default behaviour of pointer events fired after the drag sequence has been started instead of initialized. Also, we toggle native drag interactions when the drag sequence is ended.
1 parent dde12c2 commit e59c302

File tree

5 files changed

+81
-26
lines changed

5 files changed

+81
-26
lines changed

src/cdk/drag-drop/directives/drag.spec.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -636,6 +636,24 @@ describe('CdkDrag', () => {
636636
expect(styles.touchAction || (styles as any).webkitUserDrag).toBe('none');
637637
}));
638638

639+
it('should reenable native drag interactions after dragging', fakeAsync(() => {
640+
const fixture = createComponent(StandaloneDraggable);
641+
fixture.detectChanges();
642+
const dragElement = fixture.componentInstance.dragElement.nativeElement;
643+
const styles = dragElement.style;
644+
645+
expect(styles.touchAction || (styles as any).webkitUserDrag).toBeFalsy();
646+
647+
startDraggingViaMouse(fixture, dragElement);
648+
dispatchMouseEvent(document, 'mousemove', 50, 100);
649+
fixture.detectChanges();
650+
expect(styles.touchAction || (styles as any).webkitUserDrag).toBe('none');
651+
652+
dispatchMouseEvent(document, 'mouseup', 50, 100);
653+
fixture.detectChanges();
654+
expect(styles.touchAction || (styles as any).webkitUserDrag).toBeFalsy();
655+
}));
656+
639657
it('should stop propagation for the drag sequence start event', fakeAsync(() => {
640658
const fixture = createComponent(StandaloneDraggable);
641659
fixture.detectChanges();

src/cdk/drag-drop/drag-drop-registry.spec.ts

Lines changed: 29 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,14 @@ describe('DragDropRegistry', () => {
4040
const firstItem = testComponent.dragItems.first;
4141

4242
expect(registry.isDragging(firstItem)).toBe(false);
43-
registry.startDragging(firstItem, createMouseEvent('mousedown'));
43+
registry.initializeDragging(firstItem, createMouseEvent('mousedown'));
4444
expect(registry.isDragging(firstItem)).toBe(true);
4545
});
4646

4747
it('should be able to stop dragging an item', () => {
4848
const firstItem = testComponent.dragItems.first;
4949

50-
registry.startDragging(firstItem, createMouseEvent('mousedown'));
50+
registry.initializeDragging(firstItem, createMouseEvent('mousedown'));
5151
expect(registry.isDragging(firstItem)).toBe(true);
5252

5353
registry.stopDragging(firstItem);
@@ -57,7 +57,7 @@ describe('DragDropRegistry', () => {
5757
it('should stop dragging an item if it is removed', () => {
5858
const firstItem = testComponent.dragItems.first;
5959

60-
registry.startDragging(firstItem, createMouseEvent('mousedown'));
60+
registry.initializeDragging(firstItem, createMouseEvent('mousedown'));
6161
expect(registry.isDragging(firstItem)).toBe(true);
6262

6363
registry.removeDragItem(firstItem);
@@ -68,7 +68,7 @@ describe('DragDropRegistry', () => {
6868
const spy = jasmine.createSpy('pointerMove spy');
6969
const subscription = registry.pointerMove.subscribe(spy);
7070

71-
registry.startDragging(testComponent.dragItems.first, createMouseEvent('mousedown'));
71+
registry.initializeDragging(testComponent.dragItems.first, createMouseEvent('mousedown'));
7272
dispatchMouseEvent(document, 'mousemove');
7373

7474
expect(spy).toHaveBeenCalled();
@@ -80,7 +80,7 @@ describe('DragDropRegistry', () => {
8080
const spy = jasmine.createSpy('pointerMove spy');
8181
const subscription = registry.pointerMove.subscribe(spy);
8282

83-
registry.startDragging(testComponent.dragItems.first,
83+
registry.initializeDragging(testComponent.dragItems.first,
8484
createTouchEvent('touchstart') as TouchEvent);
8585
dispatchTouchEvent(document, 'touchmove');
8686

@@ -94,7 +94,7 @@ describe('DragDropRegistry', () => {
9494
const subscription = registry.pointerMove.subscribe(spy);
9595

9696
fixture.nativeElement.addEventListener('mousemove', (e: MouseEvent) => e.stopPropagation());
97-
registry.startDragging(testComponent.dragItems.first, createMouseEvent('mousedown'));
97+
registry.initializeDragging(testComponent.dragItems.first, createMouseEvent('mousedown'));
9898
dispatchMouseEvent(fixture.nativeElement.querySelector('div'), 'mousemove');
9999

100100
expect(spy).toHaveBeenCalled();
@@ -106,7 +106,7 @@ describe('DragDropRegistry', () => {
106106
const spy = jasmine.createSpy('pointerUp spy');
107107
const subscription = registry.pointerUp.subscribe(spy);
108108

109-
registry.startDragging(testComponent.dragItems.first, createMouseEvent('mousedown'));
109+
registry.initializeDragging(testComponent.dragItems.first, createMouseEvent('mousedown'));
110110
dispatchMouseEvent(document, 'mouseup');
111111

112112
expect(spy).toHaveBeenCalled();
@@ -118,7 +118,7 @@ describe('DragDropRegistry', () => {
118118
const spy = jasmine.createSpy('pointerUp spy');
119119
const subscription = registry.pointerUp.subscribe(spy);
120120

121-
registry.startDragging(testComponent.dragItems.first,
121+
registry.initializeDragging(testComponent.dragItems.first,
122122
createTouchEvent('touchstart') as TouchEvent);
123123
dispatchTouchEvent(document, 'touchend');
124124

@@ -132,7 +132,7 @@ describe('DragDropRegistry', () => {
132132
const subscription = registry.pointerUp.subscribe(spy);
133133

134134
fixture.nativeElement.addEventListener('mouseup', (e: MouseEvent) => e.stopPropagation());
135-
registry.startDragging(testComponent.dragItems.first, createMouseEvent('mousedown'));
135+
registry.initializeDragging(testComponent.dragItems.first, createMouseEvent('mousedown'));
136136
dispatchMouseEvent(fixture.nativeElement.querySelector('div'), 'mouseup');
137137

138138
expect(spy).toHaveBeenCalled();
@@ -159,9 +159,9 @@ describe('DragDropRegistry', () => {
159159
const firstItem = testComponent.dragItems.first;
160160

161161
// First finger down
162-
registry.startDragging(firstItem, createTouchEvent('touchstart') as TouchEvent);
162+
registry.initializeDragging(firstItem, createTouchEvent('touchstart') as TouchEvent);
163163
// Second finger down
164-
registry.startDragging(firstItem, createTouchEvent('touchstart') as TouchEvent);
164+
registry.initializeDragging(firstItem, createTouchEvent('touchstart') as TouchEvent);
165165
// First finger up
166166
registry.stopDragging(firstItem);
167167

@@ -194,15 +194,23 @@ describe('DragDropRegistry', () => {
194194
expect(dispatchTouchEvent(document, 'touchmove').defaultPrevented).toBe(false);
195195
});
196196

197+
it('should not revent the default `touchmove` action when an item is only initialized', () => {
198+
registry.initializeDragging(testComponent.dragItems.first,
199+
createTouchEvent('touchstart') as TouchEvent);
200+
expect(dispatchTouchEvent(document, 'touchmove').defaultPrevented).toBe(false);
201+
});
202+
197203
it('should prevent the default `touchmove` action when an item is being dragged', () => {
198-
registry.startDragging(testComponent.dragItems.first,
204+
registry.initializeDragging(testComponent.dragItems.first,
199205
createTouchEvent('touchstart') as TouchEvent);
206+
registry.startDragging(testComponent.dragItems.first);
200207
expect(dispatchTouchEvent(document, 'touchmove').defaultPrevented).toBe(true);
201208
});
202209

203210
it('should prevent the default `touchmove` if event propagation is stopped', () => {
204-
registry.startDragging(testComponent.dragItems.first,
211+
registry.initializeDragging(testComponent.dragItems.first,
205212
createTouchEvent('touchstart') as TouchEvent);
213+
registry.startDragging(testComponent.dragItems.first);
206214

207215
fixture.nativeElement.addEventListener('touchmove', (e: TouchEvent) => e.stopPropagation());
208216

@@ -215,16 +223,22 @@ describe('DragDropRegistry', () => {
215223
expect(dispatchFakeEvent(document, 'selectstart').defaultPrevented).toBe(false);
216224
});
217225

226+
it('should not prevent the default `selectstart` action when an item is initialized', () => {
227+
registry.initializeDragging(testComponent.dragItems.first, createMouseEvent('mousedown'));
228+
expect(dispatchFakeEvent(document, 'selectstart').defaultPrevented).toBe(false);
229+
});
230+
218231
it('should prevent the default `selectstart` action when an item is being dragged', () => {
219-
registry.startDragging(testComponent.dragItems.first, createMouseEvent('mousedown'));
232+
registry.initializeDragging(testComponent.dragItems.first, createMouseEvent('mousedown'));
233+
registry.startDragging(testComponent.dragItems.first);
220234
expect(dispatchFakeEvent(document, 'selectstart').defaultPrevented).toBe(true);
221235
});
222236

223237
it('should dispatch `scroll` events if the viewport is scrolled while dragging', () => {
224238
const spy = jasmine.createSpy('scroll spy');
225239
const subscription = registry.scroll.subscribe(spy);
226240

227-
registry.startDragging(testComponent.dragItems.first, createMouseEvent('mousedown'));
241+
registry.initializeDragging(testComponent.dragItems.first, createMouseEvent('mousedown'));
228242
dispatchFakeEvent(document, 'scroll');
229243

230244
expect(spy).toHaveBeenCalled();

src/cdk/drag-drop/drag-drop-registry.ts

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,11 @@ export class DragDropRegistry<I, C extends {id: string}> implements OnDestroy {
3535
/** Registered drag item instances. */
3636
private _dragInstances = new Set<I>();
3737

38+
/** Drag item instances for which a drag sequence has been initialized. */
39+
private _initializedDragSequences = new Set<I>();
40+
3841
/** Drag item instances that are currently being dragged. */
39-
private _activeDragInstances = new Set<I>();
42+
private _startedDragSequences = new Set<I>();
4043

4144
/** Keeps track of the event listeners that we've bound to the `document`. */
4245
private _globalListeners = new Map<string, {
@@ -114,15 +117,15 @@ export class DragDropRegistry<I, C extends {id: string}> implements OnDestroy {
114117
* @param drag Drag instance which is being dragged.
115118
* @param event Event that initiated the dragging.
116119
*/
117-
startDragging(drag: I, event: TouchEvent | MouseEvent) {
120+
initializeDragging(drag: I, event: TouchEvent | MouseEvent) {
118121
// Do not process the same drag twice to avoid memory leaks and redundant listeners
119-
if (this._activeDragInstances.has(drag)) {
122+
if (this._initializedDragSequences.has(drag)) {
120123
return;
121124
}
122125

123-
this._activeDragInstances.add(drag);
126+
this._initializedDragSequences.add(drag);
124127

125-
if (this._activeDragInstances.size === 1) {
128+
if (this._initializedDragSequences.size === 1) {
126129
const isTouchEvent = event.type.startsWith('touch');
127130
const moveEvent = isTouchEvent ? 'touchmove' : 'mousemove';
128131
const upEvent = isTouchEvent ? 'touchend' : 'mouseup';
@@ -159,18 +162,28 @@ export class DragDropRegistry<I, C extends {id: string}> implements OnDestroy {
159162
}
160163
}
161164

165+
startDragging(drag: I) {
166+
if (this._startedDragSequences.has(drag)) {
167+
return;
168+
}
169+
170+
this._startedDragSequences.add(drag);
171+
}
172+
162173
/** Stops dragging a drag item instance. */
163174
stopDragging(drag: I) {
164-
this._activeDragInstances.delete(drag);
165175

166-
if (this._activeDragInstances.size === 0) {
176+
this._startedDragSequences.delete(drag);
177+
this._initializedDragSequences.delete(drag);
178+
179+
if (this._initializedDragSequences.size === 0) {
167180
this._clearGlobalListeners();
168181
}
169182
}
170183

171184
/** Gets whether a drag item instance is currently being dragged. */
172185
isDragging(drag: I) {
173-
return this._activeDragInstances.has(drag);
186+
return this._initializedDragSequences.has(drag);
174187
}
175188

176189
/**
@@ -195,7 +208,7 @@ export class DragDropRegistry<I, C extends {id: string}> implements OnDestroy {
195208
* @param event Event whose default action should be prevented.
196209
*/
197210
private _preventDefaultWhileDragging = (event: Event) => {
198-
if (this._activeDragInstances.size) {
211+
if (this._startedDragSequences.size) {
199212
event.preventDefault();
200213
}
201214
}

src/cdk/drag-drop/drag-ref.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -530,6 +530,11 @@ export class DragRef<T = any> {
530530
return;
531531
}
532532

533+
// We need to prevent default here in case the pointer move starts a scroll sequence.
534+
// If we do not prevent the scroll from starting, then we won't be able to prevent future
535+
// touchemove events.
536+
event.preventDefault();
537+
533538
// Prevent other drag sequences from starting while something in the container is still
534539
// being dragged. This can happen while we're waiting for the drop animation to finish
535540
// and can cause errors, because some elements might still be moving around.
@@ -600,6 +605,8 @@ export class DragRef<T = any> {
600605
* @param event Browser event object that ended the sequence.
601606
*/
602607
private _endDragSequence(event: MouseEvent | TouchEvent) {
608+
toggleNativeDragInteractions(this._rootElement, true);
609+
603610
// Note that here we use `isDragging` from the service, rather than from `this`.
604611
// The difference is that the one from the service reflects whether a dragging sequence
605612
// has been initiated, whereas the one on `this` includes whether the user has passed
@@ -646,6 +653,8 @@ export class DragRef<T = any> {
646653

647654
/** Starts the dragging sequence. */
648655
private _startDragSequence(event: MouseEvent | TouchEvent) {
656+
this._dragDropRegistry.startDragging(this);
657+
649658
// Emit the event on the item before the one on the container.
650659
this.started.next({source: this});
651660

@@ -742,7 +751,7 @@ export class DragRef<T = any> {
742751
this._pointerDirectionDelta = {x: 0, y: 0};
743752
this._pointerPositionAtLastDirectionChange = {x: pointerPosition.x, y: pointerPosition.y};
744753
this._dragStartTime = Date.now();
745-
this._dragDropRegistry.startDragging(this, event);
754+
this._dragDropRegistry.initializeDragging(this, event);
746755
}
747756

748757
/** Cleans up the DOM artifacts that were added to facilitate the element being dragged. */

tools/public_api_guard/cdk/drag-drop.d.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,13 +215,14 @@ export declare class DragDropRegistry<I, C extends {
215215
readonly scroll: Subject<Event>;
216216
constructor(_ngZone: NgZone, _document: any);
217217
getDropContainer(id: string): C | undefined;
218+
initializeDragging(drag: I, event: TouchEvent | MouseEvent): void;
218219
isDragging(drag: I): boolean;
219220
ngOnDestroy(): void;
220221
registerDragItem(drag: I): void;
221222
registerDropContainer(drop: C): void;
222223
removeDragItem(drag: I): void;
223224
removeDropContainer(drop: C): void;
224-
startDragging(drag: I, event: TouchEvent | MouseEvent): void;
225+
startDragging(drag: I): void;
225226
stopDragging(drag: I): void;
226227
}
227228

0 commit comments

Comments
 (0)