From 68b53f78e2f2641b29e743a71f63e4d5159bd196 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Sat, 10 Nov 2018 00:43:55 +0000 Subject: [PATCH] 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. --- src/cdk/drag-drop/drag-drop.md | 5 ++++- src/cdk/drag-drop/drag.spec.ts | 27 +++++++++++++++++++++++++++ src/cdk/drag-drop/drag.ts | 12 ++++++++++-- 3 files changed, 41 insertions(+), 3 deletions(-) diff --git a/src/cdk/drag-drop/drag-drop.md b/src/cdk/drag-drop/drag-drop.md index 81144c75feb..546606a010f 100644 --- a/src/cdk/drag-drop/drag-drop.md +++ b/src/cdk/drag-drop/drag-drop.md @@ -113,7 +113,10 @@ restrict the user to only be able to do so using a handle element, you can do it ### Customizing the drag preview When a `cdkDrag` element is picked up, it will create a preview element visible while dragging. By default, this will be a clone of the original element positioned next to the user's cursor. -This preview can be customized, though, by providing a custom template via `*cdkDragPreview`: +This preview can be customized, though, by providing a custom template via `*cdkDragPreview`. +Note that the cloned element will remove its `id` attribute in order to avoid having multiple +elements with the same `id` on the page. This will cause any CSS that targets that `id` not +to be applied. diff --git a/src/cdk/drag-drop/drag.spec.ts b/src/cdk/drag-drop/drag.spec.ts index 971b4d7b31b..4400e62a900 100644 --- a/src/cdk/drag-drop/drag.spec.ts +++ b/src/cdk/drag-drop/drag.spec.ts @@ -896,6 +896,19 @@ describe('CdkDrag', () => { expect(preview.parentNode).toBeFalsy('Expected preview to be removed from the DOM'); })); + it('should clear the id from the preview', fakeAsync(() => { + const fixture = createComponent(DraggableInDropZone); + fixture.detectChanges(); + const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement; + item.id = 'custom-id'; + + startDraggingViaMouse(fixture, item); + + const preview = document.querySelector('.cdk-drag-preview')! as HTMLElement; + + expect(preview.getAttribute('id')).toBeFalsy(); + })); + it('should not create a preview if the element was not dragged far enough', fakeAsync(() => { const fixture = createComponent(DraggableInDropZone, [], 5); fixture.detectChanges(); @@ -1053,6 +1066,20 @@ describe('CdkDrag', () => { expect(placeholder.parentNode).toBeFalsy('Expected placeholder to be removed from the DOM'); })); + it('should remove the id from the placeholder', fakeAsync(() => { + const fixture = createComponent(DraggableInDropZone); + fixture.detectChanges(); + const item = fixture.componentInstance.dragItems.toArray()[1].element.nativeElement; + + item.id = 'custom-id'; + + startDraggingViaMouse(fixture, item); + + const placeholder = document.querySelector('.cdk-drag-placeholder')! as HTMLElement; + + expect(placeholder.getAttribute('id')).toBeFalsy(); + })); + it('should not create placeholder if the element was not dragged far enough', fakeAsync(() => { const fixture = createComponent(DraggableInDropZone, [], 5); fixture.detectChanges(); diff --git a/src/cdk/drag-drop/drag.ts b/src/cdk/drag-drop/drag.ts index 441f7160a6b..e829f6017d2 100644 --- a/src/cdk/drag-drop/drag.ts +++ b/src/cdk/drag-drop/drag.ts @@ -586,7 +586,7 @@ export class CdkDrag implements AfterViewInit, OnDestroy { const element = this._rootElement; const elementRect = element.getBoundingClientRect(); - preview = element.cloneNode(true) as HTMLElement; + preview = deepCloneNode(element); preview.style.width = `${elementRect.width}px`; preview.style.height = `${elementRect.height}px`; preview.style.transform = getTransform(elementRect.left, elementRect.top); @@ -616,7 +616,7 @@ export class CdkDrag implements AfterViewInit, OnDestroy { ); placeholder = this._placeholderRef.rootNodes[0]; } else { - placeholder = this._rootElement.cloneNode(true) as HTMLElement; + placeholder = deepCloneNode(this._rootElement); } placeholder.classList.add('cdk-drag-placeholder'); @@ -823,3 +823,11 @@ interface Point { function getTransform(x: number, y: number): string { return `translate3d(${x}px, ${y}px, 0)`; } + +/** Creates a deep clone of an element. */ +function deepCloneNode(node: HTMLElement): HTMLElement { + const clone = node.cloneNode(true) as HTMLElement; + // Remove the `id` to avoid having multiple elements with the same id on the page. + clone.removeAttribute('id'); + return clone; +}