Skip to content

Commit 4134580

Browse files
crisbetoannieyw
authored andcommitted
fix(cdk/drag-drop): don't block scrolling if it happens before delay has elapsed (#21382)
The `DragRef` has the ability to add a delay to the dragging so that scrolling isn't blocked on touch devices. Currently this doesn't really work, because we always block the native event as a result of a949db3. These changes move some code around so that we only block the events if the item considers itself as being in a "dragged" state (the delay has elapsed and the user is past the drag threshold). Fixes #17923. (cherry picked from commit eb318d9)
1 parent 175d4e4 commit 4134580

File tree

5 files changed

+128
-111
lines changed

5 files changed

+128
-111
lines changed

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

+19-28
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,7 @@ describe('CdkDrag', () => {
318318
dispatchTouchEvent(fixture.componentInstance.dragElement.nativeElement, 'touchstart');
319319
fixture.detectChanges();
320320

321+
dispatchTouchEvent(document, 'touchmove');
321322
expect(dispatchTouchEvent(document, 'touchmove').defaultPrevented).toBe(true);
322323

323324
dispatchTouchEvent(document, 'touchend');
@@ -1059,6 +1060,24 @@ describe('CdkDrag', () => {
10591060
'Expected element to be dragged after all the time has passed.');
10601061
}));
10611062

