Skip to content

Commit 0816d8e

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 4b15b78 commit 0816d8e

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

741+
it('should clear the id from the preview', fakeAsync(() => {
742+
const fixture = createComponent(DraggableInDropZone);
743+
fixture.detectChanges();
744+
const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement;
745+
item.id = 'custom-id';
746+
747+
startDraggingViaMouse(fixture, item);
748+
749+
const preview = document.querySelector('.cdk-drag-preview')! as HTMLElement;
750+
751+
expect(preview.getAttribute('id')).toBeFalsy();
752+
}));
753+
741754
it('should not create a preview if the element was not dragged far enough', fakeAsync(() => {
742755
const fixture = createComponent(DraggableInDropZone, [], 5);
743756
fixture.detectChanges();
@@ -895,6 +908,20 @@ describe('CdkDrag', () => {
895908
expect(placeholder.parentNode).toBeFalsy('Expected placeholder to be removed from the DOM');
896909
}));
897910

911+
it('should remove the id from the placeholder', fakeAsync(() => {
912+
const fixture = createComponent(DraggableInDropZone);
913+
fixture.detectChanges();
914+
const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement;
915+
916+
item.id = 'custom-id';
917+
918+
startDraggingViaMouse(fixture, item);
919+
920+
const placeholder = document.querySelector('.cdk-drag-placeholder')! as HTMLElement;
921+
922+
expect(placeholder.getAttribute('id')).toBeFalsy();
923+
}));
924+
898925
it('should not create placeholder if the element was not dragged far enough', fakeAsync(() => {
899926
const fixture = createComponent(DraggableInDropZone, [], 5);
900927
fixture.detectChanges();

src/cdk/drag-drop/drag.ts

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

523-
preview = element.cloneNode(true) as HTMLElement;
523+
preview = deepCloneNode(element);
524524
preview.style.width = `${elementRect.width}px`;
525525
preview.style.height = `${elementRect.height}px`;
526526
this._setTransform(preview, elementRect.left, elementRect.top);
@@ -543,7 +543,7 @@ export class CdkDrag<T = any> implements AfterViewInit, OnDestroy {
543543
);
544544
placeholder = this._placeholderRef.rootNodes[0];
545545
} else {
546-
placeholder = this._rootElement.cloneNode(true) as HTMLElement;
546+
placeholder = deepCloneNode(this._rootElement);
547547
}
548548

549549
placeholder.classList.add('cdk-drag-placeholder');
@@ -751,3 +751,11 @@ interface Point {
751751
x: number;
752752
y: number;
753753
}
754+
755+
/** Creates a deep clone of an element. */
756+
function deepCloneNode(node: HTMLElement): HTMLElement {
757+
const clone = node.cloneNode(true) as HTMLElement;
758+
// Remove the `id` to avoid having multiple elements with the same id on the page.
759+
clone.removeAttribute('id');
760+
return clone;
761+
}

0 commit comments

Comments
 (0)