Skip to content

Commit ffa43db

Browse files
committed
fix(drag-drop): account for out of view container and stacking order
Currently we use the `ClientRect` of each drop container to determine whether an item can be dropped into it. Since the `ClientRect` doesn't know whether the container is scrolled out of the view or is positioned under another element, users can end up accidentally moving items into containers that aren't visible. These changes rework the logic to look for which containers intersect with the user's pointer, and to filter based on whether the container is the top-most element at the pointer's position. Fixes #14231.
1 parent df488b0 commit ffa43db

File tree

4 files changed

+96
-4
lines changed

4 files changed

+96
-4
lines changed

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

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {CdkDragDrop} from './drag-events';
2626
import {moveItemInArray} from './drag-utils';
2727
import {CdkDropList} from './drop-list';
2828
import {CdkDragHandle} from './drag-handle';
29+
import {extendStyles} from './drag-styling';
2930

3031
const ITEM_HEIGHT = 25;
3132
const ITEM_WIDTH = 75;
@@ -978,6 +979,8 @@ describe('CdkDrag', () => {
978979
.toBe('ltr', 'Expected preview element to inherit the directionality.');
979980
expect(previewRect.width).toBe(itemRect.width, 'Expected preview width to match element');
980981
expect(previewRect.height).toBe(itemRect.height, 'Expected preview height to match element');
982+
expect(preview.style.pointerEvents)
983+
.toBe('none', 'Expected pointer events to be disabled on the preview');
981984

982985
dispatchMouseEvent(document, 'mouseup');
983986
fixture.detectChanges();
@@ -2208,6 +2211,47 @@ describe('CdkDrag', () => {
22082211
expect(fixture.componentInstance.droppedSpy).not.toHaveBeenCalled();
22092212
}));
22102213

2214+
it('should not be able to drop an element into a container that is under another element',
2215+
fakeAsync(() => {
2216+
const fixture = createComponent(ConnectedDropZones);
2217+
fixture.detectChanges();
2218+
2219+
const groups = fixture.componentInstance.groupedDragItems.slice();
2220+
const element = groups[0][1].element.nativeElement;
2221+
const dropInstances = fixture.componentInstance.dropInstances.toArray();
2222+
const targetRect = groups[1][2].element.nativeElement.getBoundingClientRect();
2223+
const coverElement = document.createElement('div');
2224+
const targetGroupRect = dropInstances[1].element.nativeElement.getBoundingClientRect();
2225+
2226+
// Add an extra element that covers the target container.
2227+
fixture.nativeElement.appendChild(coverElement);
2228+
extendStyles(coverElement.style, {
2229+
position: 'fixed',
2230+
top: targetGroupRect.top + 'px',
2231+
left: targetGroupRect.left + 'px',
2232+
bottom: targetGroupRect.bottom + 'px',
2233+
right: targetGroupRect.right + 'px',
2234+
background: 'orange'
2235+
});
2236+
2237+
dragElementViaMouse(fixture, element, targetRect.left + 1, targetRect.top + 1);
2238+
flush();
2239+
fixture.detectChanges();
2240+
2241+
const event = fixture.componentInstance.droppedSpy.calls.mostRecent().args[0];
2242+
2243+
expect(event).toBeTruthy();
2244+
2245+
expect(event.previousIndex).toBe(1);
2246+
expect(event).toEqual({
2247+
previousIndex: 1,
2248+
currentIndex: 1,
2249+
item: groups[0][1],
2250+
container: dropInstances[0],
2251+
previousContainer: dropInstances[0]
2252+
});
2253+
}));
2254+
22112255
});
22122256

22132257
});

src/cdk/drag-drop/drag.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -632,6 +632,9 @@ export class CdkDrag<T = any> implements AfterViewInit, OnDestroy {
632632
}
633633

