Skip to content

Commit 68b53f7

Browse files
crisbetovivian-hu-zz
authored andcommittedNov 10, 2018
fix(drag-drop): avoid generating elements with duplicate ids (#13489)
If the consumer hasn't passed in a custom drag placeholder or preview, we clone the element that is being dragged. This can cause the DOM to have multiple elements with the same id.
1 parent 962dbeb commit 68b53f7

File tree

3 files changed

+41
-3
lines changed

3 files changed

+41
-3
lines changed
 

‎src/cdk/drag-drop/drag-drop.md

+4-1
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,10 @@ restrict the user to only be able to do so using a handle element, you can do it
113113
### Customizing the drag preview
114114
When a `cdkDrag` element is picked up, it will create a preview element visible while dragging.
115115
By default, this will be a clone of the original element positioned next to the user's cursor.
116-
This preview can be customized, though, by providing a custom template via `*cdkDragPreview`:
116+
This preview can be customized, though, by providing a custom template via `*cdkDragPreview`.
117+
Note that the cloned element will remove its `id` attribute in order to avoid having multiple
118+
elements with the same `id` on the page. This will cause any CSS that targets that `id` not
119+
to be applied.
117120

118121
<!-- example(cdk-drag-drop-custom-preview) -->
119122

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

+27
Original file line numberDiff line numberDiff line change
@@ -896,6 +896,19 @@ describe('CdkDrag', () => {
896896
expect(preview.parentNode).toBeFalsy('Expected preview to be removed from the DOM');
897897
}));
898898

899+
it('should clear the id from the preview', fakeAsync(() => {
900+
const fixture = createComponent(DraggableInDropZone);
901+
fixture.detectChanges();
902+
const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement;
903+
item.id = 'custom-id';
904+
905+
startDraggingViaMouse(fixture, item);
906+
907+
const preview = document.querySelector('.cdk-drag-preview')! as HTMLElement;
908+
909+
expect(preview.getAttribute('id')).toBeFalsy();
910+
}));
911+
899912
it('should not create a preview if the element was not dragged far enough', fakeAsync(() => {
900913
const fixture = createComponent(DraggableInDropZone, [], 5);
901914
fixture.detectChanges();
@@ -1053,6 +1066,20 @@ describe('CdkDrag', () => {
10531066
expect(placeholder.parentNode).toBeFalsy('Expected placeholder to be removed from the DOM');
10541067
}));
10551068

1069+
it('should remove the id from the placeholder', fakeAsync(() => {
1070+
const fixture = createComponent(DraggableInDropZone);
1071+
fixture.detectChanges();
1072+
const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement;
1073+
1074+
item.id = 'custom-id';
1075+
1076+
startDraggingViaMouse(fixture, item);
1077+
1078+
const placeholder = document.querySelector('.cdk-drag-placeholder')! as HTMLElement;
1079+
1080+
expect(placeholder.getAttribute('id')).toBeFalsy();
1081+
}));
1082+
10561083
it('should not create placeholder if the element was not dragged far enough', fakeAsync(() => {
10571084
const fixture = createComponent(DraggableInDropZone, [], 5);
10581085
fixture.detectChanges();

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

+10-2
Original file line numberDiff line numberDiff line change
@@ -586,7 +586,7 @@ export class CdkDrag<T = any> implements AfterViewInit, OnDestroy {
586586
const element = this._rootElement;
587587
const elementRect = element.getBoundingClientRect();
588588

589-
preview = element.cloneNode(true) as HTMLElement;
589+
preview = deepCloneNode(element);
590590
preview.style.width = `${elementRect.width}px`;
591591
preview.style.height = `${elementRect.height}px`;
592592
preview.style.transform = getTransform(elementRect.left, elementRect.top);
@@ -616,7 +616,7 @@ export class CdkDrag<T = any> implements AfterViewInit, OnDestroy {
616616
);
617617
placeholder = this._placeholderRef.rootNodes[0];
618618
} else {
619-
placeholder = this._rootElement.cloneNode(true) as HTMLElement;
619+
placeholder = deepCloneNode(this._rootElement);
620620
}
621621

622622
placeholder.classList.add('cdk-drag-placeholder');
@@ -823,3 +823,11 @@ interface Point {
823823
function getTransform(x: number, y: number): string {
824824
return `translate3d(${x}px, ${y}px, 0)`;
825825
}
826+
827+
/** Creates a deep clone of an element. */
828+
function deepCloneNode(node: HTMLElement): HTMLElement {
829+
const clone = node.cloneNode(true) as HTMLElement;
830+
// Remove the `id` to avoid having multiple elements with the same id on the page.
831+
clone.removeAttribute('id');
832+
return clone;
833+
}

0 commit comments

Comments
 (0)
Please sign in to comment.