Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(cdk/drag-drop): don't block scrolling if it happens before delay has elapsed #21382

Merged
merged 1 commit into from
Jan 6, 2021
Merged

fix(cdk/drag-drop): don't block scrolling if it happens before delay has elapsed #21382

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 19 additions & 28 deletions src/cdk/drag-drop/directives/drag.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,7 @@ describe('CdkDrag', () => {
dispatchTouchEvent(fixture.componentInstance.dragElement.nativeElement, 'touchstart');
fixture.detectChanges();

dispatchTouchEvent(document, 'touchmove');
expect(dispatchTouchEvent(document, 'touchmove').defaultPrevented).toBe(true);

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

it('should not prevent the default touch action before the delay has elapsed', fakeAsync(() => {
spyOn(Date, 'now').and.callFake(() => currentTime);
let currentTime = 0;

const fixture = createComponent(StandaloneDraggable);
fixture.componentInstance.dragStartDelay = 500;
fixture.detectChanges();
const dragElement = fixture.componentInstance.dragElement.nativeElement;

expect(dragElement.style.transform).toBeFalsy('Expected element not to be moved by default.');

dispatchTouchEvent(dragElement, 'touchstart');
fixture.detectChanges();
currentTime += 250;

expect(dispatchTouchEvent(document, 'touchmove', 50, 100).defaultPrevented).toBe(false);
}));

it('should handle the drag delay as a string', fakeAsync(() => {
// We can't use Jasmine's `clock` because Zone.js interferes with it.
spyOn(Date, 'now').and.callFake(() => currentTime);
Expand Down Expand Up @@ -1206,34 +1225,6 @@ describe('CdkDrag', () => {
subscription.unsubscribe();
}));

it('should prevent the default `mousemove` action even before the drag threshold has ' +
'been reached', fakeAsync(() => {
const fixture = createComponent(StandaloneDraggable, [], 5);
fixture.detectChanges();
const dragElement = fixture.componentInstance.dragElement.nativeElement;

dispatchMouseEvent(dragElement, 'mousedown', 2, 2);
fixture.detectChanges();
const mousemoveEvent = dispatchMouseEvent(document, 'mousemove', 2, 2);
fixture.detectChanges();

expect(mousemoveEvent.defaultPrevented).toBe(true);
}));

it('should prevent the default `touchmove` action even before the drag threshold has ' +
'been reached', fakeAsync(() => {
const fixture = createComponent(StandaloneDraggable, [], 5);
fixture.detectChanges();
const dragElement = fixture.componentInstance.dragElement.nativeElement;

dispatchTouchEvent(dragElement, 'touchstart', 2, 2);
fixture.detectChanges();
const touchmoveEvent = dispatchTouchEvent(document, 'touchmove', 2, 2);
fixture.detectChanges();

expect(touchmoveEvent.defaultPrevented).toBe(true);
}));

it('should be able to configure the drag input defaults through a provider', fakeAsync(() => {
const config: DragDropConfig = {
draggingDisabled: true,
Expand Down
140 changes: 73 additions & 67 deletions src/cdk/drag-drop/drag-drop-registry.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {QueryList, ViewChildren, Component} from '@angular/core';
import {Component} from '@angular/core';
import {fakeAsync, TestBed, ComponentFixture, inject} from '@angular/core/testing';
import {
createMouseEvent,
Expand All @@ -9,25 +9,21 @@ import {
} from '@angular/cdk/testing/private';
import {DragDropRegistry} from './drag-drop-registry';
import {DragDropModule} from './drag-drop-module';
import {CdkDrag} from './directives/drag';
import {CdkDropList} from './directives/drop-list';

describe('DragDropRegistry', () => {
let fixture: ComponentFixture<SimpleDropZone>;
let testComponent: SimpleDropZone;
let registry: DragDropRegistry<CdkDrag, CdkDropList>;
let fixture: ComponentFixture<BlankComponent>;
let registry: DragDropRegistry<DragItem, DragList>;

beforeEach(fakeAsync(() => {
TestBed.configureTestingModule({
imports: [DragDropModule],
declarations: [SimpleDropZone],
declarations: [BlankComponent],
}).compileComponents();

fixture = TestBed.createComponent(SimpleDropZone);
testComponent = fixture.componentInstance;
fixture = TestBed.createComponent(BlankComponent);
fixture.detectChanges();

inject([DragDropRegistry], (c: DragDropRegistry<CdkDrag, CdkDropList>) => {
inject([DragDropRegistry], (c: DragDropRegistry<DragItem, DragList>) => {
registry = c;
})();
}));
Expand All @@ -37,38 +33,38 @@ describe('DragDropRegistry', () => {
});

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

expect(registry.isDragging(firstItem)).toBe(false);
registry.startDragging(firstItem, createMouseEvent('mousedown'));
expect(registry.isDragging(firstItem)).toBe(true);
expect(registry.isDragging(item)).toBe(false);
registry.startDragging(item, createMouseEvent('mousedown'));
expect(registry.isDragging(item)).toBe(true);
});

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

registry.startDragging(firstItem, createMouseEvent('mousedown'));
expect(registry.isDragging(firstItem)).toBe(true);
registry.startDragging(item, createMouseEvent('mousedown'));
expect(registry.isDragging(item)).toBe(true);

registry.stopDragging(firstItem);
expect(registry.isDragging(firstItem)).toBe(false);
registry.stopDragging(item);
expect(registry.isDragging(item)).toBe(false);
});

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

registry.startDragging(firstItem, createMouseEvent('mousedown'));
expect(registry.isDragging(firstItem)).toBe(true);
registry.startDragging(item, createMouseEvent('mousedown'));
expect(registry.isDragging(item)).toBe(true);

registry.removeDragItem(firstItem);
expect(registry.isDragging(firstItem)).toBe(false);
registry.removeDragItem(item);
expect(registry.isDragging(item)).toBe(false);
});

it('should dispatch `mousemove` events after starting to drag via the mouse', () => {
const spy = jasmine.createSpy('pointerMove spy');
const subscription = registry.pointerMove.subscribe(spy);

registry.startDragging(testComponent.dragItems.first, createMouseEvent('mousedown'));
const item = new DragItem(true);
registry.startDragging(item, createMouseEvent('mousedown'));
dispatchMouseEvent(document, 'mousemove');

expect(spy).toHaveBeenCalled();
Expand All @@ -79,9 +75,8 @@ describe('DragDropRegistry', () => {
it('should dispatch `touchmove` events after starting to drag via touch', () => {
const spy = jasmine.createSpy('pointerMove spy');
const subscription = registry.pointerMove.subscribe(spy);

registry.startDragging(testComponent.dragItems.first,
createTouchEvent('touchstart') as TouchEvent);
const item = new DragItem(true);
registry.startDragging(item, createTouchEvent('touchstart') as TouchEvent);
dispatchTouchEvent(document, 'touchmove');

expect(spy).toHaveBeenCalled();
Expand All @@ -92,10 +87,10 @@ describe('DragDropRegistry', () => {
it('should dispatch pointer move events if event propagation is stopped', () => {
const spy = jasmine.createSpy('pointerMove spy');
const subscription = registry.pointerMove.subscribe(spy);

const item = new DragItem(true);
fixture.nativeElement.addEventListener('mousemove', (e: MouseEvent) => e.stopPropagation());
registry.startDragging(testComponent.dragItems.first, createMouseEvent('mousedown'));
dispatchMouseEvent(fixture.nativeElement.querySelector('div'), 'mousemove');
registry.startDragging(item, createMouseEvent('mousedown'));
dispatchMouseEvent(fixture.nativeElement, 'mousemove');

expect(spy).toHaveBeenCalled();

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

registry.startDragging(testComponent.dragItems.first, createMouseEvent('mousedown'));
registry.startDragging(item, createMouseEvent('mousedown'));
dispatchMouseEvent(document, 'mouseup');

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

registry.startDragging(testComponent.dragItems.first,
createTouchEvent('touchstart') as TouchEvent);
registry.startDragging(item, createTouchEvent('touchstart') as TouchEvent);
dispatchTouchEvent(document, 'touchend');

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

fixture.nativeElement.addEventListener('mouseup', (e: MouseEvent) => e.stopPropagation());
registry.startDragging(testComponent.dragItems.first, createMouseEvent('mousedown'));
dispatchMouseEvent(fixture.nativeElement.querySelector('div'), 'mouseup');
registry.startDragging(item, createMouseEvent('mousedown'));
dispatchMouseEvent(fixture.nativeElement, 'mouseup');

expect(spy).toHaveBeenCalled();

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

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

// First finger down
registry.startDragging(firstItem, createTouchEvent('touchstart') as TouchEvent);
registry.startDragging(item, createTouchEvent('touchstart') as TouchEvent);
// Second finger down
registry.startDragging(firstItem, createTouchEvent('touchstart') as TouchEvent);
registry.startDragging(item, createTouchEvent('touchstart') as TouchEvent);
// First finger up
registry.stopDragging(firstItem);
registry.stopDragging(item);

// Ensure dragging is over - registry is empty
expect(registry.isDragging(firstItem)).toBe(false);
expect(registry.isDragging(item)).toBe(false);

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

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

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

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

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

const event = dispatchTouchEvent(fixture.nativeElement, 'touchmove');
expect(event.defaultPrevented).toBe(true);
});

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

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

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

registry.startDragging(testComponent.dragItems.first, createMouseEvent('mousedown'));
registry.startDragging(item, createMouseEvent('mousedown'));
dispatchFakeEvent(document, 'scroll');

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

class DragItem {
isDragging() { return this.shouldBeDragging; }
constructor(public shouldBeDragging = false) {
registry.registerDragItem(this);
}
}

class DragList {
constructor() {
registry.registerDropContainer(this);
}
}

@Component({template: ``})
class BlankComponent {}
});

@Component({
template: `
<div cdkDropList id="items" [cdkDropListData]="items">
<div *ngFor="let item of items" cdkDrag>{{item}}</div>
</div>

<div cdkDropList id="items" *ngIf="showDuplicateContainer"></div>
`
})
class SimpleDropZone {
@ViewChildren(CdkDrag) dragItems: QueryList<CdkDrag>;
@ViewChildren(CdkDropList) dropInstances: QueryList<CdkDropList>;
items = ['Zero', 'One', 'Two', 'Three'];
showDuplicateContainer = false;
}
Loading