634634
extendStyles(preview.style, {
635+
// It's important that we disable the pointer events on the preview, because
636+
// it can throw off the `document.elementFromPoint` calls in the `CdkDropList`.
637+
pointerEvents: 'none',
635638
position: 'fixed',
636639
top: '0',
637640
left: '0',

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

Lines changed: 48 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,10 @@ import {
2020
Optional,
2121
Directive,
2222
ChangeDetectorRef,
23+
Inject,
2324
} from '@angular/core';
2425
import {Directionality} from '@angular/cdk/bidi';
26+
import {DOCUMENT} from '@angular/common';
2527
import {CdkDrag} from './drag';
2628
import {DragDropRegistry} from './drag-drop-registry';
2729
import {CdkDragDrop, CdkDragEnter, CdkDragExit, CdkDragSortEvent} from './drag-events';
@@ -90,6 +92,8 @@ interface ListPositionCacheEntry {
9092
}
9193
})
9294
export class CdkDropList<T = any> implements OnInit, OnDestroy {
95+
private _document: Document | undefined;
96+
9397
/** Draggable items in the container. */
9498
@ContentChildren(forwardRef(() => CdkDrag)) _draggables: QueryList<CdkDrag>;
9599

@@ -157,7 +161,17 @@ export class CdkDropList<T = any> implements OnInit, OnDestroy {
157161
private _dragDropRegistry: DragDropRegistry<CdkDrag, CdkDropList<T>>,
158162
private _changeDetectorRef: ChangeDetectorRef,
159163
@Optional() private _dir?: Directionality,
160-
@Optional() private _group?: CdkDropListGroup<CdkDropList>) {}
164+
@Optional() private _group?: CdkDropListGroup<CdkDropList>,
165+
// @breaking-change 8.0.0 `_document` parameter to be made required.
166+
@Optional() @Inject(DOCUMENT) _document?: any) {
167+
168+
// @breaking-change 8.0.0 Remove null checks once `_document` parameter is required.
169+
if (_document) {
170+
this._document = _document;
171+
} else if (typeof document !== 'undefined') {
172+
this._document = document;
173+
}
174+
}
161175

162176
ngOnInit() {
163177
this._dragDropRegistry.registerDropContainer(this);
@@ -376,8 +390,39 @@ export class CdkDropList<T = any> implements OnInit, OnDestroy {
376390
* @param y Position of the item along the Y axis.
377391
*/
378392
_getSiblingContainerFromPosition(item: CdkDrag, x: number, y: number): CdkDropList | null {
379-
const result = this._positionCache.siblings
380-
.find(sibling => isInsideClientRect(sibling.clientRect, x, y));
393+
const results = this._positionCache.siblings.filter(sibling => {
394+
return isInsideClientRect(sibling.clientRect, x, y);
395+
});
396+
397+
// No drop containers are intersecting with the pointer.
398+
if (!results.length) {
399+
return null;
400+
}
401+
402+
let result: ListPositionCacheEntry | undefined = results[0];
403+
404+
// @breaking-change 8.0.0 remove null check once the
405+
// `_document` is made into a required parameter.
406+
if (this._document) {
407+
const elementFromPoint = this._document.elementFromPoint(x, y);
408+
409+
// If there's no element at the pointer position, then
410+
// the client rect is probably scrolled out of the view.
411+
if (!elementFromPoint) {
412+
return null;
413+
}
414+
415+
// The `ClientRect`, that we're using to find the container over which the user is
416+
// hovering, doesn't give us any information on whether the element has been scrolled
417+
// out of the view or whether it's overlapping with other containers. This means that
418+
// we could end up transferring the item into a container that's invisible or is positioned
419+
// below another one. We use the result from `elementFromPoint` to get the top-most element
420+
// at the pointer position and to find whether it's one of the intersecting drop containers.
421+
result = results.find(sibling => {
422+
const element = sibling.drop.element.nativeElement;
423+
return element === elementFromPoint || element.contains(elementFromPoint);
424+
});
425+
}
381426

382427
return result && result.drop.enterPredicate(item, result.drop) ? result.drop : null;
383428
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ export declare class CdkDropList<T = any> implements OnInit, OnDestroy {
118118
lockAxis: 'x' | 'y';
119119
orientation: 'horizontal' | 'vertical';
120120
sorted: EventEmitter<CdkDragSortEvent<T>>;
121-
constructor(element: ElementRef<HTMLElement>, _dragDropRegistry: DragDropRegistry<CdkDrag, CdkDropList<T>>, _changeDetectorRef: ChangeDetectorRef, _dir?: Directionality | undefined, _group?: CdkDropListGroup<CdkDropList<any>> | undefined);
121+
constructor(element: ElementRef<HTMLElement>, _dragDropRegistry: DragDropRegistry<CdkDrag, CdkDropList<T>>, _changeDetectorRef: ChangeDetectorRef, _dir?: Directionality | undefined, _group?: CdkDropListGroup<CdkDropList<any>> | undefined, _document?: any);
122122
_canReturnItem(x: number, y: number): boolean;
123123
_getSiblingContainerFromPosition(item: CdkDrag, x: number, y: number): CdkDropList | null;
124124
_sortItem(item: CdkDrag, pointerX: number, pointerY: number, pointerDelta: {

0 commit comments

Comments
 (0)