Skip to content

fix(drag-drop): not accounting for scroll position when dragging #12098

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.

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

Merged
merged 1 commit into from
Jul 9, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions src/cdk-experimental/drag-drop/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ ng_module(
deps = [
"@rxjs",
"//src/cdk/platform",
"//src/cdk/overlay",
],
tsconfig = "//src/cdk-experimental:tsconfig-build.json",
)
Expand Down
206 changes: 164 additions & 42 deletions src/cdk-experimental/drag-drop/drag.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,21 @@ describe('CdkDrag', () => {
expect(dragElement.style.transform).toBe('translate3d(50px, 100px, 0px)');
}));

it('should drag an element freely to a particular position when the page is scrolled',
fakeAsync(() => {
const fixture = createComponent(StandaloneDraggable);
fixture.detectChanges();

const cleanup = makePageScrollable();
const dragElement = fixture.componentInstance.dragElement.nativeElement;

scrollTo(0, 500);
expect(dragElement.style.transform).toBeFalsy();
dragElementViaMouse(fixture, dragElement, 50, 100);
expect(dragElement.style.transform).toBe('translate3d(50px, 100px, 0px)');
cleanup();
}));

it('should continue dragging the element from where it was left off', fakeAsync(() => {
const fixture = createComponent(StandaloneDraggable);
fixture.detectChanges();
Expand All @@ -52,6 +67,26 @@ describe('CdkDrag', () => {
dragElementViaMouse(fixture, dragElement, 100, 200);
expect(dragElement.style.transform).toBe('translate3d(150px, 300px, 0px)');
}));

it('should continue dragging from where it was left off when the page is scrolled',
fakeAsync(() => {
const fixture = createComponent(StandaloneDraggable);
fixture.detectChanges();

const dragElement = fixture.componentInstance.dragElement.nativeElement;
const cleanup = makePageScrollable();

scrollTo(0, 500);
expect(dragElement.style.transform).toBeFalsy();

dragElementViaMouse(fixture, dragElement, 50, 100);
expect(dragElement.style.transform).toBe('translate3d(50px, 100px, 0px)');

dragElementViaMouse(fixture, dragElement, 100, 200);
expect(dragElement.style.transform).toBe('translate3d(150px, 300px, 0px)');

cleanup();
}));
});

