Skip to content

Commit

Permalink
fix(cdk/drag-drop): references to SVG not working inside preview (#20742
Browse files Browse the repository at this point in the history
)

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)
  • Loading branch information
crisbeto authored and annieyw committed Oct 16, 2020
1 parent 37bf838 commit b371b51
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 6 deletions.
14 changes: 11 additions & 3 deletions src/cdk/drag-drop/directives/drag.spec.ts
Expand Up @@ -2028,9 +2028,14 @@ describe('CdkDrag', () => {

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

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

dispatchMouseEvent(document, 'mouseup');
fixture.detectChanges();
flush();

expect(item.parentNode)
.toBe(initialParent, 'Expected element to be moved back into its old parent');
expect(item.style.display).toBeFalsy('Expected element to be visible');
expect(item.style.position).toBeFalsy('Expected element to be within the layout');
expect(item.style.top).toBeFalsy('Expected element to be within the layout');
expect(item.style.left).toBeFalsy('Expected element to be within the layout');
expect(item.style.opacity).toBeFalsy('Expected element to be visible');
expect(preview.parentNode).toBeFalsy('Expected preview to be removed from the DOM');
}));

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

this._destroyPreview();
Expand Down
13 changes: 13 additions & 0 deletions src/cdk/drag-drop/drag-styling.ts
Expand Up @@ -60,3 +60,16 @@ export function toggleNativeDragInteractions(element: HTMLElement, enable: boole
MozUserSelect: userSelect
});
}

/**
* Toggles whether an element is visible while preserving its dimensions.
* @param element Element whose visibility to toggle
* @param enable Whether the element should be visible.
* @docs-private
*/
export function toggleVisibility(element: HTMLElement, enable: boolean) {
const styles = element.style;
styles.position = enable ? '' : 'fixed';
styles.top = styles.opacity = enable ? '' : '0';
styles.left = enable ? '' : '-999em';
}

0 comments on commit b371b51

Please sign in to comment.