Skip to content

Commit 8149766

Browse files
committed
fix(drag-drop): avoid generating elements with duplicate ids
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 8b2dc82 commit 8149766

File tree

3 files changed

+41
-3
lines changed

3 files changed

+41
-3
lines changed

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,10 @@ restrict the user to only be able to do so using a handle element, you can do it
100100
### Customizing the drag preview
101101
When a `cdkDrag` element is picked up, it will create a preview element visible while dragging.
102102
By default, this will be a clone of the original element positioned next to the user's cursor.
103-
This preview can be customized, though, by providing a custom template via `*cdkDragPreview`:
103+
This preview can be customized, though, by providing a custom template via `*cdkDragPreview`.
104+
Note that the cloned element will remove its `id` attribute in order to avoid having multiple
105+
elements with the same `id` on the page. This will cause any CSS that targets that `id` not
106+
to be applied.
104107

105108
<!-- example(cdk-drag-drop-custom-preview) -->
106109

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -831,6 +831,19 @@ describe('CdkDrag', () => {
831831
expect(preview.parentNode).toBeFalsy('Expected preview to be removed from the DOM');
832832
}));
833833

834+
it('should clear the id from the preview', fakeAsync(() => {
835+
const fixture = createComponent(DraggableInDropZone);
836+
fixture.detectChanges();
837+
const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement;
838+
item.id = 'custom-id';
839+
840+
startDraggingViaMouse(fixture, item);
841+
842+
const preview = document.querySelector('.cdk-drag-preview')! as HTMLElement;
843+
844+
expect(preview.getAttribute('id')).toBeFalsy();
845+
}));
846+
834847
it('should not create a preview if the element was not dragged far enough', fakeAsync(() => {
835848
const fixture = createComponent(DraggableInDropZone, [], 5);
836849
fixture.detectChanges();
@@ -988,6 +1001,20 @@ describe('CdkDrag', () => {
9881001
expect(placeholder.parentNode).toBeFalsy('Expected placeholder to be removed from the DOM');
9891002
}));
9901003

1004+
it('should remove the id from the placeholder', fakeAsync(() => {
1005+
const fixture = createComponent(DraggableInDropZone);
1006+
fixture.detectChanges();
1007+
const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement;
1008+
1009+
item.id = 'custom-id';
1010+
1011+
startDraggingViaMouse(fixture, item);
1012+
1013+
const placeholder = document.querySelector('.cdk-drag-placeholder')! as HTMLElement;
1014+
1015+
expect(placeholder.getAttribute('id')).toBeFalsy();
1016+
}));
1017+
9911018
it('should not create placeholder if the element was not dragged far enough', fakeAsync(() => {
9921019
const fixture = createComponent(DraggableInDropZone, [], 5);
9931020
fixture.detectChanges();

src/cdk/drag-drop/drag.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -566,7 +566,7 @@ export class CdkDrag<T = any> implements AfterViewInit, OnDestroy {
566566
const element = this._rootElement;
567567
const elementRect = element.getBoundingClientRect();
568568

569-
preview = element.cloneNode(true) as HTMLElement;
569+
preview = deepCloneNode(element);
570570
preview.style.width = `${elementRect.width}px`;
571571
preview.style.height = `${elementRect.height}px`;
572572
preview.style.transform = getTransform(elementRect.left, elementRect.top);
@@ -596,7 +596,7 @@ export class CdkDrag<T = any> implements AfterViewInit, OnDestroy {
596596
);
597597
placeholder = this._placeholderRef.rootNodes[0];
598598
} else {
599-
placeholder = this._rootElement.cloneNode(true) as HTMLElement;
599+
placeholder = deepCloneNode(this._rootElement);
600600
}
601601

602602
placeholder.classList.add('cdk-drag-placeholder');
@@ -803,3 +803,11 @@ interface Point {
803803
function getTransform(x: number, y: number): string {
804804
return `translate3d(${x}px, ${y}px, 0)`;
805805
}
806+
807+
/** Creates a deep clone of an element. */
808+
function deepCloneNode(node: HTMLElement): HTMLElement {
809+
const clone = node.cloneNode(true) as HTMLElement;
810+
// Remove the `id` to avoid having multiple elements with the same id on the page.
811+
clone.removeAttribute('id');
812+
return clone;
813+
}

0 commit comments

Comments
 (0)