Skip to content

Commit 06294d1

Browse files
authored
fix(cdk/drag-drop): references to SVG not working inside preview (#20742)
When we create a drag preview we clone the original element, move it to the bottom of the `body`, set it to `display: none` and clear all `id` attributes from the clone. As a result, SVG references inside the element break, because the source node is invisible. These changes resolve the issue by moving the original element outside the layout and making it invisible, instead of using `display: none`. Fixes #20720.
1 parent a461929 commit 06294d1

File tree

3 files changed

+27
-6
lines changed

3 files changed

+27
-6
lines changed

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

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2035,9 +2035,14 @@ describe('CdkDrag', () => {
20352035

20362036
const preview = document.querySelector('.cdk-drag-preview')! as HTMLElement;
20372037
const previewRect = preview.getBoundingClientRect();
2038+
const zeroPxRegex = /^0(px)?$/;
20382039

20392040
expect(item.parentNode).toBe(document.body, 'Expected element to be moved out into the body');
2040-
expect(item.style.display).toBe('none', 'Expected element to be hidden');
2041+
expect(item.style.position).toBe('fixed', 'Expected element to be removed from layout');
2042+
// Use a regex here since some browsers normalize 0 to 0px, but others don't.
2043+
expect(item.style.top).toMatch(zeroPxRegex, 'Expected element to be removed from layout');
2044+
expect(item.style.left).toBe('-999em', 'Expected element to be removed from layout');
2045+
expect(item.style.opacity).toBe('0', 'Expected element to be invisible');
20412046
expect(preview).toBeTruthy('Expected preview to be in the DOM');
20422047
expect(preview.textContent!.trim())
20432048
.toContain('One', 'Expected preview content to match element');
@@ -2049,15 +2054,18 @@ describe('CdkDrag', () => {
20492054
.toBe('none', 'Expected pointer events to be disabled on the preview');
20502055
expect(preview.style.zIndex).toBe('1000', 'Expected preview to have a high default zIndex.');
20512056
// Use a regex here since some browsers normalize 0 to 0px, but others don't.
2052-
expect(preview.style.margin).toMatch(/^0(px)?$/, 'Expected the preview margin to be reset.');
2057+
expect(preview.style.margin).toMatch(zeroPxRegex, 'Expected the preview margin to be reset.');
20532058

20542059
dispatchMouseEvent(document, 'mouseup');
20552060
fixture.detectChanges();
20562061
flush();
20572062

20582063
expect(item.parentNode)
20592064
.toBe(initialParent, 'Expected element to be moved back into its old parent');
2060-
expect(item.style.display).toBeFalsy('Expected element to be visible');
2065+
expect(item.style.position).toBeFalsy('Expected element to be within the layout');
2066+
expect(item.style.top).toBeFalsy('Expected element to be within the layout');
2067+
expect(item.style.left).toBeFalsy('Expected element to be within the layout');
2068+
expect(item.style.opacity).toBeFalsy('Expected element to be visible');
20612069
expect(preview.parentNode).toBeFalsy('Expected preview to be removed from the DOM');
20622070
}));
20632071

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import {coerceBooleanProperty, coerceElement} from '@angular/cdk/coercion';
1414
import {Subscription, Subject, Observable} from 'rxjs';
1515
import {DropListRefInternal as DropListRef} from './drop-list-ref';
1616
import {DragDropRegistry} from './drag-drop-registry';
17-
import {extendStyles, toggleNativeDragInteractions} from './drag-styling';
17+
import {extendStyles, toggleNativeDragInteractions, toggleVisibility} from './drag-styling';
1818
import {getTransformTransitionDurationInMs} from './transition-duration';
1919
import {getMutableClientRect, adjustClientRect} from './client-rect';
2020
import {ParentPositionTracker} from './parent-position-tracker';
@@ -732,7 +732,7 @@ export class DragRef<T = any> {
732732
// We move the element out at the end of the body and we make it hidden, because keeping it in
733733
// place will throw off the consumer's `:last-child` selectors. We can't remove the element
734734
// from the DOM completely, because iOS will stop firing all subsequent events in the chain.
735-
element.style.display = 'none';
735+
toggleVisibility(element, false);
736736
this._document.body.appendChild(parent.replaceChild(placeholder, element));
737737
getPreviewInsertionPoint(this._document).appendChild(preview);
738738
this.started.next({source: this}); // Emit before notifying the container.
@@ -827,7 +827,7 @@ export class DragRef<T = any> {
827827
// It's important that we maintain the position, because moving the element around in the DOM
828828
// can throw off `NgFor` which does smart diffing and re-creates elements only when necessary,
829829
// while moving the existing elements in all other cases.
830-
this._rootElement.style.display = '';
830+
toggleVisibility(this._rootElement, true);
831831
this._anchor.parentNode!.replaceChild(this._rootElement, this._anchor);
832832

833833
this._destroyPreview();

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,3 +60,16 @@ export function toggleNativeDragInteractions(element: HTMLElement, enable: boole
6060
MozUserSelect: userSelect
6161
});
6262
}
63+
64+
/**
65+
* Toggles whether an element is visible while preserving its dimensions.
66+
* @param element Element whose visibility to toggle
67+
* @param enable Whether the element should be visible.
68+
* @docs-private
69+
*/
70+
export function toggleVisibility(element: HTMLElement, enable: boolean) {
71+
const styles = element.style;
72+
styles.position = enable ? '' : 'fixed';
73+
styles.top = styles.opacity = enable ? '' : '0';
74+
styles.left = enable ? '' : '-999em';
75+
}

0 commit comments

Comments
 (0)