Skip to content

Commit dcf2ed9

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 141afcf commit dcf2ed9

File tree

4 files changed

+94
-4
lines changed

4 files changed

+94
-4
lines changed

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

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import {moveItemInArray} from './drag-utils';
2727
import {CdkDropList} from './drop-list';
2828
import {CdkDragHandle} from './drag-handle';
2929
import {CdkDropListGroup} from './drop-list-group';
30+
import {extendStyles} from './drag-styling';
3031

3132
const ITEM_HEIGHT = 25;
3233
const ITEM_WIDTH = 75;
@@ -989,6 +990,8 @@ describe('CdkDrag', () => {
989990
.toBe('ltr', 'Expected preview element to inherit the directionality.');
990991
expect(previewRect.width).toBe(itemRect.width, 'Expected preview width to match element');
991992
expect(previewRect.height).toBe(itemRect.height, 'Expected preview height to match element');
993+
expect(preview.style.pointerEvents)
994+
.toBe('none', 'Expected pointer events to be disabled on the preview');
992995

993996
dispatchMouseEvent(document, 'mouseup');
994997
fixture.detectChanges();
@@ -2227,6 +2230,45 @@ describe('CdkDrag', () => {
22272230
expect(Array.from(component.group._items)).toEqual([component.listOne, component.listTwo]);
22282231
}));
22292232

2233+
it('should not be able to drop an element into a container that is under another element',
2234+
fakeAsync(() => {
2235+
const fixture = createComponent(ConnectedDropZones);
2236+
fixture.detectChanges();
2237+
2238+
const groups = fixture.componentInstance.groupedDragItems.slice();
2239+
const element = groups[0][1].element.nativeElement;
2240+
const dropInstances = fixture.componentInstance.dropInstances.toArray();
2241+
const targetRect = groups[1][2].element.nativeElement.getBoundingClientRect();
2242+
const coverElement = document.createElement('div');
2243+
const targetGroupRect = dropInstances[1].element.nativeElement.getBoundingClientRect();
2244+
2245+
// Add an extra element that covers the target container.
2246+
fixture.nativeElement.appendChild(coverElement);
2247+
extendStyles(coverElement.style, {
2248+
position: 'fixed',
2249+
top: targetGroupRect.top + 'px',
2250+
left: targetGroupRect.left + 'px',
2251+
bottom: targetGroupRect.bottom + 'px',
2252+
right: targetGroupRect.right + 'px',
2253+
background: 'orange'
2254+
});
2255+
2256+
dragElementViaMouse(fixture, element, targetRect.left + 1, targetRect.top + 1);
2257+
flush();
2258+
fixture.detectChanges();
2259+
2260+
const event = fixture.componentInstance.droppedSpy.calls.mostRecent().args[0];
2261+
2262+
expect(event).toBeTruthy();
2263+
expect(event).toEqual({
2264+
previousIndex: 1,
2265+
currentIndex: 1,
2266+
item: groups[0][1],
2267+
container: dropInstances[0],
2268+
previousContainer: dropInstances[0]
2269+
});
2270+
}));
2271+
22302272
});
22312273

22322274
});

src/cdk/drag-drop/drag.ts

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

640640
extendStyles(preview.style, {
641+
// It's important that we disable the pointer events on the preview, because
642+
// it can throw off the `document.elementFromPoint` calls in the `CdkDropList`.
643+
pointerEvents: 'none',
641644
position: 'fixed',
642645
top: '0',
643646
left: '0',

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

Lines changed: 48 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,10 @@ import {
2121
Directive,
2222
ChangeDetectorRef,
2323
SkipSelf,
24+
Inject,
2425
} from '@angular/core';
2526
import {Directionality} from '@angular/cdk/bidi';
27+
import {DOCUMENT} from '@angular/common';
2628
import {CdkDrag} from './drag';
2729
import {DragDropRegistry} from './drag-drop-registry';
2830
import {CdkDragDrop, CdkDragEnter, CdkDragExit, CdkDragSortEvent} from './drag-events';
@@ -93,6 +95,8 @@ interface ListPositionCacheEntry {
9395
}
9496
})
9597
export class CdkDropList<T = any> implements OnInit, OnDestroy {
98+
private _document: Document | undefined;
99+
96100
/** Draggable items in the container. */
97101
@ContentChildren(forwardRef(() => CdkDrag)) _draggables: QueryList<CdkDrag>;
98102

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

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

385430
return result && result.drop.enterPredicate(item, result.drop) ? result.drop : null;
386431
}

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)