Skip to content

Commit

Permalink
fix(cdk/drag-drop): not detecting parent when projected using ngTempl…
Browse files Browse the repository at this point in the history
…ateOutlet (#21668)

Some logic was added in #21227 which would only `stopPropagation` when dragging if a
parent drag item is detected. The problem is that we resolve the parent using DI which
won't work if the item is projected through something like `ngTemplateOutlet`.

These changes resolve the issue by falling back to resolving the parent through the DOM.
  • Loading branch information
crisbeto authored Jan 25, 2021
1 parent abdc4a0 commit 92cb67e
Show file tree
Hide file tree
Showing 4 changed files with 112 additions and 7 deletions.
61 changes: 61 additions & 0 deletions src/cdk/drag-drop/directives/drag.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5592,6 +5592,21 @@ describe('CdkDrag', () => {

expect(event.stopPropagation).toHaveBeenCalled();
}));

it('should stop event propagation when dragging item nested via ng-template', fakeAsync(() => {
const fixture = createComponent(NestedDragsThroughTemplate);
fixture.detectChanges();
const dragElement = fixture.componentInstance.item.nativeElement;

const event = createMouseEvent('mousedown');
spyOn(event, 'stopPropagation').and.callThrough();

dispatchEvent(dragElement, event);
fixture.detectChanges();

expect(event.stopPropagation).toHaveBeenCalled();
}));

});
});

Expand Down Expand Up @@ -6557,6 +6572,52 @@ class NestedDragsComponent {
itemDragReleasedSpy = jasmine.createSpy('item drag released spy');
}

@Component({
styles: [`
:host {
height: 400px;
width: 400px;
position: absolute;
}
.container {
height: 200px;
width: 200px;
position: absolute;
}
.item {
height: 50px;
width: 50px;
position: absolute;
}
`],
template: `
<div
cdkDrag
#container
class="container"
(cdkDragStarted)="containerDragStartedSpy($event)"
(cdkDragMoved)="containerDragMovedSpy($event)"
(cdkDragReleased)="containerDragReleasedSpy($event)">
<ng-container [ngTemplateOutlet]="itemTemplate"></ng-container>
</div>
<ng-template #itemTemplate>
<div
cdkDrag
class="item"
#item
(cdkDragStarted)="itemDragStartedSpy($event)"
(cdkDragMoved)="itemDragMovedSpy($event)"
(cdkDragReleased)="itemDragReleasedSpy($event)">
</div>
</ng-template>
`
})
class NestedDragsThroughTemplate {
@ViewChild('container') container: ElementRef;
@ViewChild('item') item: ElementRef;
}

