Skip to content

Commit 9311f64

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 efeefd1 commit 9311f64

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
@@ -779,6 +779,19 @@ describe('CdkDrag', () => {
779779
expect(preview.parentNode).toBeFalsy('Expected preview to be removed from the DOM');
780780
}));
781781

782+
it('should clear the id from the preview', fakeAsync(() => {
783+
const fixture = createComponent(DraggableInDropZone);
784+
fixture.detectChanges();
785+
const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement;
786+
item.id = 'custom-id';
787+
788+
startDraggingViaMouse(fixture, item);
789+
790+
const preview = document.querySelector('.cdk-drag-preview')! as HTMLElement;
791+
792+
expect(preview.getAttribute('id')).toBeFalsy();
793+
}));
794+
782795
it('should not create a preview if the element was not dragged far enough', fakeAsync(() => {
783796
const fixture = createComponent(DraggableInDropZone, [], 5);
784797
fixture.detectChanges();
@@ -936,6 +949,20 @@ describe('CdkDrag', () => {
936949
expect(placeholder.parentNode).toBeFalsy('Expected placeholder to be removed from the DOM');
937950
}));
938951

952+
it('should remove the id from the placeholder', fakeAsync(() => {
953+
const fixture = createComponent(DraggableInDropZone);
954+
fixture.detectChanges();
955+
const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement;
956+
957+
item.id = 'custom-id';
958+
959+
startDraggingViaMouse(fixture, item);
960+
961+
const placeholder = document.querySelector('.cdk-drag-placeholder')! as HTMLElement;
962+
963+
expect(placeholder.getAttribute('id')).toBeFalsy();
964+
}));
965+
939966
it('should not create placeholder if the element was not dragged far enough', fakeAsync(() => {
940967
const fixture = createComponent(DraggableInDropZone, [], 5);
941968
fixture.detectChanges();

src/cdk/drag-drop/drag.ts

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

553-
preview = element.cloneNode(true) as HTMLElement;
553+
preview = deepCloneNode(element);
554554
preview.style.width = `${elementRect.width}px`;
555555
preview.style.height = `${elementRect.height}px`;
556556
preview.style.transform = getTransform(elementRect.left, elementRect.top);
@@ -580,7 +580,7 @@ export class CdkDrag<T = any> implements AfterViewInit, OnDestroy {
580580
);
581581
placeholder = this._placeholderRef.rootNodes[0];
582582
} else {
583-
placeholder = this._rootElement.cloneNode(true) as HTMLElement;
583+
placeholder = deepCloneNode(this._rootElement);
584584
}
585585

586586
placeholder.classList.add('cdk-drag-placeholder');
@@ -787,3 +787,11 @@ interface Point {
787787
function getTransform(x: number, y: number): string {
788788
return `translate3d(${x}px, ${y}px, 0)`;
789789
}
790+
791+
/** Creates a deep clone of an element. */
792+
function deepCloneNode(node: HTMLElement): HTMLElement {
793+
const clone = node.cloneNode(true) as HTMLElement;
794+
// Remove the `id` to avoid having multiple elements with the same id on the page.
795+
clone.removeAttribute('id');
796+
return clone;
797+
}

0 commit comments

Comments
 (0)