Skip to content

Commit d4b505a

Browse files
authored
fix(cdk/drag-drop): throw if drop list or handle are set on a non-element node (#20668)
Currently if a `cdkDropList` or `cdkDragHandle` is attached to a non-element node, a cryptic error is thrown. These changes add a proper error, similar to the one we have on `cdkDrag`. Fixes #13540.
1 parent c4078aa commit d4b505a

File tree

7 files changed

+76
-9
lines changed

7 files changed

+76
-9
lines changed
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
/**
10+
* Asserts that a particular node is an element.
11+
* @param node Node to be checked.
12+
* @param name Name to attach to the error message.
13+
*/
14+
export function assertElementNode(node: Node, name: string): asserts node is HTMLElement {
15+
if (node.nodeType !== 1) {
16+
throw Error(`${name} must be attached to an element node. ` +
17+
`Currently attached to "${node.nodeName}".`);
18+
}
19+
}

src/cdk/drag-drop/directives/drag-handle.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
} from '@angular/core';
2020
import {Subject} from 'rxjs';
2121
import {CDK_DRAG_PARENT} from '../drag-parent';
22+
import {assertElementNode} from './assertions';
2223

2324
/**
2425
* Injection token that can be used to reference instances of `CdkDragHandle`. It serves as
@@ -54,6 +55,11 @@ export class CdkDragHandle implements OnDestroy {
5455
constructor(
5556
public element: ElementRef<HTMLElement>,
5657
@Inject(CDK_DRAG_PARENT) @Optional() @SkipSelf() parentDrag?: any) {
58+
59+
if (typeof ngDevMode === 'undefined' || ngDevMode) {
60+
assertElementNode(element.nativeElement, 'cdkDragHandle');
61+
}
62+
5763
this._parentDrag = parentDrag;
5864
}
5965

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

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -952,7 +952,7 @@ describe('CdkDrag', () => {
952952
expect(dragElement.style.transform).toBe('translate3d(50px, 50px, 0px)');
953953
}));
954954

955-
it('should throw if attached to an ng-container', fakeAsync(() => {
955+
it('should throw if drag item is attached to an ng-container', fakeAsync(() => {
956956
expect(() => {
957957
createComponent(DraggableOnNgContainer).detectChanges();
958958
flush();
@@ -1459,6 +1459,13 @@ describe('CdkDrag', () => {
14591459
expect(dragElement.style.webkitTapHighlightColor).toBe('purple');
14601460
}));
14611461

1462+
it('should throw if drag handle is attached to an ng-container', fakeAsync(() => {
1463+
expect(() => {
1464+
createComponent(DragHandleOnNgContainer).detectChanges();
1465+
flush();
1466+
}).toThrowError(/^cdkDragHandle must be attached to an element node/);
1467+
}));
1468+
14621469
});
14631470

14641471
describe('in a drop container', () => {
@@ -4209,6 +4216,13 @@ describe('CdkDrag', () => {
42094216
expect(spy).not.toHaveBeenCalled();
42104217
}));
42114218

4219+
it('should throw if drop list is attached to an ng-container', fakeAsync(() => {
4220+
expect(() => {
4221+
createComponent(DropListOnNgContainer).detectChanges();
4222+
flush();
4223+
}).toThrowError(/^cdkDropList must be attached to an element node/);
4224+
}));
4225+
42124226
});
42134227

42144228
describe('in a connected drop container', () => {
@@ -6201,6 +6215,24 @@ class NestedDropListGroups {
62016215
class DraggableOnNgContainer {}
62026216

62036217

6218+
@Component({
6219+
template: `
6220+
<div cdkDrag>
6221+
<ng-container cdkDragHandle></ng-container>
6222+
</div>
6223+
`
6224+
})
6225+
class DragHandleOnNgContainer {}
6226+
6227+
6228+
@Component({
6229+
template: `
6230+
<ng-container cdkDropList></ng-container>
6231+
`
6232+
})
6233+
class DropListOnNgContainer {}
6234+
6235+
62046236
@Component({
62056237
changeDetection: ChangeDetectionStrategy.OnPush,
62066238
template: `

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

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ import {DragRef, Point} from '../drag-ref';
5454
import {CDK_DROP_LIST, CdkDropListInternal as CdkDropList} from './drop-list';
5555
import {DragDrop} from '../drag-drop';
5656
import {CDK_DRAG_CONFIG, DragDropConfig, DragStartDelay, DragAxis} from './config';
57+
import {assertElementNode} from './assertions';
5758

5859
/** Element that can be moved inside a CdkDropList container. */
5960
@Directive({
@@ -182,7 +183,11 @@ export class CdkDrag<T = any> implements AfterViewInit, OnChanges, OnDestroy {
182183
public element: ElementRef<HTMLElement>,
183184
/** Droppable container that the draggable is a part of. */
184185
@Inject(CDK_DROP_LIST) @Optional() @SkipSelf() public dropContainer: CdkDropList,
185-
@Inject(DOCUMENT) private _document: any, private _ngZone: NgZone,
186+
/**
187+
* @deprecated `_document` parameter no longer being used and will be removed.
188+
* @breaking-change 12.0.0
189+
*/
190+
@Inject(DOCUMENT) _document: any, private _ngZone: NgZone,
186191
private _viewContainerRef: ViewContainerRef,
187192
@Optional() @Inject(CDK_DRAG_CONFIG) config: DragDropConfig,
188193
@Optional() private _dir: Directionality, dragDrop: DragDrop,
@@ -322,10 +327,8 @@ export class CdkDrag<T = any> implements AfterViewInit, OnChanges, OnDestroy {
322327
const rootElement = this.rootElementSelector ?
323328
getClosestMatchingAncestor(element, this.rootElementSelector) : element;
324329

325-
if (rootElement && rootElement.nodeType !== this._document.ELEMENT_NODE &&
326-
(typeof ngDevMode === 'undefined' || ngDevMode)) {
327-
throw Error(`cdkDrag must be attached to an element node. ` +
328-
`Currently attached to "${rootElement.nodeName}".`);
330+
if (rootElement && (typeof ngDevMode === 'undefined' || ngDevMode)) {
331+
assertElementNode(rootElement, 'cdkDrag');
329332
}
330333

331334
this._dragRef.withRootElement(rootElement || element);

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import {DragDrop} from '../drag-drop';
3131
import {DropListOrientation, DragAxis, DragDropConfig, CDK_DRAG_CONFIG} from './config';
3232
import {Subject} from 'rxjs';
3333
import {startWith, takeUntil} from 'rxjs/operators';
34+
import {assertElementNode} from './assertions';
3435

3536
/** Counter used to generate unique ids for drop zones. */
3637
let _uniqueIdCounter = 0;
@@ -60,7 +61,7 @@ export const CDK_DROP_LIST = new InjectionToken<CdkDropList>('CdkDropList');
6061
],
6162
host: {
6263
'class': 'cdk-drop-list',
63-
'[id]': 'id',
64+
'[attr.id]': 'id',
6465
'[class.cdk-drop-list-disabled]': 'disabled',
6566
'[class.cdk-drop-list-dragging]': '_dropListRef.isDragging()',
6667
'[class.cdk-drop-list-receiving]': '_dropListRef.isReceiving()',
@@ -174,6 +175,11 @@ export class CdkDropList<T = any> implements OnDestroy {
174175
@Optional() @Inject(CDK_DROP_LIST_GROUP) @SkipSelf()
175176
private _group?: CdkDropListGroup<CdkDropList>,
176177
@Optional() @Inject(CDK_DRAG_CONFIG) config?: DragDropConfig) {
178+
179+
if (typeof ngDevMode === 'undefined' || ngDevMode) {
180+
assertElementNode(element.nativeElement, 'cdkDropList');
181+
}
182+
177183
this._dropListRef = dragDrop.createDropList(element);
178184
this._dropListRef.data = this;
179185

src/dev-app/drag-drop/drag-drop-demo.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ <h2>To do</h2>
99
[cdkDropListData]="todo">
1010
<div *ngFor="let item of todo" cdkDrag>
1111
{{item}}
12-
<mat-icon cdkDragHandle svgIcon="dnd-move"></mat-icon>
12+
<ng-container cdkDragHandle>move</ng-container>
1313
</div>
1414
</div>
1515
</div>

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,8 @@ export declare class CdkDrag<T = any> implements AfterViewInit, OnChanges, OnDes
4141
started: EventEmitter<CdkDragStart>;
4242
constructor(
4343
element: ElementRef<HTMLElement>,
44-
dropContainer: CdkDropList, _document: any, _ngZone: NgZone, _viewContainerRef: ViewContainerRef, config: DragDropConfig, _dir: Directionality, dragDrop: DragDrop, _changeDetectorRef: ChangeDetectorRef, _selfHandle?: CdkDragHandle | undefined);
44+
dropContainer: CdkDropList,
45+
_document: any, _ngZone: NgZone, _viewContainerRef: ViewContainerRef, config: DragDropConfig, _dir: Directionality, dragDrop: DragDrop, _changeDetectorRef: ChangeDetectorRef, _selfHandle?: CdkDragHandle | undefined);
4546
getFreeDragPosition(): {
4647
readonly x: number;
4748
readonly y: number;

0 commit comments

Comments
 (0)