describe('touch dragging', () => {
Expand All @@ -65,6 +100,21 @@ describe('CdkDrag', () => {
expect(dragElement.style.transform).toBe('translate3d(50px, 100px, 0px)');
}));

it('should drag an element freely to a particular position when the page is scrolled',
fakeAsync(() => {
const fixture = createComponent(StandaloneDraggable);
fixture.detectChanges();

const dragElement = fixture.componentInstance.dragElement.nativeElement;
const cleanup = makePageScrollable();

scrollTo(0, 500);
expect(dragElement.style.transform).toBeFalsy();
dragElementViaTouch(fixture, dragElement, 50, 100);
expect(dragElement.style.transform).toBe('translate3d(50px, 100px, 0px)');
cleanup();
}));

it('should continue dragging the element from where it was left off', fakeAsync(() => {
const fixture = createComponent(StandaloneDraggable);
fixture.detectChanges();
Expand All @@ -79,6 +129,26 @@ describe('CdkDrag', () => {
expect(dragElement.style.transform).toBe('translate3d(150px, 300px, 0px)');
}));

it('should continue dragging from where it was left off when the page is scrolled',
fakeAsync(() => {
const fixture = createComponent(StandaloneDraggable);
fixture.detectChanges();

const dragElement = fixture.componentInstance.dragElement.nativeElement;
const cleanup = makePageScrollable();

scrollTo(0, 500);
expect(dragElement.style.transform).toBeFalsy();

dragElementViaTouch(fixture, dragElement, 50, 100);
expect(dragElement.style.transform).toBe('translate3d(50px, 100px, 0px)');

dragElementViaTouch(fixture, dragElement, 100, 200);
expect(dragElement.style.transform).toBe('translate3d(150px, 300px, 0px)');

cleanup();
}));

it('should prevent the default `touchmove` action on the page while dragging',
fakeAsync(() => {
const fixture = createComponent(StandaloneDraggable);
Expand Down Expand Up @@ -257,58 +327,36 @@ describe('CdkDrag', () => {
it('should move the placeholder as an item is being sorted down', fakeAsync(() => {
const fixture = createComponent(DraggableInDropZone);
fixture.detectChanges();
assertDownwardSorting(fixture);
}));

const items = fixture.componentInstance.dragItems.toArray();
const draggedItem = items[0].element.nativeElement;
const {top, left} = draggedItem.getBoundingClientRect();

dispatchMouseEvent(draggedItem, 'mousedown', left, top);
fixture.detectChanges();

const placeholder = document.querySelector('.cdk-drag-placeholder')! as HTMLElement;

// Drag over each item one-by-one going downwards.
for (let i = 0; i < items.length; i++) {
const elementRect = items[i].element.nativeElement.getBoundingClientRect();

// Add a few pixels to the top offset so we get some overlap.
dispatchMouseEvent(document, 'mousemove', elementRect.left, elementRect.top + 5);
it('should move the placeholder as an item is being sorted down on a scrolled page',
fakeAsync(() => {
const fixture = createComponent(DraggableInDropZone);
fixture.detectChanges();
expect(getElementIndex(placeholder)).toBe(i);
}
const cleanup = makePageScrollable();

dispatchMouseEvent(document, 'mouseup');
fixture.detectChanges();
flush();
}));
scrollTo(0, 500);
assertDownwardSorting(fixture);
cleanup();
}));

it('should move the placeholder as an item is being sorted up', fakeAsync(() => {
const fixture = createComponent(DraggableInDropZone);
fixture.detectChanges();
assertUpwardSorting(fixture);
}));

const items = fixture.componentInstance.dragItems.toArray();
const draggedItem = items[items.length - 1].element.nativeElement;
const {top, left} = draggedItem.getBoundingClientRect();

dispatchMouseEvent(draggedItem, 'mousedown', left, top);
fixture.detectChanges();

const placeholder = document.querySelector('.cdk-drag-placeholder')! as HTMLElement;

// Drag over each item one-by-one going upwards.
for (let i = items.length - 1; i > -1; i--) {
const elementRect = items[i].element.nativeElement.getBoundingClientRect();

// Remove a few pixels from the bottom offset so we get some overlap.
dispatchMouseEvent(document, 'mousemove', elementRect.left, elementRect.bottom - 5);
it('should move the placeholder as an item is being sorted up on a scrolled page',
fakeAsync(() => {
const fixture = createComponent(DraggableInDropZone);
fixture.detectChanges();
expect(getElementIndex(placeholder)).toBe(Math.min(i + 1, items.length - 1));
}
const cleanup = makePageScrollable();

dispatchMouseEvent(document, 'mouseup');
fixture.detectChanges();
flush();
}));
scrollTo(0, 500);
assertUpwardSorting(fixture);
cleanup();
}));

it('should clean up the preview element if the item is destroyed mid-drag', fakeAsync(() => {
const fixture = createComponent(DraggableInDropZone);
Expand Down Expand Up @@ -580,3 +628,77 @@ function dragElementViaTouch(fixture: ComponentFixture<any>,
function getElementIndex(element: HTMLElement) {
return element.parentElement ? Array.from(element.parentElement.children).indexOf(element) : -1;
}

/**
* Adds a large element to the page in order to make it scrollable.
* @returns Function that should be used to clean up after the test is done.
*/
function makePageScrollable() {
const veryTallElement = document.createElement('div');
veryTallElement.style.width = '100%';
veryTallElement.style.height = '2000px';
document.body.appendChild(veryTallElement);

return () => {
scrollTo(0, 0);
veryTallElement.parentNode!.removeChild(veryTallElement);
};
}

/**
* Asserts that sorting an element down works correctly.
* @param fixture Fixture against which to run the assertions.
*/
function assertDownwardSorting(fixture: ComponentFixture<DraggableInDropZone>) {
const items = fixture.componentInstance.dragItems.toArray();
const draggedItem = items[0].element.nativeElement;
const {top, left} = draggedItem.getBoundingClientRect();

dispatchMouseEvent(draggedItem, 'mousedown', left, top);
fixture.detectChanges();

const placeholder = document.querySelector('.cdk-drag-placeholder')! as HTMLElement;

// Drag over each item one-by-one going downwards.
for (let i = 0; i < items.length; i++) {
const elementRect = items[i].element.nativeElement.getBoundingClientRect();

// Add a few pixels to the top offset so we get some overlap.
dispatchMouseEvent(document, 'mousemove', elementRect.left, elementRect.top + 5);
fixture.detectChanges();
expect(getElementIndex(placeholder)).toBe(i);
}

dispatchMouseEvent(document, 'mouseup');
fixture.detectChanges();
flush();
}

/**
* Asserts that sorting an element up works correctly.
* @param fixture Fixture against which to run the assertions.
*/
function assertUpwardSorting(fixture: ComponentFixture<DraggableInDropZone>) {
const items = fixture.componentInstance.dragItems.toArray();
const draggedItem = items[items.length - 1].element.nativeElement;
const {top, left} = draggedItem.getBoundingClientRect();

dispatchMouseEvent(draggedItem, 'mousedown', left, top);
fixture.detectChanges();

const placeholder = document.querySelector('.cdk-drag-placeholder')! as HTMLElement;

// Drag over each item one-by-one going upwards.
for (let i = items.length - 1; i > -1; i--) {
const elementRect = items[i].element.nativeElement.getBoundingClientRect();

// Remove a few pixels from the bottom offset so we get some overlap.
dispatchMouseEvent(document, 'mousemove', elementRect.left, elementRect.bottom - 5);
fixture.detectChanges();
expect(getElementIndex(placeholder)).toBe(Math.min(i + 1, items.length - 1));
}

dispatchMouseEvent(document, 'mouseup');
fixture.detectChanges();
flush();
}
28 changes: 20 additions & 8 deletions src/cdk-experimental/drag-drop/drag.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {supportsPassiveEventListeners} from '@angular/cdk/platform';
import {CdkDragStart, CdkDragEnd, CdkDragExit, CdkDragEnter, CdkDragDrop} from './drag-events';
import {CdkDragPreview} from './drag-preview';
import {CdkDragPlaceholder} from './drag-placeholder';
import {ViewportRuler} from '@angular/cdk/overlay';

/** Event options that can be used to bind an active event. */
const activeEventOptions = supportsPassiveEventListeners() ? {passive: false} : false;
Expand Down Expand Up @@ -84,6 +85,9 @@ export class CdkDrag implements AfterContentInit, OnDestroy {
/** Drop container in which the CdkDrag resided when dragging began. */
private _initialContainer: CdkDropContainer;

/** Cached scroll position on the page when the element was picked up. */
private _scrollPosition: {top: number, left: number};

/** Element that can be used to drag the draggable item. */
@ContentChild(CdkDragHandle) _handle: CdkDragHandle;

Expand Down Expand Up @@ -117,7 +121,8 @@ export class CdkDrag implements AfterContentInit, OnDestroy {
@Inject(CDK_DROP_CONTAINER) @Optional() @SkipSelf() public dropContainer: CdkDropContainer,
@Inject(DOCUMENT) document: any,
private _ngZone: NgZone,
private _viewContainerRef: ViewContainerRef) {
private _viewContainerRef: ViewContainerRef,
private _viewportRuler: ViewportRuler) {
this._document = document;
}

Expand All @@ -135,7 +140,7 @@ export class CdkDrag implements AfterContentInit, OnDestroy {
dragElement.addEventListener('mousedown', this._pointerDown);
dragElement.addEventListener('touchstart', this._pointerDown);

// Webkit won't preventDefault on a dynamically-added `touchmove` listener, which means that
// WebKit won't preventDefault on a dynamically-added `touchmove` listener, which means that
// we need to add one ahead of time. See https://bugs.webkit.org/show_bug.cgi?id=184250.
// TODO: move into a central registry.
this._ngZone.runOutsideAngular(() => {
Expand Down Expand Up @@ -165,6 +170,7 @@ export class CdkDrag implements AfterContentInit, OnDestroy {

this._isDragging = true;
this._initialContainer = this.dropContainer;
this._scrollPosition = this._viewportRuler.getViewportScrollPosition();

// If we have a custom preview template, the element won't be visible anyway so we avoid the
// extra `getBoundingClientRect` calls and just move the preview next to the cursor.
Expand Down Expand Up @@ -371,10 +377,12 @@ export class CdkDrag implements AfterContentInit, OnDestroy {
const elementRect = this.element.nativeElement.getBoundingClientRect();
const handleElement = this._handle ? this._handle.element.nativeElement : null;
const referenceRect = handleElement ? handleElement.getBoundingClientRect() : elementRect;
const x = this._isTouchEvent(event) ? event.targetTouches[0].pageX - referenceRect.left :
event.offsetX;
const y = this._isTouchEvent(event) ? event.targetTouches[0].pageY - referenceRect.top :
event.offsetY;
const x = this._isTouchEvent(event) ?
event.targetTouches[0].pageX - referenceRect.left - this._scrollPosition.left :
event.offsetX;
const y = this._isTouchEvent(event) ?
event.targetTouches[0].pageY - referenceRect.top - this._scrollPosition.top :
event.offsetY;

return {
x: referenceRect.left - elementRect.left + x,
Expand Down Expand Up @@ -456,8 +464,12 @@ export class CdkDrag implements AfterContentInit, OnDestroy {

/** Determines the point of the page that was touched by the user. */
private _getPointerPositionOnPage(event: MouseEvent | TouchEvent): Point {
return this._isTouchEvent(event) ? {x: event.touches[0].pageX, y: event.touches[0].pageY} :
{x: event.pageX, y: event.pageY};
const point = this._isTouchEvent(event) ? event.touches[0] : event;

return {
x: point.pageX - this._scrollPosition.left,
y: point.pageY - this._scrollPosition.top
};
}

/** Listener used to prevent `touchmove` events while the element is being dragged. */
Expand Down
2 changes: 1 addition & 1 deletion src/cdk-experimental/drag-drop/drop.scss
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
$cdk-z-index-drag-preview: 1000;

.cdk-drag-preview {
position: absolute;
position: fixed;
top: 0;
left: 0;
z-index: $cdk-z-index-drag-preview;
Expand Down