Skip to content

fix(drag-drop): auto scrolling not working if list uses scroll snapping #18294

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
Jan 27, 2020
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
51 changes: 51 additions & 0 deletions src/cdk/drag-drop/directives/drag.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3560,6 +3560,57 @@ describe('CdkDrag', () => {
expect(list.lockAxis).toBe('y');
}));

it('should disable scroll snapping while the user is dragging', fakeAsync(() => {
const fixture = createComponent(DraggableInDropZone);
fixture.detectChanges();
const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement;
const styles: any = fixture.componentInstance.dropInstance.element.nativeElement.style;

// This test only applies to browsers that support scroll snapping.
if (!('scrollSnapType' in styles) && !('msScrollSnapType' in styles)) {
return;
}

expect(styles.scrollSnapType || styles.msScrollSnapType).toBeFalsy();

startDraggingViaMouse(fixture, item);

expect(styles.scrollSnapType || styles.msScrollSnapType).toBe('none');

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

expect(styles.scrollSnapType || styles.msScrollSnapType).toBeFalsy();
}));

it('should restore the previous inline scroll snap value', fakeAsync(() => {
const fixture = createComponent(DraggableInDropZone);
fixture.detectChanges();
const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement;
const styles: any = fixture.componentInstance.dropInstance.element.nativeElement.style;

// This test only applies to browsers that support scroll snapping.
if (!('scrollSnapType' in styles) && !('msScrollSnapType' in styles)) {
return;
}

styles.scrollSnapType = styles.msScrollSnapType = 'block';
expect(styles.scrollSnapType || styles.msScrollSnapType).toBe('block');

startDraggingViaMouse(fixture, item);

expect(styles.scrollSnapType || styles.msScrollSnapType).toBe('none');

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

expect(styles.scrollSnapType || styles.msScrollSnapType).toBe('block');
}));

});

describe('in a connected drop container', () => {
Expand Down
13 changes: 13 additions & 0 deletions src/cdk/drag-drop/drop-list-ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,9 @@ export class DropListRef<T = any> {
/** Elements that can be scrolled while the user is dragging. */
private _scrollableElements: HTMLElement[];

/** Initial value for the element's `scroll-snap-type` style. */
private _initialScrollSnap: string;

constructor(
element: ElementRef<HTMLElement> | HTMLElement,
private _dragDropRegistry: DragDropRegistry<DragRef, DropListRef>,
Expand Down Expand Up @@ -233,8 +236,15 @@ export class DropListRef<T = any> {

/** Starts dragging an item. */
start(): void {
const styles = coerceElement(this.element).style;
this.beforeStarted.next();
this._isDragging = true;

// We need to disable scroll snapping while the user is dragging, because it breaks automatic
// scrolling. The browser seems to round the value based on the snapping points which means
// that we can't increment/decrement the scroll position.
this._initialScrollSnap = styles.msScrollSnapType || (styles as any).scrollSnapType || '';
(styles as any).scrollSnapType = styles.msScrollSnapType = 'none';
this._cacheItems();
this._siblings.forEach(sibling => sibling._startReceiving(this));
this._viewportScrollSubscription.unsubscribe();
Expand Down Expand Up @@ -597,6 +607,9 @@ export class DropListRef<T = any> {
private _reset() {
this._isDragging = false;

const styles = coerceElement(this.element).style;
(styles as any).scrollSnapType = styles.msScrollSnapType = this._initialScrollSnap;

// TODO(crisbeto): may have to wait for the animations to finish.
this._activeDraggables.forEach(item => item.getRootElement().style.transform = '');
this._siblings.forEach(sibling => sibling._stopReceiving(this));
Expand Down