Skip to content

Commit b371b51

Browse files
crisbetoannieyw
authored andcommitted
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. (cherry picked from commit 06294d1)
1 parent 37bf838 commit b371b51

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
@@ -2028,9 +2028,14 @@ describe('CdkDrag', () => {
20282028

20292029
const preview = document.querySelector('.cdk-drag-preview')! as HTMLElement;
20302030
const previewRect = preview.getBoundingClientRect();
2031+
const zeroPxRegex = /^0(px)?$/;
20312032

20322033
expect(item.parentNode).toBe(document.body, 'Expected element to be moved out into the body');
2033-
expect(item.style.display).toBe('none', 'Expected element to be hidden');
2034+
expect(item.style.position).toBe('fixed', 'Expected element to be removed from layout');
2035+
// Use a regex here since some browsers normalize 0 to 0px, but others don't.
2036+
expect(item.style.top).toMatch(zeroPxRegex, 'Expected element to be removed from layout');
2037+
expect(item.style.left).toBe('-999em', 'Expected element to be removed from layout');
2038+
expect(item.style.opacity).toBe('0', 'Expected element to be invisible');
20342039
expect(preview).toBeTruthy('Expected preview to be in the DOM');
20352040
expect(preview.textContent!.trim())
20362041
.toContain('One', 'Expected preview content to match element');
@@ -2042,15 +2047,18 @@ describe('CdkDrag', () => {
20422047
.toBe('none', 'Expected pointer events to be disabled on the preview');
20432048
expect(preview.style.zIndex).toBe('1000', 'Expected preview to have a high default zIndex.');
20442049
// Use a regex here since some browsers normalize 0 to 0px, but others don't.
2045-
expect(preview.style.margin).toMatch(/^0(px)?$/, 'Expected the preview margin to be reset.');
2050+
expect(preview.style.margin).toMatch(zeroPxRegex, 'Expected the preview margin to be reset.');
20462051

20472052
dispatchMouseEvent(document, 'mouseup');
20482053
fixture.detectChanges();
20492054
flush();
20502055

20512056
expect(item.parentNode)
20522057
.toBe(initialParent, 'Expected element to be moved back into its old parent');
2053-
expect(item.style.display).toBeFalsy('Expected element to be visible');
2058+
expect(item.style.position).toBeFalsy('Expected element to be within the layout');
2059+
expect(item.style.top).toBeFalsy('Expected element to be within the layout');
2060+
expect(item.style.left).toBeFalsy('Expected element to be within the layout');
2061+
expect(item.style.opacity).toBeFalsy('Expected element to be visible');
20542062
expect(preview.parentNode).toBeFalsy('Expected preview to be removed from the DOM');
20552063
}));
20562064

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)