Skip to content

Commit

Permalink
fix(drag-drop): error if custom preview or placeholder node is not an…
Browse files Browse the repository at this point in the history
… element (#16409)

When the custom preview or placeholder root node is provided we assume that it's an `HTMLElement` and we try to set some styles on it, however it's possible for it to be a text node (e.g. `<ng-template cdkDragPreview>Hello</ng-template>`) which will cause an error to be thrown. These changes wrap the node in a `div` if it's not an element node.
  • Loading branch information
crisbeto authored and jelbourn committed Sep 9, 2019
1 parent 357da7b commit 8a4bed5
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 2 deletions.
68 changes: 68 additions & 0 deletions src/cdk/drag-drop/directives/drag.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2720,6 +2720,21 @@ describe('CdkDrag', () => {
expect(preview.style.transform).toBe('translate3d(100px, 50px, 0px)');
}));

it('should not throw when custom preview only has text', fakeAsync(() => {
const fixture = createComponent(DraggableInDropZoneWithCustomTextOnlyPreview);
fixture.detectChanges();
const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement;

expect(() => {
startDraggingViaMouse(fixture, item);
}).not.toThrow();

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

expect(preview).toBeTruthy();
expect(preview.textContent!.trim()).toContain('Hello One');
}));

it('should be able to customize the placeholder', fakeAsync(() => {
const fixture = createComponent(DraggableInDropZoneWithCustomPlaceholder);
fixture.detectChanges();
Expand Down Expand Up @@ -2753,6 +2768,21 @@ describe('CdkDrag', () => {
expect(placeholder.textContent!.trim()).not.toContain('Custom placeholder');
}));

it('should not throw when custom placeholder only has text', fakeAsync(() => {
const fixture = createComponent(DraggableInDropZoneWithCustomTextOnlyPlaceholder);
fixture.detectChanges();
const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement;

expect(() => {
startDraggingViaMouse(fixture, item);
}).not.toThrow();

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

expect(placeholder).toBeTruthy();
expect(placeholder.textContent!.trim()).toContain('Hello One');
}));

it('should clear the `transform` value from siblings when item is dropped`', fakeAsync(() => {
const fixture = createComponent(DraggableInDropZone);
fixture.detectChanges();
Expand Down Expand Up @@ -4497,6 +4527,28 @@ class DraggableInDropZoneWithCustomPreview {
}


@Component({
template: `
<div cdkDropList style="width: 100px; background: pink;">
<div
*ngFor="let item of items"
cdkDrag
[cdkDragConstrainPosition]="constrainPosition"
[cdkDragBoundary]="boundarySelector"
style="width: 100%; height: ${ITEM_HEIGHT}px; background: red;">
{{item}}
<ng-template cdkDragPreview>Hello {{item}}</ng-template>
</div>
</div>
`
})
class DraggableInDropZoneWithCustomTextOnlyPreview {
@ViewChild(CdkDropList, {static: false}) dropInstance: CdkDropList;
@ViewChildren(CdkDrag) dragItems: QueryList<CdkDrag>;
items = ['Zero', 'One', 'Two', 'Three'];
}


@Component({
template: `
<div cdkDropList style="width: 100px; background: pink;">
Expand All @@ -4516,6 +4568,22 @@ class DraggableInDropZoneWithCustomPlaceholder {
renderPlaceholder = true;
}

@Component({
template: `
<div cdkDropList style="width: 100px; background: pink;">
<div *ngFor="let item of items" cdkDrag
style="width: 100%; height: ${ITEM_HEIGHT}px; background: red;">
{{item}}
<ng-template cdkDragPlaceholder>Hello {{item}}</ng-template>
</div>
</div>
`
})
class DraggableInDropZoneWithCustomTextOnlyPlaceholder {
@ViewChildren(CdkDrag) dragItems: QueryList<CdkDrag>;
items = ['Zero', 'One', 'Two', 'Three'];
}

const CONNECTED_DROP_ZONES_STYLES = [`
.cdk-drop-list {
display: block;
Expand Down
20 changes: 18 additions & 2 deletions src/cdk/drag-drop/drag-ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -849,7 +849,7 @@ export class DragRef<T = any> {
if (previewTemplate) {
const viewRef = previewConfig!.viewContainer.createEmbeddedView(previewTemplate,
previewConfig!.context);
preview = viewRef.rootNodes[0];
preview = getRootNode(viewRef, this._document);
this._previewRef = viewRef;
preview.style.transform =
getTransform(this._pickupPositionOnPage.x, this._pickupPositionOnPage.y);
Expand Down Expand Up @@ -941,7 +941,7 @@ export class DragRef<T = any> {
placeholderTemplate,
placeholderConfig!.context
);
placeholder = this._placeholderRef.rootNodes[0];
placeholder = getRootNode(this._placeholderRef, this._document);
} else {
placeholder = deepCloneNode(this._rootElement);
}
Expand Down Expand Up @@ -1231,3 +1231,19 @@ function getPreviewInsertionPoint(documentRef: any): HTMLElement {
documentRef.msFullscreenElement ||
documentRef.body;
}

/**
* Gets the root HTML element of an embedded view.
* If the root is not an HTML element it gets wrapped in one.
*/
function getRootNode(viewRef: EmbeddedViewRef<any>, _document: Document): HTMLElement {
const rootNode: Node = viewRef.rootNodes[0];

if (rootNode.nodeType !== _document.ELEMENT_NODE) {
const wrapper = _document.createElement('div');
wrapper.appendChild(rootNode);
return wrapper;
}

return rootNode as HTMLElement;
}

0 comments on commit 8a4bed5

Please sign in to comment.