1063+
it('should not prevent the default touch action before the delay has elapsed', fakeAsync(() => {
1064+
spyOn(Date, 'now').and.callFake(() => currentTime);
1065+
let currentTime = 0;
1066+
1067+
const fixture = createComponent(StandaloneDraggable);
1068+
fixture.componentInstance.dragStartDelay = 500;
1069+
fixture.detectChanges();
1070+
const dragElement = fixture.componentInstance.dragElement.nativeElement;
1071+
1072+
expect(dragElement.style.transform).toBeFalsy('Expected element not to be moved by default.');
1073+
1074+
dispatchTouchEvent(dragElement, 'touchstart');
1075+
fixture.detectChanges();
1076+
currentTime += 250;
1077+
1078+
expect(dispatchTouchEvent(document, 'touchmove', 50, 100).defaultPrevented).toBe(false);
1079+
}));
1080+
10621081
it('should handle the drag delay as a string', fakeAsync(() => {
10631082
// We can't use Jasmine's `clock` because Zone.js interferes with it.
10641083
spyOn(Date, 'now').and.callFake(() => currentTime);
@@ -1206,34 +1225,6 @@ describe('CdkDrag', () => {
12061225
subscription.unsubscribe();
12071226
}));
12081227

1209-
it('should prevent the default `mousemove` action even before the drag threshold has ' +
1210-
'been reached', fakeAsync(() => {
1211-
const fixture = createComponent(StandaloneDraggable, [], 5);
1212-
fixture.detectChanges();
1213-
const dragElement = fixture.componentInstance.dragElement.nativeElement;
1214-
1215-
dispatchMouseEvent(dragElement, 'mousedown', 2, 2);
1216-
fixture.detectChanges();
1217-
const mousemoveEvent = dispatchMouseEvent(document, 'mousemove', 2, 2);
1218-
fixture.detectChanges();
1219-
1220-
expect(mousemoveEvent.defaultPrevented).toBe(true);
1221-
}));
1222-
1223-
it('should prevent the default `touchmove` action even before the drag threshold has ' +
1224-
'been reached', fakeAsync(() => {
1225-
const fixture = createComponent(StandaloneDraggable, [], 5);
1226-
fixture.detectChanges();
1227-
const dragElement = fixture.componentInstance.dragElement.nativeElement;
1228-
1229-
dispatchTouchEvent(dragElement, 'touchstart', 2, 2);
1230-
fixture.detectChanges();
1231-
const touchmoveEvent = dispatchTouchEvent(document, 'touchmove', 2, 2);
1232-
fixture.detectChanges();
1233-
1234-
expect(touchmoveEvent.defaultPrevented).toBe(true);
1235-
}));
1236-
12371228
it('should be able to configure the drag input defaults through a provider', fakeAsync(() => {
12381229
const config: DragDropConfig = {
12391230
draggingDisabled: true,

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

+73-67
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {QueryList, ViewChildren, Component} from '@angular/core';
1+
import {Component} from '@angular/core';
22
import {fakeAsync, TestBed, ComponentFixture, inject} from '@angular/core/testing';
33
import {
44
createMouseEvent,
@@ -9,25 +9,21 @@ import {
99
} from '@angular/cdk/testing/private';
1010
import {DragDropRegistry} from './drag-drop-registry';
1111
import {DragDropModule} from './drag-drop-module';
12-
import {CdkDrag} from './directives/drag';
13-
import {CdkDropList} from './directives/drop-list';
1412

1513
describe('DragDropRegistry', () => {
16-
let fixture: ComponentFixture<SimpleDropZone>;
17-
let testComponent: SimpleDropZone;
18-
let registry: DragDropRegistry<CdkDrag, CdkDropList>;
14+
let fixture: ComponentFixture<BlankComponent>;
15+
let registry: DragDropRegistry<DragItem, DragList>;
1916

2017
beforeEach(fakeAsync(() => {
2118
TestBed.configureTestingModule({
2219
imports: [DragDropModule],
23-
declarations: [SimpleDropZone],
20+
declarations: [BlankComponent],
2421
}).compileComponents();
2522

26-
fixture = TestBed.createComponent(SimpleDropZone);
27-
testComponent = fixture.componentInstance;
23+
fixture = TestBed.createComponent(BlankComponent);
2824
fixture.detectChanges();
2925

30-
inject([DragDropRegistry], (c: DragDropRegistry<CdkDrag, CdkDropList>) => {
26+
inject([DragDropRegistry], (c: DragDropRegistry<DragItem, DragList>) => {
3127
registry = c;
3228
})();
3329
}));
@@ -37,38 +33,38 @@ describe('DragDropRegistry', () => {
3733
});
3834

3935
it('should be able to start dragging an item', () => {
40-
const firstItem = testComponent.dragItems.first;
36+
const item = new DragItem();
4137

42-
expect(registry.isDragging(firstItem)).toBe(false);
43-
registry.startDragging(firstItem, createMouseEvent('mousedown'));
44-
expect(registry.isDragging(firstItem)).toBe(true);
38+
expect(registry.isDragging(item)).toBe(false);
39+
registry.startDragging(item, createMouseEvent('mousedown'));
40+
expect(registry.isDragging(item)).toBe(true);
4541
});
4642

4743
it('should be able to stop dragging an item', () => {
48-
const firstItem = testComponent.dragItems.first;
44+
const item = new DragItem();
4945

50-
registry.startDragging(firstItem, createMouseEvent('mousedown'));
51-
expect(registry.isDragging(firstItem)).toBe(true);
46+
registry.startDragging(item, createMouseEvent('mousedown'));
47+
expect(registry.isDragging(item)).toBe(true);
5248

53-
registry.stopDragging(firstItem);
54-
expect(registry.isDragging(firstItem)).toBe(false);
49+
registry.stopDragging(item);
50+
expect(registry.isDragging(item)).toBe(false);
5551
});
5652

5753
it('should stop dragging an item if it is removed', () => {
58-
const firstItem = testComponent.dragItems.first;
54+
const item = new DragItem();
5955

60-
registry.startDragging(firstItem, createMouseEvent('mousedown'));
61-
expect(registry.isDragging(firstItem)).toBe(true);
56+
registry.startDragging(item, createMouseEvent('mousedown'));
57+
expect(registry.isDragging(item)).toBe(true);
6258

63-
registry.removeDragItem(firstItem);
64-
expect(registry.isDragging(firstItem)).toBe(false);
59+
registry.removeDragItem(item);
60+
expect(registry.isDragging(item)).toBe(false);
6561
});
6662

6763
it('should dispatch `mousemove` events after starting to drag via the mouse', () => {
6864
const spy = jasmine.createSpy('pointerMove spy');
6965
const subscription = registry.pointerMove.subscribe(spy);
70-
71-
registry.startDragging(testComponent.dragItems.first, createMouseEvent('mousedown'));
66+
const item = new DragItem(true);
67+
registry.startDragging(item, createMouseEvent('mousedown'));
7268
dispatchMouseEvent(document, 'mousemove');
7369

7470
expect(spy).toHaveBeenCalled();
@@ -79,9 +75,8 @@ describe('DragDropRegistry', () => {
7975
it('should dispatch `touchmove` events after starting to drag via touch', () => {
8076
const spy = jasmine.createSpy('pointerMove spy');
8177
const subscription = registry.pointerMove.subscribe(spy);
82-
83-
registry.startDragging(testComponent.dragItems.first,
84-
createTouchEvent('touchstart') as TouchEvent);
78+
const item = new DragItem(true);
79+
registry.startDragging(item, createTouchEvent('touchstart') as TouchEvent);
8580
dispatchTouchEvent(document, 'touchmove');
8681

8782
expect(spy).toHaveBeenCalled();
@@ -92,10 +87,10 @@ describe('DragDropRegistry', () => {
9287
it('should dispatch pointer move events if event propagation is stopped', () => {
9388
const spy = jasmine.createSpy('pointerMove spy');
9489
const subscription = registry.pointerMove.subscribe(spy);
95-
90+
const item = new DragItem(true);
9691
fixture.nativeElement.addEventListener('mousemove', (e: MouseEvent) => e.stopPropagation());
97-
registry.startDragging(testComponent.dragItems.first, createMouseEvent('mousedown'));
98-
dispatchMouseEvent(fixture.nativeElement.querySelector('div'), 'mousemove');
92+
registry.startDragging(item, createMouseEvent('mousedown'));
93+
dispatchMouseEvent(fixture.nativeElement, 'mousemove');
9994

10095
expect(spy).toHaveBeenCalled();
10196

@@ -105,8 +100,9 @@ describe('DragDropRegistry', () => {
105100
it('should dispatch `mouseup` events after ending the drag via the mouse', () => {
106101
const spy = jasmine.createSpy('pointerUp spy');
107102
const subscription = registry.pointerUp.subscribe(spy);
103+
const item = new DragItem();
108104

109-
registry.startDragging(testComponent.dragItems.first, createMouseEvent('mousedown'));
105+
registry.startDragging(item, createMouseEvent('mousedown'));
110106
dispatchMouseEvent(document, 'mouseup');
111107

112108
expect(spy).toHaveBeenCalled();
@@ -117,9 +113,9 @@ describe('DragDropRegistry', () => {
117113
it('should dispatch `touchend` events after ending the drag via touch', () => {
118114
const spy = jasmine.createSpy('pointerUp spy');
119115
const subscription = registry.pointerUp.subscribe(spy);
116+
const item = new DragItem();
120117

121-
registry.startDragging(testComponent.dragItems.first,
122-
createTouchEvent('touchstart') as TouchEvent);
118+
registry.startDragging(item, createTouchEvent('touchstart') as TouchEvent);
123119
dispatchTouchEvent(document, 'touchend');
124120

125121
expect(spy).toHaveBeenCalled();
@@ -130,10 +126,11 @@ describe('DragDropRegistry', () => {
130126
it('should dispatch pointer up events if event propagation is stopped', () => {
131127
const spy = jasmine.createSpy('pointerUp spy');
132128
const subscription = registry.pointerUp.subscribe(spy);
129+
const item = new DragItem();
133130

134131
fixture.nativeElement.addEventListener('mouseup', (e: MouseEvent) => e.stopPropagation());
135-
registry.startDragging(testComponent.dragItems.first, createMouseEvent('mousedown'));
136-
dispatchMouseEvent(fixture.nativeElement.querySelector('div'), 'mouseup');
132+
registry.startDragging(item, createMouseEvent('mousedown'));
133+
dispatchMouseEvent(fixture.nativeElement, 'mouseup');
137134

138135
expect(spy).toHaveBeenCalled();
139136

@@ -156,17 +153,17 @@ describe('DragDropRegistry', () => {
156153
});
157154

158155
it('should not emit pointer events when dragging is over (multi touch)', () => {
159-
const firstItem = testComponent.dragItems.first;
156+
const item = new DragItem();
160157

161158
// First finger down
162-
registry.startDragging(firstItem, createTouchEvent('touchstart') as TouchEvent);
159+
registry.startDragging(item, createTouchEvent('touchstart') as TouchEvent);
163160
// Second finger down
164-
registry.startDragging(firstItem, createTouchEvent('touchstart') as TouchEvent);
161+
registry.startDragging(item, createTouchEvent('touchstart') as TouchEvent);
165162
// First finger up
166-
registry.stopDragging(firstItem);
163+
registry.stopDragging(item);
167164

168165
// Ensure dragging is over - registry is empty
169-
expect(registry.isDragging(firstItem)).toBe(false);
166+
expect(registry.isDragging(item)).toBe(false);
170167

171168
const pointerUpSpy = jasmine.createSpy('pointerUp spy');
172169
const pointerMoveSpy = jasmine.createSpy('pointerMove spy');
@@ -191,19 +188,27 @@ describe('DragDropRegistry', () => {
191188
});
192189

193190
it('should prevent the default `touchmove` action when an item is being dragged', () => {
194-
registry.startDragging(testComponent.dragItems.first,
195-
createTouchEvent('touchstart') as TouchEvent);
191+
const item = new DragItem(true);
192+
registry.startDragging(item, createTouchEvent('touchstart') as TouchEvent);
196193
expect(dispatchTouchEvent(document, 'touchmove').defaultPrevented).toBe(true);
197194
});
198195

199-
it('should prevent the default `touchmove` if event propagation is stopped', () => {
200-
registry.startDragging(testComponent.dragItems.first,
201-
createTouchEvent('touchstart') as TouchEvent);
196+
it('should prevent the default `touchmove` if the item does not consider itself as being ' +
197+
'dragged yet', () => {
198+
const item = new DragItem(false);
199+
registry.startDragging(item, createTouchEvent('touchstart') as TouchEvent);
200+
expect(dispatchTouchEvent(document, 'touchmove').defaultPrevented).toBe(false);
202201

203-
fixture.nativeElement.addEventListener('touchmove', (e: TouchEvent) => e.stopPropagation());
202+
item.shouldBeDragging = true;
203+
expect(dispatchTouchEvent(document, 'touchmove').defaultPrevented).toBe(true);
204+
});
204205

205-
const event = dispatchTouchEvent(fixture.nativeElement.querySelector('div'), 'touchmove');
206+
it('should prevent the default `touchmove` if event propagation is stopped', () => {
207+
const item = new DragItem(true);
208+
registry.startDragging(item, createTouchEvent('touchstart') as TouchEvent);
209+
fixture.nativeElement.addEventListener('touchmove', (e: TouchEvent) => e.stopPropagation());
206210

211+
const event = dispatchTouchEvent(fixture.nativeElement, 'touchmove');
207212
expect(event.defaultPrevented).toBe(true);
208213
});
209214

@@ -212,15 +217,17 @@ describe('DragDropRegistry', () => {
212217
});
213218

214219
it('should prevent the default `selectstart` action when an item is being dragged', () => {
215-
registry.startDragging(testComponent.dragItems.first, createMouseEvent('mousedown'));
220+
const item = new DragItem(true);
221+
registry.startDragging(item, createMouseEvent('mousedown'));
216222
expect(dispatchFakeEvent(document, 'selectstart').defaultPrevented).toBe(true);
217223
});
218224

219225
it('should dispatch `scroll` events if the viewport is scrolled while dragging', () => {
220226
const spy = jasmine.createSpy('scroll spy');
221227
const subscription = registry.scroll.subscribe(spy);
228+
const item = new DragItem();
222229

223-
registry.startDragging(testComponent.dragItems.first, createMouseEvent('mousedown'));
230+
registry.startDragging(item, createMouseEvent('mousedown'));
224231
dispatchFakeEvent(document, 'scroll');
225232

226233
expect(spy).toHaveBeenCalled();
@@ -237,20 +244,19 @@ describe('DragDropRegistry', () => {
237244
subscription.unsubscribe();
238245
});
239246

247+
class DragItem {
248+
isDragging() { return this.shouldBeDragging; }
249+
constructor(public shouldBeDragging = false) {
250+
registry.registerDragItem(this);
251+
}
252+
}
253+
254+
class DragList {
255+
constructor() {
256+
registry.registerDropContainer(this);
257+
}
258+
}
259+
260+
@Component({template: ``})
261+
class BlankComponent {}
240262
});
241-
242-
@Component({
243-
template: `
244-
<div cdkDropList id="items" [cdkDropListData]="items">
245-
<div *ngFor="let item of items" cdkDrag>{{item}}</div>
246-
</div>
247-
248-
<div cdkDropList id="items" *ngIf="showDuplicateContainer"></div>
249-
`
250-
})
251-
class SimpleDropZone {
252-
@ViewChildren(CdkDrag) dragItems: QueryList<CdkDrag>;
253-
@ViewChildren(CdkDropList) dropInstances: QueryList<CdkDropList>;
254-
items = ['Zero', 'One', 'Two', 'Three'];
255-
showDuplicateContainer = false;
256-
}

0 commit comments

Comments
 (0)