@Component({
styles: [`
.drop-list {
Expand Down
40 changes: 37 additions & 3 deletions src/cdk/drag-drop/directives/drag.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,19 +56,22 @@ import {DragDrop} from '../drag-drop';
import {CDK_DRAG_CONFIG, DragDropConfig, DragStartDelay, DragAxis} from './config';
import {assertElementNode} from './assertions';

const DRAG_HOST_CLASS = 'cdk-drag';

/** Element that can be moved inside a CdkDropList container. */
@Directive({
selector: '[cdkDrag]',
exportAs: 'cdkDrag',
host: {
'class': 'cdk-drag',
'class': DRAG_HOST_CLASS,
'[class.cdk-drag-disabled]': 'disabled',
'[class.cdk-drag-dragging]': '_dragRef.isDragging()',
},
providers: [{provide: CDK_DRAG_PARENT, useExisting: CdkDrag}]
})
export class CdkDrag<T = any> implements AfterViewInit, OnChanges, OnDestroy {
private _destroyed = new Subject<void>();
private static _dragInstances: CdkDrag[] = [];

/** Reference to the underlying drag instance. */
_dragRef: DragRef<CdkDrag<T>>;
Expand Down Expand Up @@ -193,17 +196,21 @@ export class CdkDrag<T = any> implements AfterViewInit, OnChanges, OnDestroy {
@Optional() private _dir: Directionality, dragDrop: DragDrop,
private _changeDetectorRef: ChangeDetectorRef,
@Optional() @Self() @Inject(CDK_DRAG_HANDLE) private _selfHandle?: CdkDragHandle,
@Optional() @SkipSelf() @Inject(CDK_DRAG_PARENT) parentDrag?: CdkDrag) {
@Optional() @SkipSelf() @Inject(CDK_DRAG_PARENT) private _parentDrag?: CdkDrag) {
this._dragRef = dragDrop.createDrag(element, {
dragStartThreshold: config && config.dragStartThreshold != null ?
config.dragStartThreshold : 5,
pointerDirectionChangeThreshold: config && config.pointerDirectionChangeThreshold != null ?
config.pointerDirectionChangeThreshold : 5,
zIndex: config?.zIndex,
parentDragRef: parentDrag?._dragRef
});
this._dragRef.data = this;

// We have to keep track of the drag instances in order to be able to match an element to
// a drag instance. We can't go through the global registry of `DragRef`, because the root
// element could be different.
CdkDrag._dragInstances.push(this);

if (config) {
this._assignDefaults(config);
}
Expand Down Expand Up @@ -318,6 +325,10 @@ export class CdkDrag<T = any> implements AfterViewInit, OnChanges, OnDestroy {
this.dropContainer.removeItem(this);
}

const index = CdkDrag._dragInstances.indexOf(this);
if (index > -1) {
CdkDrag._dragInstances.splice(index, -1);
}
this._destroyed.next();
this._destroyed.complete();
this._dragRef.dispose();
Expand Down Expand Up @@ -392,6 +403,29 @@ export class CdkDrag<T = any> implements AfterViewInit, OnChanges, OnDestroy {
}
}
});

// This only needs to be resolved once.
ref.beforeStarted.pipe(take(1)).subscribe(() => {
// If we managed to resolve a parent through DI, use it.
if (this._parentDrag) {
ref.withParent(this._parentDrag._dragRef);
return;
}

// Otherwise fall back to resolving the parent by looking up the DOM. This can happen if
// the item was projected into another item by something like `ngTemplateOutlet`.
let parent = this.element.nativeElement.parentElement;
while (parent) {
// `classList` needs to be null checked, because IE doesn't have it on some elements.
if (parent.classList?.contains(DRAG_HOST_CLASS)) {
ref.withParent(CdkDrag._dragInstances.find(drag => {
return drag.element.nativeElement === parent;
})?._dragRef || null);
break;
}
parent = parent.parentElement;
}
});
}

/** Handles the events from the underlying `DragRef`. */
Expand Down
15 changes: 12 additions & 3 deletions src/cdk/drag-drop/drag-ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,9 @@ export class DragRef<T = any> {
/** Layout direction of the item. */
private _direction: Direction = 'ltr';

/** Ref that the current drag item is nested in. */
private _parentDragRef: DragRef<unknown> | null;

/**
* Cached shadow root that the element is placed in. `null` means that the element isn't in
* the shadow DOM and `undefined` means that it hasn't been resolved yet. Should be read via
Expand Down Expand Up @@ -324,7 +327,7 @@ export class DragRef<T = any> {
private _viewportRuler: ViewportRuler,
private _dragDropRegistry: DragDropRegistry<DragRef, DropListRef>) {

this.withRootElement(element);
this.withRootElement(element).withParent(_config.parentDragRef || null);
this._parentPositions = new ParentPositionTracker(_document, _viewportRuler);
_dragDropRegistry.registerDragItem(this);
}
Expand Down Expand Up @@ -430,6 +433,12 @@ export class DragRef<T = any> {
return this;
}

/** Sets the parent ref that the ref is nested in. */
withParent(parent: DragRef<unknown> | null): this {
this._parentDragRef = parent;
return this;
}

/** Removes the dragging functionality from the DOM element. */
dispose() {
this._removeRootElementListeners(this._rootElement);
Expand Down Expand Up @@ -461,7 +470,7 @@ export class DragRef<T = any> {
this._resizeSubscription.unsubscribe();
this._parentPositions.clear();
this._boundaryElement = this._rootElement = this._ownerSVGElement = this._placeholderTemplate =
this._previewTemplate = this._anchor = null!;
this._previewTemplate = this._anchor = this._parentDragRef = null!;
}

/** Checks whether the element is currently being dragged. */
Expand Down Expand Up @@ -790,7 +799,7 @@ export class DragRef<T = any> {
private _initializeDragSequence(referenceElement: HTMLElement, event: MouseEvent | TouchEvent) {
// Stop propagation if the item is inside another
// draggable so we don't start multiple drag sequences.
if (this._config.parentDragRef) {
if (this._parentDragRef) {
event.stopPropagation();
}

Expand Down
3 changes: 2 additions & 1 deletion tools/public_api_guard/cdk/drag-drop.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export declare class CdkDrag<T = any> implements AfterViewInit, OnChanges, OnDes
constructor(
element: ElementRef<HTMLElement>,
dropContainer: CdkDropList,
_document: any, _ngZone: NgZone, _viewContainerRef: ViewContainerRef, config: DragDropConfig, _dir: Directionality, dragDrop: DragDrop, _changeDetectorRef: ChangeDetectorRef, _selfHandle?: CdkDragHandle | undefined, parentDrag?: CdkDrag);
_document: any, _ngZone: NgZone, _viewContainerRef: ViewContainerRef, config: DragDropConfig, _dir: Directionality, dragDrop: DragDrop, _changeDetectorRef: ChangeDetectorRef, _selfHandle?: CdkDragHandle | undefined, _parentDrag?: CdkDrag<any> | undefined);
getFreeDragPosition(): {
readonly x: number;
readonly y: number;
Expand Down Expand Up @@ -318,6 +318,7 @@ export declare class DragRef<T = any> {
withBoundaryElement(boundaryElement: ElementRef<HTMLElement> | HTMLElement | null): this;
withDirection(direction: Direction): this;
withHandles(handles: (HTMLElement | ElementRef<HTMLElement>)[]): this;
withParent(parent: DragRef<unknown> | null): this;
withPlaceholderTemplate(template: DragHelperTemplate | null): this;
withPreviewTemplate(template: DragPreviewTemplate | null): this;
withRootElement(rootElement: ElementRef<HTMLElement> | HTMLElement): this;
Expand Down

0 comments on commit 92cb67e

Please sign in to comment.